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

Fix asynchronous test will fail due to timeout issue. #4669

Merged
merged 2 commits into from
Oct 15, 2017

Conversation

H1Gdev
Copy link
Contributor

@H1Gdev H1Gdev commented Oct 12, 2017

  • Send error event to Node.js process.

Summary

Execute following asynchronous test.

it('asynchronous test', (done) => {
  setTimeout(function() {
    expect(false).toBeTruthy();
    done();
  }, 1 * 1000);
});

Expect:

    expect(received).toBeTruthy()

    Expected value to be truthy, instead received
      false

Actual:

Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL.

Test plan

Above test result as expected.

  ● describe ? asynchronous test

    expect(received).toBeTruthy()

    Expected value to be truthy, instead received
      false

      at __tests__/test.spec.js:23:21
      at Timeout.callback [as _onTimeout] (node_modules/jsdom/lib/jsdom/browser/Window.js:523:19)
      at ontimeout (timers.js:365:14)
      at tryOnTimeout (timers.js:237:5)
      at Timer.listOnTimeout (timers.js:207:5)

@SimenB
Copy link
Member

SimenB commented Oct 12, 2017

I don't understand how this i happening but I can confirm both the issue, and the fact that this diff fixes it. Thanks!

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs an integration test

@H1Gdev
Copy link
Contributor Author

H1Gdev commented Oct 13, 2017

@SimenB
Thank you for your review.

When throw exception in callback function, jsdom window catches and send error event.(like web browser)

So in this PR receive error event from jsdom and send error event to Node.js process.

@SimenB
Copy link
Member

SimenB commented Oct 13, 2017

Cool, thanks for the explanation!

Adding an integration test for this should be pretty simple, just assert on stdout that it contains e.g. Expected value to be truthy, instead received

* Send error event to Node.js process.
@SimenB
Copy link
Member

SimenB commented Oct 14, 2017

Thanks!

I updated the test you wrote to actually run in jsdom (it passed without your changes as well, as it ran in node environment).

* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @jest-environment jsdom
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now there is no way to assert what testEnvironment a test is run in. But I doubt someone will change this anyways, so shouldn't be a problem

@cpojer cpojer merged commit 291e836 into jestjs:master Oct 15, 2017
@cpojer
Copy link
Member

cpojer commented Oct 15, 2017

This is a nice fix, thank you for sending a PR.

@SimenB
Copy link
Member

SimenB commented Oct 15, 2017

@H1Gdev would you mind sending a PR adding a line to the changelog about this? 🙂

@H1Gdev
Copy link
Contributor Author

H1Gdev commented Oct 15, 2017

@SimenB
Yes of course.

@H1Gdev H1Gdev deleted the error branch October 16, 2017 00:26
H1Gdev added a commit to H1Gdev/jest that referenced this pull request Nov 8, 2017
cpojer pushed a commit that referenced this pull request Nov 8, 2017
@dfroger dfroger mentioned this pull request Nov 16, 2017
alexsoble added a commit to studentinsights/studentinsights that referenced this pull request Aug 14, 2018
+ Better error handling: got stuck for a bit on #1973 because jest was giving me an "Timeout - Async callback was not invoked within timeout" error, when the real issue was much simpler than that
+ See jestjs/jest#4669 for details.
@github-actions
Copy link

This pull request 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 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants