Skip to content

Conversation

@MylesBorins
Copy link
Contributor

Implement asynchronous equivalent for assert.throws() and
assert.doesNotThrow().

PR-URL: #18023
Reviewed-By: James M Snell [email protected]
Reviewed-By: Shingo Inoue [email protected]
Reviewed-By: Joyee Cheung [email protected]
Reviewed-By: Ruben Bridgewater [email protected]

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-botnodejs-github-bot added assert Issues and PRs related to the assert subsystem. v8.x labels Nov 1, 2018
Implement asynchronous equivalent for assert.throws() and assert.doesNotThrow(). PR-URL: nodejs#18023 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Shingo Inoue <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
@MylesBorinsMylesBorinsforce-pushed the backport-assert-rejects branch from 9822e10 to 4ee80bcCompareNovember 1, 2018 17:23
@MylesBorinsMylesBorins added the semver-minor PRs that contain new features and should be released in the next minor version. label Nov 1, 2018
Copy link
Member

@BridgeARBridgeAR left a comment

Choose a reason for hiding this comment

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

LGTM

@BridgeAR
Copy link
Member

@MylesBorins thanks a lot! 👍

@MylesBorins
Copy link
ContributorAuthor

@BridgeAR are there any other semver-minor changes to assert we should look at backporting for the next 8.x?

@not-an-aardvark
Copy link
Contributor

It would probably be worth also backporting the two commits from #19650 at the same time, since they modified the assert.rejects API. The unmodified version of the API from #18023 never appeared in a release (see the note at the top of #19650 for more info).

This updates the test in `test/parallel/test-assert-async.js` to add an assertion that the Promises used in the test end up fulfilled. Previously, if an assertion failure occurred, the Promises would have rejected and a warning would have been logged, but the test would still have exit code 0. PR-URL: nodejs#19650 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
This updates `assert.rejects()` to disallow any errors that are thrown synchronously from the given function. Previously, throwing an error would cause the same behavior as returning a rejected Promise. Fixes: nodejs#19646 PR-URL: nodejs#19650 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
@MylesBorins
Copy link
ContributorAuthor

@MylesBorins
Copy link
ContributorAuthor

one more ci... should be working now

https://ci.nodejs.org/job/node-test-pull-request/18309/

@MylesBorins
Copy link
ContributorAuthor

landed in 7f34c27...0d241ba

MylesBorins pushed a commit that referenced this pull request Nov 4, 2018
Implement asynchronous equivalent for assert.throws() and assert.doesNotThrow(). Backport-PR-URL: #24019 PR-URL: #18023 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Shingo Inoue <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 4, 2018
This updates the test in `test/parallel/test-assert-async.js` to add an assertion that the Promises used in the test end up fulfilled. Previously, if an assertion failure occurred, the Promises would have rejected and a warning would have been logged, but the test would still have exit code 0. Backport-PR-URL: #24019 PR-URL: #19650 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 4, 2018
This updates `assert.rejects()` to disallow any errors that are thrown synchronously from the given function. Previously, throwing an error would cause the same behavior as returning a rejected Promise. Fixes: #19646 Backport-PR-URL: #24019 PR-URL: #19650 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

assertIssues and PRs related to the assert subsystem.semver-minorPRs that contain new features and should be released in the next minor version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@MylesBorins@nodejs-github-bot@BridgeAR@not-an-aardvark@feugy