assert: adds rejects() and doesNotReject() by feugy · Pull Request #18023 · nodejs/node · GitHub
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions doc/api/assert.md
64 changes: 49 additions & 15 deletions lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -425,14 +425,23 @@ function getActual(block) {
return NO_EXCEPTION_SENTINEL;
}

// Expected to throw an error.
assert.throws = function throws(block, error, message) {
const actual = getActual(block);
async function waitForActual(block) {
if (typeof block !== 'function') {
throw new ERR_INVALID_ARG_TYPE('block', 'Function', block);
}
try {
await block();
} catch (e) {
return e;
}
return NO_EXCEPTION_SENTINEL;
}

function expectsError(stackStartFn, actual, error, message) {
if (typeof error === 'string') {
if (arguments.length === 3)
if (arguments.length === 4) {
throw new ERR_INVALID_ARG_TYPE('error', ['Function', 'RegExp'], error);

}
message = error;
error = null;
}
Expand All @@ -443,21 +452,21 @@ assert.throws = function throws(block, error, message) {
details += ` (${error.name})`;
}
details += message ? `: ${message}` : '.';
const fnType = stackStartFn === rejects ? 'rejection' : 'exception';
innerFail({
actual,
expected: error,
operator: 'throws',
message: `Missing expected exception${details}`,
stackStartFn: throws
operator: stackStartFn.name,
message: `Missing expected ${fnType}${details}`,
stackStartFn
});
}
if (error && expectedException(actual, error, message) === false) {
throw actual;
}
};
}

assert.doesNotThrow = function doesNotThrow(block, error, message) {
const actual = getActual(block);
function expectsNoError(stackStartFn, actual, error, message) {
if (actual === NO_EXCEPTION_SENTINEL)
return;

Expand All @@ -468,16 +477,41 @@ assert.doesNotThrow = function doesNotThrow(block, error, message) {

if (!error || expectedException(actual, error)) {
const details = message ? `: ${message}` : '.';
const fnType = stackStartFn === doesNotReject ? 'rejection' : 'exception';
innerFail({
actual,
expected: error,
operator: 'doesNotThrow',
message: `Got unwanted exception${details}\n${actual && actual.message}`,
stackStartFn: doesNotThrow
operator: stackStartFn.name,
message: `Got unwanted ${fnType}${details}\n${actual && actual.message}`,
stackStartFn
});
}
throw actual;
};
}

function throws(block, ...args) {
expectsError(throws, getActual(block), ...args);
}

assert.throws = throws;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit (non-blocking): I personally prefer the following to save lines:

assert.throws = function throws(block, ...args) {
  expectsError(throws, getActual(block), ...args);
};

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I also prefer function expressions.
But even named, function expression do not benefit from hoisting.

This would imply to use assert.doesNotReject() here, and here.

Same goes for other functions.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good point. We could check for the function name instead but that could theoretically be manipulated.

So just keep it as is. Thanks for the heads up.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hm, while looking at the code again: we actually already use the function name and do not strictly test if it it a "valid" function. So we could indeed just switch to name checking only.


async function rejects(block, ...args) {
expectsError(rejects, await waitForActual(block), ...args);
}

assert.rejects = rejects;

function doesNotThrow(block, ...args) {
expectsNoError(doesNotThrow, getActual(block), ...args);
}

assert.doesNotThrow = doesNotThrow;

async function doesNotReject(block, ...args) {
expectsNoError(doesNotReject, await waitForActual(block), ...args);
}

assert.doesNotReject = doesNotReject;

assert.ifError = function ifError(err) {
if (err !== null && err !== undefined) {
Expand Down
66 changes: 66 additions & 0 deletions test/parallel/test-assert-async.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const { promisify } = require('util');
const wait = promisify(setTimeout);

/* eslint-disable prefer-common-expectserror, no-restricted-properties */

// Test assert.rejects() and assert.doesNotReject() by checking their
// expected output and by verifying that they do not work sync

assert.rejects(
() => assert.fail(),
common.expectsError({
code: 'ERR_ASSERTION',
type: assert.AssertionError,
message: 'Failed'
})
);

assert.doesNotReject(() => {});

{
const promise = assert.rejects(async () => {
await wait(1);
assert.fail();
}, common.expectsError({
code: 'ERR_ASSERTION',
type: assert.AssertionError,
message: 'Failed'
}));
assert.doesNotReject(() => promise);
}

{
const promise = assert.doesNotReject(async () => {
await wait(1);
throw new Error();
});
assert.rejects(() => promise,
(err) => {
assert(err instanceof assert.AssertionError,
`${err.name} is not instance of AssertionError`);
assert.strictEqual(err.code, 'ERR_ASSERTION');
assert(/^Got unwanted rejection\.\n$/.test(err.message));
assert.strictEqual(err.operator, 'doesNotReject');
assert.ok(!err.stack.includes('at Function.doesNotReject'));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Without stack trace cleaning:

AssertionError [ERR_ASSERTION]: Got unwanted rejection.

    at new AssertionError (internal/errors.js:320:11)
    at innerFail (assert.js:74:9)
    at expectsNoError (assert.js:464:5)
    at Function.doesNotReject (assert.js:494:3)
    at <anonymous>

With cleaning:

AssertionError [ERR_ASSERTION]: Got unwanted rejection.

    at <anonymous>

return true;
}
);
}

{
const promise = assert.rejects(() => {});
assert.rejects(() => promise,
(err) => {
assert(err instanceof assert.AssertionError,
`${err.name} is not instance of AssertionError`);
assert.strictEqual(err.code, 'ERR_ASSERTION');
assert(/^Missing expected rejection\.$/.test(err.message));
assert.strictEqual(err.operator, 'rejects');
assert.ok(!err.stack.includes('at Function.rejects'));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Without stack trace cleaning:

AssertionError [ERR_ASSERTION]: Missing expected rejection.
    at new AssertionError (internal/errors.js:320:11)
    at innerFail (assert.js:74:9)
    at expectsError (assert.js:439:5)
    at Function.rejects (assert.js:482:3)
    at <anonymous>

With cleaning:

AssertionError [ERR_ASSERTION]: Missing expected rejection.
    at <anonymous>

return true;
}
);
}
11 changes: 11 additions & 0 deletions test/parallel/test-assert.js