chaijs / chai-as-promised

Extends Chai with assertions about promises.
MIT License
1.42k stars 109 forks source link

node 15, eventually.throw ? #267

Open drom opened 4 years ago

drom commented 4 years ago

This construct works in Node 10,12,14 and breaks in Node 15

expect( func(arg) ).to.eventually.throw(TypeError);

Real test case: https://github.com/sifive/duh-core/blob/master/test/invalid.js#L26

Node 12 (worked before) https://github.com/sifive/duh-core/runs/1301279599?check_suite_focus=true#step:4:49

Node 15 (does not work as expected) https://github.com/sifive/duh-core/runs/1301279658?check_suite_focus=true#step:4:60

markstos commented 3 years ago

This behavior change is due to a documented breaking change in Node 15:

Throw on Unhandled Rejections As of Node.js 15, the default mode for unhandledRejection is changed to throw > (from warn). In throw mode, if an unhandledRejection hook is not set, the unhandledRejection is raised as an uncaught exception. Users that have an unhandledRejection hook should see no change in behavior, and it’s still possible to switch modes using the --unhandled-rejections=mode process flag.

So you can see that the code will still work with Node 15 or 16 if you call it with:

node --unhandled-rejections=warn

You can also workaround it by adding this to your tests, realizing that you are modifying the global environment:

process.on('unhandledRejection', (err, p) => {
  console.error('unhandledRejection', err.stack, p)
})

A global solution to add this to your tests is to create a "bootstrap" file that is loaded before all your tests run and add the code there. But this may cause your code to issue warning when under test but then throw in production. 🤷🏼

mocha --require 'test/lib/bootstrap.js'
markstos commented 3 years ago

Another option is to quit using chai-as-promised to test rejected promises. This simplifies your code, doesn't require a global handler that might have unintended side-effects and is fairly straightforward.

The pattern is to attach a then handler to your promise that asserts failure if called, then attach a catch/reject handler to the promise that does nothing if called, becoming a "successful" promise in the process. Here's an example:

return Promise.reject(new Error()).then(() => assert.fail(), () => {});

That's using a two-argument call to then where the second argument is the do-nothing rejection handler. If you want to test that a successul promise would fail if used with this patten, here's an example where the assertion will fail because the first promise will successed, triggering assert.fail():

 return Promise.resolve().then(() => assert.fail(), () => {});

This is approach I'm going with. Promises are already complex enough so removing the extra complexity added by the global side-effects of chai-as-promised can be considered an improvement to the code base.