Skip to content

assert: ensure .rejects() disallows sync throws

Rodrigo Muino Tomonari requested to merge github/fork/not-an-aardvark/assert-rejects-sync-error into master
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

This PR has two commits; the first commit fixes an issue with the assert tests, and the second commit fixes https://github.com/nodejs/node/issues/19646.

First commit:

test: ensure failed assertions cause build to fail

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.

Second commit:

assert: ensure .rejects() disallows sync throws

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: https://github.com/nodejs/node/issues/19646

Note that the second commit would be semver-major, but it modifies an API (introduced in https://github.com/nodejs/node/pull/18023) that has not been backported to a release branch yet. As a result, the second commit could be backported as long as it's applied at the same time as https://github.com/nodejs/node/pull/18023.

Merge request reports