Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

awaiting a rejected promise can still pass test #812

Closed
rankida opened this issue Feb 22, 2018 · 2 comments · Fixed by #813
Assignees
Labels
bug
Milestone

Comments

@rankida
Copy link
Contributor

@rankida rankida commented Feb 22, 2018

The following test "passes" in lab@15.2.2 when I would not expect it to as it awaits a rejected promise

it.only('lab should mark as fail, but doesn`t', async () => {

    let rejectP;
    const p = new Promise((resolve, reject) => {

        rejectP = reject;
    });
    setTimeout(rejectP, 5); // if I passed a param to rejectP the test would fail

    await p;

    console.log("never called - but test passes");
    throw new Error('This test should fail, but passes');
});

What happens is that await p results in an uncaught exception, but because rejectP was called with no parameter then all the checks like if (err in the runner pass.

A quick hack for me would be to add the below || 'rejected' to lib/runner.js#L581

return finish(ex || 'rejected');

I am happy to prepare a PR if you are agreeable. Let me know.

@lerouxb

This comment has been minimized.

Copy link

@lerouxb lerouxb commented Feb 22, 2018

Both of these pass too:

'use strict';

const { describe, it } = exports.lab = require('lab').script();

describe('throw', () => {

  it('should not pass (async)', async () => {

    throw null;
  });

  it('should not pass (sync)', () => {

    throw null;
  });
});

(Or obviously undefined or anything falsey)

@rankida

This comment has been minimized.

Copy link
Contributor Author

@rankida rankida commented Feb 22, 2018

This has rather dangerous in my opinion because it gives a false sense of success and unless you are following strict TDD and watching it go red first then you can easily think a feature is working when it is not.

For this reason I have opened a PR - #813

@geek geek closed this in #813 Feb 25, 2018
@geek geek added the bug label Feb 25, 2018
@geek geek added this to the 15.3.0 milestone Feb 25, 2018
@geek geek self-assigned this Feb 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.