jest-circus doesn't fail tests on timeouts set by jest.setTimeout

[SimenB]

🐛 Bug Report

jest-circus happily passes a test which times out.

To Reproduce

See instructions for running circus here: #4362.

Then one can run the following test (which should fail, but it passes): https://github.com/facebook/jest/blob/d4a7fca5a6da9d2545467bff8b6a6043b1c3aa3c/integration-tests/failures/__tests__/async_failures.test.js#L27-L31

Expected behavior

It should fail

Link to repl or repo (highly encouraged)

The Jest repo reproduces.

[SimenB]

This wasn't fixed, just referenced

[aaronabramov]

hm.. it was supposed to be fixed in #6300
don't we have a test for it?

[aaronabramov]

it might keep the test green but fail the whole suite

[SimenB]

Ah, I didn't actually test it, just noticed this issue was closed by wrong PR. Lemme double check

[SimenB]

Nope, still doesn't fail the test.

'use strict';

test('timeout', done => {
  jest.setTimeout(5);
  setTimeout(done, 50);
});

$ JEST_CIRCUS=1 ../../jest async
 PASS  __tests__/async_failures.test.js
  ✓ timeout (55ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        0.645s, estimated 1s
Ran all test suites matching /async/i.

$ ../../jest async
 FAIL  __tests__/async_failures.test.js
  ✕ timeout (17ms)

  ● timeout

    Timeout - Async callback was not invoked within the 5ms timeout specified by jest.setTimeout.

       9 | 'use strict';
      10 |
    > 11 | test('timeout', done => {
         | ^
      12 |   jest.setTimeout(5);
      13 |   setTimeout(done, 50);
      14 | });

      at Spec (../../packages/jest-jasmine2/build/jasmine/Spec.js:85:20)
      at Object.<anonymous> (__tests__/async_failures.test.js:11:1)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 total
Snapshots:   0 total
Time:        0.427s, estimated 1s
Ran all test suites matching /async/i.

[SimenB]

Timeout in general work, but not setting it though jest.setTimeout

[aaronabramov]

i remember now! didn't we decide to remove this case?
using jest.setTimeout from within the test makes it change the timeout after the timer already started ticking, which will require some dynamic re-checking of elapsed time.

[thymikee]

Maybe we could disable calling this inside function, since there is a third argument for that?

[SimenB]

I'm personally fine with dropping it, men it should throw then, not silently do nothing.

@cpojer?

[cpojer]

I'd first like to see an analysis of how this would impact fb, if at all. I think we can change it, but only if @aaronabramov takes care of the upgrade.

[aaronabramov]

it wasn't than bad when i ran it with jest-circus, although i definitely saw a few timeouts. I'll have an update later when i fix all other broken tests :)

[nvh95]

I just had some tests with jest.setTimeout inside it block of code (using jest-circus 26.0.1). Move them out of it or test will work.

// Will not work
describe('components/Modals/ExpertMatchingStatic', () => {
  it('test might takes 6000ms', () => {
    jest.setTimeout(7000);
   // ... 
  });
});

// Will work
describe('components/Modals/ExpertMatchingStatic', () => {
  jest.setTimeout(7000);
  it('test might takes 6000ms', () => {
    // ... 
  });
});

[joscha]

Is there some news on this or is the suggested way of working around this to only ever change the timeout in a describe block but never a test itself?

[github-actions]

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 14 days.

[github-actions]

This issue was closed because it has been stalled for 7 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

[github-actions]

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.