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

Tests can pass despite assertion failures #4401

Closed
henrycatalinismith opened this issue Aug 31, 2017 · 11 comments
Closed

Tests can pass despite assertion failures #4401

henrycatalinismith opened this issue Aug 31, 2017 · 11 comments

Comments

@henrycatalinismith
Copy link

Hey there! I suspect there might be a good explanation for this, but it also seems possible for it to be a bug, so here goes! 😄

Current Behaviour

Tests can pass despite the failure of some of the assertions that they execute. As per the instructions in the issue template, I've prepared a minimal repository on GitHub that demonstrates the issue with as little unnecessary extra detail as possible. I'll paste the code here as well since it's small enough to do so.

function dodgyDoubler(n, cb) {
  setTimeout(() => cb(99999), 10); // send a wrong answer first
  setTimeout(() => cb(n * 2), 20); // then send the right answer
}

describe('dodgyDoubler', () => {
  it('eventually doubles a number', done => {
    dodgyDoubler(3, six => {
      console.log(six);
      expect(six).toEqual(6);
      done();
    });
  });
});

Expected Behaviour

If any assertion fails, the test should fail. In my example above, the expect(six).toEqual(6) assertion fails but the test passes anyway.

Misc Runtime Environment Information

  • Jest 20.0.4
  • Node 8.4.0
  • npm 5.3.0
  • macOS 10.12.6
@palmerj3
Copy link
Contributor

@rogeliog perhaps you could take a look? Would be nice to know if this is at least confirmed by Jest collaborators. Confirmed on my end.

Sorry for the ping but this has gotten no acknowledgement in 2 weeks :)

@thymikee
Copy link
Collaborator

This should be handled properly in Jest 21, see: #2059.

@henrycatalinismith
Copy link
Author

henrycatalinismith commented Sep 14, 2017

I've upgraded the example repo to Jest 21.1.0 and unfortunately the bug still persists.

Kind of inclined to leave this issue closed though, as I sense this is somehow a fundamentally difficult thing about building a JavaScript unit testing framework. Fixing it would probably entail a significant backwards compatibility breakage as well, cos I suspect there are a lot of folks out there with test suites that depend on this bug in order to pass, despite lots of assertion failures they've never noticed.

If it's not easily fixable, one thing I think would be a lovely alternative is if Jest could be configured to print some kind of warning when an AssertionError is thrown by a passing test. I'd certainly love to be able to quantify the impact of this behaviour in our codebase and check it's not hiding any actual bugs 😄

@thymikee
Copy link
Collaborator

cc @dignifiedquire

@dignifiedquire
Copy link
Contributor

As far as I understand the thing that could be fixed here is that done should throw when called a second time. The explanation as to why this passes atm is that the test is considered finished once done is called so later exceptions are not tracked anymore as expected.

@henrycatalinismith
Copy link
Author

In that example I think the done only runs once. The first time the callback fires, the assertion fails and stops the function, so you never reach done. The second time, the assertion passes and done is called. Been looking into it a bit today and it seems like something must be swallowing that assertion error. I tried injecting a process.on('uncaughtException') handler into the transformed source here but it doesn't fire. Seems like a really really tough one IMO.

@dignifiedquire
Copy link
Contributor

right, sorry the first one is the wrong one, I misread interesting this sounds like a bug definitely then

@palmerj3
Copy link
Contributor

Since this seems valid can it be reopened? @dignifiedquire

@thymikee thymikee reopened this Sep 18, 2017
@kimjoar
Copy link
Contributor

kimjoar commented Sep 26, 2017

EDIT: Oh, this might relate to #2059, but not this issue. Sorry about the noise.

This fails when using jsdom, but not when using the node env.

Using node:

$ npx jest --env node
 FAIL  test/index.test.js
  ● exception promise

    expect(received).toEqual(expected)
    
    Expected value to equal:
      false
    Received:
      true
      
      at Timeout.setTimeout [as _onTimeout] (test/index.test.js:3:18)
      at ontimeout (timers.js:469:11)
      at tryOnTimeout (timers.js:304:5)
      at Timer.listOnTimeout (timers.js:264:5)

  ✕ exception promise (18ms)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 total
Snapshots:   0 total
Time:        0.216s, estimated 6s
Ran all test suites.

Using jsdom:

$ npx jest --env jsdom
 FAIL  test/index.test.js (5.114s)
  ● exception promise

    Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL.
      
      at node_modules/jest-jasmine2/build/queue_runner.js:65:21
      at ontimeout (timers.js:469:11)
      at tryOnTimeout (timers.js:304:5)
      at Timer.listOnTimeout (timers.js:264:5)

  ✕ exception promise (5003ms)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 total
Snapshots:   0 total
Time:        5.733s
Ran all test suites.

Minimal repro: https://github.com/kjbekkelund/jest-timeout-issue-jsdom

@thymikee
Copy link
Collaborator

thymikee commented Jun 6, 2018

@kimjoar on Jest 23 it consistently fails on both environments :)

@thymikee thymikee closed this as completed Jun 6, 2018
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants