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-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-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]>
@MylesBorins MylesBorins force-pushed the backport-assert-rejects branch from 9822e10 to 4ee80bc Compare November 1, 2018 17:23
@MylesBorins MylesBorins requested a review from BridgeAR November 1, 2018 19:06
@MylesBorins MylesBorins 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

@BridgeAR BridgeAR 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

BridgeAR commented Nov 1, 2018

@MylesBorins thanks a lot! 👍

@MylesBorins
Copy link
Contributor Author

@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
Contributor Author

@MylesBorins
Copy link
Contributor Author

one more ci... should be working now

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

@MylesBorins
Copy link
Contributor Author

landed in 7f34c27...0d241ba

@MylesBorins MylesBorins closed this Nov 4, 2018
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 free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assert Issues and PRs related to the assert subsystem. semver-minor PRs 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