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

feat(jest-circus): Fail tests if a test takes a done callback and have return values #9129

Merged
merged 6 commits into from
May 2, 2020

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Nov 4, 2019

Summary

Using both promises and the callback is confusing as we don't actually wait for the promise at all. People should prefer promises (async functions) for all async work anyways as propagation is handled for them.

Not sure of we wanna land this for 25 or not. It's potentially a pain to fix, although using https://github.com/jest-community/eslint-plugin-jest/blob/master/docs/rules/no-test-callback.md makes it somewhat easier.

Didn't bother adding this to jasmine as it has some internal Spec and our promisifying thing that confused me.

This also fixes the case where

test('foo', () => {
  return 'bar';
});

would fail due to the returned string, but

test('foo', cb => {
  cb();

  return 'bar';
});

would not

Test plan

e2e test added. Haven't ran this locally in case

Copy link
Contributor

@jeysal jeysal left a comment

Choose a reason for hiding this comment

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

Nice

packages/jest-circus/src/utils.ts Outdated Show resolved Hide resolved
@SimenB SimenB added this to the Jest 26 milestone Apr 23, 2020
@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 11, 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