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

[new rule] prefer await expect(...).resolves #803

Closed
G-Rath opened this issue Mar 27, 2021 · 3 comments · Fixed by #822
Closed

[new rule] prefer await expect(...).resolves #803

G-Rath opened this issue Mar 27, 2021 · 3 comments · Fixed by #822

Comments

@G-Rath
Copy link
Collaborator

G-Rath commented Mar 27, 2021

Pretty much the opposite to the now-deprecated no-expect-resolves: prefer doing await expect(...).resolves over expect(await ...) as it gives a better failure message.

With resolves, if the promise rejects it'll fail the assertion, giving you an error on the lines of expected promise to resolve but it rejected with ....
expect(await ...) however means that the rejection will be thrown "outside" of the expect, giving you a more generic "test thrown an unexpected error" message.

This also affected the behaviour of expect.hasAssertions & co, as with resolves you'll be hitting an assertion which fails, which is considered a "passin the eyes ofhasAssertions`.

I think prefer-expect-resolves could be a good name for this new rule.

@G-Rath
Copy link
Collaborator Author

G-Rath commented Apr 6, 2021

@SimenB I just came across an interesting difference that I didn't realise with the stack traces.

This code:

await expect(
  reactor.react(message, [['robot_face', true]])
).resolves.toBeUndefined();

Gives me this stacktrace:

Error: expect(received).resolves.toBeUndefined()

Received promise rejected instead of resolved
Rejected to value: [TypeError: Cannot read property 'join' of undefined]

    at expect (/c/Users/G-Rath/workspace/projects-ackama/pr-monitor/backend/node_modules/expect/build/index.js:134:15)
    at Object.<anonymous> (/c/Users/G-Rath/workspace/projects-ackama/pr-monitor/backend/test/src/reactor/MessageReactor.spec.ts:129:17)
    at Promise.then.completed (/c/Users/G-Rath/workspace/projects-ackama/pr-monitor/backend/node_modules/jest-circus/build/utils.js:276:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (/c/Users/G-Rath/workspace/projects-ackama/pr-monitor/backend/node_modules/jest-circus/build/utils.js:216:10)
    at _callCircusTest (/c/Users/G-Rath/workspace/projects-ackama/pr-monitor/backend/node_modules/jest-circus/build/run.js:212:40)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at _runTest (/c/Users/G-Rath/workspace/projects-ackama/pr-monitor/backend/node_modules/jest-circus/build/run.js:149:3)
    at _runTestsForDescribeBlock (/c/Users/G-Rath/workspace/projects-ackama/pr-monitor/backend/node_modules/jest-circus/build/run.js:63:9)
    at _runTestsForDescribeBlock (/c/Users/G-Rath/workspace/projects-ackama/pr-monitor/backend/node_modules/jest-circus/build/run.js:57:9)

Whereas the expect(await ...) version gives me this stacktrace:

Cannot read property 'join' of undefined
TypeError: Cannot read property 'join' of undefined
    at MessageReactor._ensureInChannel (/c/Users/G-Rath/workspace/projects-ackama/pr-monitor/backend/src/reactor/MessageReactor.ts:69:39)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at MessageReactor._react (/c/Users/G-Rath/workspace/projects-ackama/pr-monitor/backend/src/reactor/MessageReactor.ts:87:7)

While the first is (imo) nicer as it's the test failing rather than throwing a raw error, its stacktrace doesn't point me to where the error was actually thrown - instead it just points me at jest-circus internals.

Is this known on the jest side? I've found a few reported issues that seem close but nothing that seems to match this exactly.

@github-actions
Copy link

🎉 This issue has been resolved in version 24.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link

🎉 This issue has been resolved in version 25.0.0-next.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant