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

assert: .throws accept objects #17584

Closed
wants to merge 1 commit into from
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
26 changes: 23 additions & 3 deletions doc/api/assert.md
Original file line number Diff line number Diff line change
Expand Up @@ -711,18 +711,21 @@ instead of the `AssertionError`.
<!-- YAML
added: v0.1.21
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/REPLACEME
description: The `error` parameter can now be an object as well.
- version: v4.2.0
pr-url: https://github.com/nodejs/node/pull/3276
description: The `error` parameter can now be an arrow function.
-->
* `block` {Function}
* `error` {RegExp|Function}
* `error` {RegExp|Function|object}
* `message` {any}

Expects the function `block` to throw an error.

If specified, `error` can be a constructor, [`RegExp`][], or validation
function.
If specified, `error` can be a constructor, [`RegExp`][], a validation
function, or an object where each property will be tested for.

If specified, `message` will be the message provided by the `AssertionError` if
the block fails to throw.
Expand Down Expand Up @@ -768,6 +771,23 @@ assert.throws(
);
```

Custom error object / error instance:

```js
assert.throws(
() => {
const err = new TypeError('Wrong value');
err.code = 404;
throw err;
},
{
name: 'TypeError',
message: 'Wrong value'
// Note that only properties on the error object will be tested!
}
);
```

Note that `error` can not be a string. If a string is provided as the second
argument, then `error` is assumed to be omitted and the string will be used for
`message` instead. This can lead to easy-to-miss mistakes. Please read the
Expand Down
43 changes: 39 additions & 4 deletions lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
const { isDeepEqual, isDeepStrictEqual } =
require('internal/util/comparisons');
const errors = require('internal/errors');
const { inspect } = require('util');

// The assert module provides functions that throw
// AssertionError's when particular conditions are not met. The
Expand Down Expand Up @@ -193,10 +194,44 @@ assert.notStrictEqual = function notStrictEqual(actual, expected, message) {
}
};

function expectedException(actual, expected) {
function createMsg(msg, key, actual, expected) {
if (msg)
return msg;
return `${key}: expected ${inspect(expected[key])}, ` +
`not ${inspect(actual[key])}`;
}

function expectedException(actual, expected, msg) {
if (typeof expected !== 'function') {
// Should be a RegExp, if not fail hard
return expected.test(actual);
if (expected instanceof RegExp)
return expected.test(actual);
// assert.doesNotThrow does not accept objects.
if (arguments.length === 2) {
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'expected',
['Function', 'RegExp'], expected);
}
// The name and message could be non enumerable. Therefore test them
// explicitly.
if ('name' in expected) {
assert.strictEqual(
actual.name,
expected.name,
createMsg(msg, 'name', actual, expected));
}
if ('message' in expected) {
assert.strictEqual(
actual.message,
expected.message,
createMsg(msg, 'message', actual, expected));
}
const keys = Object.keys(expected);
for (const key of keys) {
Copy link
Contributor

Choose a reason for hiding this comment

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

name and message have already been tested for, so perhaps they should be explicitly not tested for again? I imagine that testing for them would be a very common scenario, in which case not running deepStrictEqual will probably run a bit faster. Thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess the performance will be exactly the same. deepStrictEqual will only do

// Already existing
function equal(a, b) { if (a === b) return true }

// This check is executed
if (equal(a, b) !== true) throw new Error(msg)

assert.deepStrictEqual(
actual[key],
expected[key],
createMsg(msg, key, actual, expected));
}
return true;
}
// Guard instanceof against arrow functions as they don't have a prototype.
if (expected.prototype !== undefined && actual instanceof expected) {
Expand Down Expand Up @@ -249,7 +284,7 @@ assert.throws = function throws(block, error, message) {
stackStartFn: throws
});
}
if (error && expectedException(actual, error) === false) {
if (error && expectedException(actual, error, message) === false) {
throw actual;
}
};
Expand Down
86 changes: 76 additions & 10 deletions test/parallel/test-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,6 @@ assert.ok(a.AssertionError.prototype instanceof Error,

assert.throws(makeBlock(a, false), a.AssertionError, 'ok(false)');

// Using a object as second arg results in a failure
assert.throws(
() => { assert.throws(() => { throw new Error(); }, { foo: 'bar' }); },
common.expectsError({
type: TypeError,
message: 'expected.test is not a function'
})
);


assert.doesNotThrow(makeBlock(a, true), a.AssertionError, 'ok(true)');

assert.doesNotThrow(makeBlock(a, 'test', 'ok(\'test\')'));
Expand Down Expand Up @@ -784,3 +774,79 @@ common.expectsError(
'Received type string'
}
);

{
const errFn = () => {
const err = new TypeError('Wrong value');
err.code = 404;
throw err;
};
const errObj = {
name: 'TypeError',
message: 'Wrong value'
};
assert.throws(errFn, errObj);

errObj.code = 404;
assert.throws(errFn, errObj);

errObj.code = '404';
common.expectsError(
// eslint-disable-next-line no-restricted-syntax
() => assert.throws(errFn, errObj),
{
code: 'ERR_ASSERTION',
type: assert.AssertionError,
message: 'code: expected \'404\', not 404'
}
);

errObj.code = 404;
errObj.foo = 'bar';
common.expectsError(
// eslint-disable-next-line no-restricted-syntax
() => assert.throws(errFn, errObj),
{
code: 'ERR_ASSERTION',
type: assert.AssertionError,
message: 'foo: expected \'bar\', not undefined'
}
);

common.expectsError(
() => assert.throws(() => { throw new Error(); }, { foo: 'bar' }, 'foobar'),
{
type: assert.AssertionError,
code: 'ERR_ASSERTION',
message: 'foobar'
}
);

common.expectsError(
() => assert.doesNotThrow(() => { throw new Error(); }, { foo: 'bar' }),
{
type: TypeError,
code: 'ERR_INVALID_ARG_TYPE',
message: 'The "expected" argument must be one of type Function or ' +
'RegExp. Received type object'
}
);

assert.throws(() => { throw new Error('e'); }, new Error('e'));
common.expectsError(
() => assert.throws(() => { throw new TypeError('e'); }, new Error('e')),
{
type: assert.AssertionError,
code: 'ERR_ASSERTION',
message: "name: expected 'Error', not 'TypeError'"
}
);
common.expectsError(
() => assert.throws(() => { throw new Error('foo'); }, new Error('')),
{
type: assert.AssertionError,
code: 'ERR_ASSERTION',
message: "message: expected '', not 'foo'"
}
);
}