unexpectedjs / unexpected-express Goto Github PK

Hi there,

I think a potential footgun has been created with the promisified version of the library when used with mocha. I didn't quite understand the importance of the return (I had no idea that you could return a promise and mocha would latch onto it) in the example it() block so I left it off at first. I also left out a 'done' parameter to the function I passed to it().

In unexpected-express 5.0.4, leaving off the done would have resulted in an exception that would immediately alert the user to the problem. However, as originally written the test passed, I got a false positive but the assertions had even been run. My suggestion, to catch this case, would be to throw an exception if no done was attached to the promise. I noticed that a bunch of then() blocks are used inside the code?

I wonder if it would be possible to store the count of the callback and assert if did not increase? That, or if that's over-engineering then maybe mention it explicitly in the docs. I also wonder if the version of mocha would matter?

At any rate, hope this helps. Alex J Burke.

lodash is not a dependency of unexpected-express, but a transitive dependency. Under normal circumstances it will work, as magicpen-media depends on lodash and is a dependency of unexpected-messy, which itself is a dep of this module. But if you have a dependency of lodash that is not compatible with magicpen-media, lodash will be hoisted into the magicpen-media/node_modules folder.

intersection is added to lodash in v3.0.0, so you need to use v2 of lodash to trigger this error.

The following works fine:

$ mkdir foobar && cd foobar
$ npm init -y
$ npm i -S unexpected-express
$ node node_modules/unexpected-express/lib/resolveExpectedResponseProperties.js 
$ echo $?
0

To reproduce the error, continue:

$ npm i lodash@^2
$ node node_modules/unexpected-express/lib/resolveExpectedResponseProperties.js 
module.js:550
    throw err;
    ^

Error: Cannot find module 'lodash/intersection'
[...]

It's not possible to test POST requests with content-type application/x-www-form-urlencoded or multipart/form-data.

I was trying to use https://www.npmjs.com/package/request#forms as an inspiration on how to test form posts with unexpected-express. But it seems the implementation for { form: { ... } } is missing

I have a need for extracting the response body (for grabbing returned state), but this feel quite wrong to do:

expect(foo, 'to yield exchange', {...})
  .then(function (exchange) {
    var body = JSON.parse(exchange.httpExchange.response._rawBody.toString('utf-8'));
    ...
   })

There wouldn't be a better way of doing this somewhere?

After getting into testing some non-idempodent flows I ended up missing something like this:

expect(foo, 'to yield exchanges satisfying', [{
    request: {...},
    response: {...}
}, {
    request: {...},
    response: {...}
}]);

Basically short-hand for

expect(foo, 'to yield exchange satisfying', {...})
    .then(function () {
         return expect(foo, 'to yield excchange satisfying', {...});
     });

Comment says Please prefer this one because it does use 'to satisfy' semantics, so I guess it is important :)

I got a basic documentation site set up, now published to http://unexpected.js.org/unexpected-express/

However, it's pretty much a copy/paste of the old README with the mistakes and uses of outdated syntax fixed. We should set up a properly structured site with a page per assertion and maybe some more example-driven tutorials.

This should also include finishing the documentation site for unexpected-messy, which would be the place to document most of the semantics of expect(app, 'to yield exchange', { request: ..., response: ...}), which could be shared with unexpected-http and unexpected-mitm as well.

// cc @alexjeffburke

This code throws error:

return expect(app, 'to yield exchange', {
...
}.then(function (context) {
  done() // Has NOT been executed
})

"before each" hook: timeout of 2000ms exceeded

"after each" hook: TODO: You have created a promise that was not returned from the it block

The code in tested express middleware (SUT) seems to be executed correctly

Current library versions:

"unexpected": "~10.9.1",
"unexpected-express": "~8.1.0",

I just accidentally did:

return expect(app, 'to yield exchange', { request: 'GET /', response: 200, headers: { ... } });

... and wondered why the headers didn't do anything.

Hello everyone! 👋

At the home page http://unexpected.js.org/unexpected-express/, there is a section called "Testing POST requests" where it says that is possible to use the data property in the request object to test POST requests. However in the example below, it says body instead of data. See attachment below.

Could you please 😄 change the example from

return expect(express().use(myMiddleware), 'to yield exchange', {
  request: {
    url: 'POST /api/',
    body: {
      title: 'Hello World',
    },
  },
  response: 200,
});

to

return expect(express().use(myMiddleware), 'to yield exchange', {
  request: {
    url: 'POST /api/',
    data: {
      title: 'Hello World',
    },
  },
  response: 200,
});

Thank you very much in advance! 🙇

Hey,

I's using unexpected-express to assert a bunch of known values in a response, but then I wanted to attach a an extra assertion in a then() on the promise to assert an extra unknown value had been attached to the body. It seems the sanitised ServerResponse object returned does not provide the response body.

I think it occurs where the library has taken care of buffering the response bytes such that assertions can be done on them - the messy.HttpResponse() construction on line 322 seems to put those bytes back together as a breakpoint just after it shows it has an _body property which does have the response object and it has been JSON parsed. However, the httpResponse _body property is never attached to the ServerResponse that is actually returned in the context.

I think a fix could be to add extra line on after 325 that does res.body = httpResponse._body (potentially protected by an if). Does my assessment seem correct? I am happy to make a pull request if desired - my only other question is whether there are other things that should also be attached e.g. headers.

Thanks, Alex J Burke.

I wrote a test something like this:

var someMiddlewareThatAddsThingsToResLocals = function (req, res, next) { next(); };

it('should add stuff to res.locals', function (done) {
    expect(express().use(someMiddlewareThatAddsThingsToResLocals), 'to yield exchange', {
        request: '/foo',
        response: {
            locals: {
                stuff: 'bar'
            }
        }
    }, done);
});

You would expect this test to fail, as the middleware is a TDD-lie, that actually does not add anything to res.locals.

@papandreou explained me that the reason is that the object is not a representation of the express response object but a HTTP-exchange object.

Properties defined on the response object in the assertion, but not having a corresponding property on the HTTP-exchange object, should probably throw an error, telling the user that said property is invalid.

By default, the HTTP server objects need to be undef'd after each test, otherwise you end up with collisions. This can be most easily demonstrated if you setup several tests that expect a 200 response, and several tests that expect other responses. Server instances built to handle test 1 may respond to test 2, etc. Better isolation is required in order to make assure reliability. It appears that the current workaround required that you only ever perform one test using unexpected-express per test file.

Right now it only works with streams and JSON (magically through req.body).