Add status and json to response

[justhamade]

This is already done but looks like there wasn't a pull request made.
SeanCannon@5a6a3d5

[darrin]

Stumbled across dupertest yesterday. Thanks for creating this project - spinning up a server was painful.

Today trying to use it for real and need status and json immediately. I'll use the @justhamade's fork until we fold this back in.

So yeah... +1. Anything needed to get this formally into a release?

[TGOlson]

@darrin would using Request.prototype.extendRes work for you? I think we'll merge this in once the PR gets some tests associated with it, but just curious if you could do:

function status(sts) {
  this.res.status = sts;
}

request(<action>)
  .extendRes({status: status})
  ...

[darrin]

@TGOlson Thanks for the rapid response! I could make that work but I'd need to do that for just about every request so that my controllers don't crash. BUT I'd like to keep my tests DRY so I'd probably create a wrapper to do this for me which would be unfortunate and ugly. What I'd really want status, json and probably some others to be part of the response all the time.

If you're opposed to adding these into the res in the framework then I wonder if it'd make sense to have the ability to extend the result from the request object so that all request actions could automatically include it... I suspect most people using your framework would have the same requirements for the res object across huge categories of test.

function status(sts) {
  this.res.status = sts;
}

request.extendRes({status: status});

request(<action>)  // has status defined per above.
  ...

Food for thought...

[TGOlson]

@darrin thanks for trying that - I totally get what you are saying about having to do it so many times. I noted this in the referenced PR, but I think the best solution would be to bring in a better mock request/response library so we don't have to treat these as one-offs. In the mean time, once that PR gets a couple tests it should get merged in soon.

In an initial iteration of this library I tried to include a request.setDefaults function that did something like you have in your example. That problem with that is that node modules are singletons, so request required from one module is the same request required by another module. This means a modification at one call site affects all the others. This puts the user in a tough spot of having to set the defaults as part of the test setup, then clear them as part of the teardown:

beforeEach(() => request.setDefaults(...));
afterEach(() => request.resetDefaults());

I think a better solution if there are a lot of defaults to set is to create a custom 'wrapper' function and then wrap request each time you want to use these defaults:

function status(sts) {
  this.res.status = sts;
}

function requestWithDefaults(req) {
  return function(action) {
    return req(action).extendRes({status: status})
  }
}

var request = requestWithDefaults(require('request'));

[darrin]

@TGOlson - thanks for the suggestion. For now I'm using the aforementioned fork but I was thinking that I might use a wrapper (pretty much as you've shown above) if you're not going to take @justhamade's pull request. Since I've got a private npm repo I'll probably wrap the whole thing into a module of my own so I can just do this once:

var request = require('express-mock-controller-request');

I see this being a complaint in your future though. You've now seen two groups... at least 3 people needing to do this - so I believe you're going to see it again. (We) People are fundamentally lazy so they want something that just works for their case. You're out of the box experience is very close to allowing me to use this for my express controllers but the mods I have to make could very well dissuade the weak hearted.

With that mindset - another approach that would allow you to work around the singleton issue you cited above would be to build in a factory method... what I'm thinking is:

var request = require('request');
var expressReq = request.for('express');

If you didn't want to actually add support specifically for 'express' BUT wanted to allow framework support so that others could - you could just allow this to be a means to setup defaults:

var request = require('request');
var expressReq = request.set('namedDefaults', { <defaults> });
var req = request.for('namedDefaults');

Obviously the set method here sets up the defaults for the named request type... and the 'for' function allows you to build a request for that type. This would allow your users to set these up and create reusable wrapped requests neatly without inventing it themselves.

[darrin]

@TGOlson -

I wanted to report back on some findings... I was able to get my tests passing using the wrapper technique we discussed. What I believe this accomplishes is the same thing that @justhamade accomplished in his fork - though honestly my focus was to get status, json and body to work so I may have missed something.

First here's the code:

    function send(body) {
        this.res.body = body;
        return this.res;
    }
    function json(object) {
        return this.res.send(object);
    }

    function status(sts) {
        this.res.status = sts;
        this.res.statusCode = sts;
        return this.res;
    }

    function expressifyRequest(req) {
        return function (action) {
            var request = req(action);
            return request
                .extendRes({
                    send: send.bind(request),
                    status: status.bind(request),
                    json: json.bind(request)
                });
        };
    }

    var request = expressifyRequest(require('dupertest'));

A couple notes:

1. I needed to `bind(request) in each function to give me access to the request from the 'this' object above...
2. I was expecting to find the contents of send in req.body... Note: I think I can buy into the simplification that send just sends the object... but have a lot of tests that assume res.body is where the results are - I need to think on that more.
3. Currently dupertest doesn't allow override of the send method bc of this code in the Request.end method:

  this.res.send = makeFunction(callback);

A quick way to get this to work and be backward compatible is here:

  var sendOverride = makeFunction(this.res.send);
  this.res.send = function(body) {
     sendOverride(body);
     return makeFunction(callback)(this.res);
  }.bind(this);

If you're okay with this change I'll put together a quick pull request and should be able to put together a quick test to show it works.

[darrin]

All - I ended up working around the issue by creating a wrapper as noted above which I use to wrap methods to every request created by dupertest using:

var request = expressifyRequest(require('dupertest'))

This has served me well except in the rare case I wanted to do something interesting with request.send. I did some work to allow override send which otherwise gets squashed when end is called. I've altered a test, added one and put it all into pull request 13 in case anyone would like.