Added a flag (-R, --rejections) to make the test fail if an unhandled Promise rejection happened during its execution

[DanReyLop]

Fixes #649

I'm open to suggestions, specially about naming.

All the relevant discussion is on the #649 issue. To sum up, I've added a flag to Lab, called --rejections (alias -R). If that flag is set, and an unhandled Promise rejection happens during a test, that test will be failed.

Without this flag, the behaviour will be exactly the same as before: The test pass, and in versions of Node.JS 6.6.0+, a warning is printed on the console (but the test still passes).

An unhandled Promise rejection looks like this:

function foo() {
    functionThatReturnsPromise()
        .then((res) =>{
            throw new Error('This error will be absolutely swallowed.');
        })
        // No .catch() block here
}

Initially, the behaviour of unhandled rejections was to be silenced and totally ignored, by design. It looks like everyone is changing their minds, because in the latest versions of Chrome and Node.JS (maybe more engines, I don't know more examples) the unhandled rejections have started to display console warnings. In 99% an unhandled rejection means that something is terribly bad with the code, and the fact that doesn't crash the entire process is just a questionable design choice. So I think it should definitely make a test fail.

[DanReyLop]

Ouch. I've added tests to cover this feature in test/cli.js, but the coverage isn't counting those tests (I assume because they spawn child processes). I haven't realized it because as a Windows user I couldn't get npm test to pass 😄

Do you have any suggestion on how to handle this? I thought about adding tests to test/runner.js directly, but since what's being tested here is a CLI flag, it made more sense to test it in test/cli.js

[geek]

instead:

expect(error).to.not.exist();

[geek]

@DanReyLop The naming looks good. For improving the code coverage, you are likely going to need to emit the rejection event manually on the process, and perhaps monkey-patch other areas.

[DanReyLop]

@geek I've ended up adding a test to tests/runner.js to cover the new code and actually make it count for the coverage. Coverage is at 100% again 😄

[geek]

Is there a reason why this needed to change?

[DanReyLop]

Not directly related to this PR, but the previous command didn't work on Windows, because the #! /bin/node trick obviously doesn't work on Windows.

Some tests keep failing on my Windows 10 machine, but with this change at least I can run them.

[lock]

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.