Skip to content

Commit

Permalink
assert: restore TypeError if no arguments
Browse files Browse the repository at this point in the history
In Node 7.x, calling `throw new assert.AssertionError()` resulted in a
TypeError.

In current master, the same call does not result in an error but, due to
the default option, it results in uninformative output ("undefined
undefined undefined").

This change removes the default argument, restoring a TypeError if there
is no argument. This also will restore our test coverage to 100%. (The
default argument is not tested in our current test suite.)

PR-URL: #12843
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
Trott committed May 8, 2017
1 parent ea1b8a5 commit f6247a9
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 10 deletions.
2 changes: 1 addition & 1 deletion lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ const assert = module.exports = ok;

// TODO(jasnell): Consider moving AssertionError into internal/errors.js
class AssertionError extends Error {
constructor(options = {}) {
constructor(options) {
if (typeof options !== 'object' || options === null) {
// Lazy because the errors module itself uses assertions, leading to
// a circular dependency. This can be eliminated by moving this class
Expand Down
22 changes: 13 additions & 9 deletions test/parallel/test-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -707,12 +707,16 @@ assert.throws(() => {
code: 'ERR_ASSERTION',
message: new RegExp(`^'${'A'.repeat(127)} === ''$`)}));

[1, true, false, '', null, Infinity, Symbol('test')].forEach((input) => {
assert.throws(
() => new assert.AssertionError(input),
common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: /^The "options" argument must be of type object$/
}));
});
{
// bad args to AssertionError constructor should throw TypeError
const args = [1, true, false, '', null, Infinity, Symbol('test'), undefined];
args.forEach((input) => {
assert.throws(
() => new assert.AssertionError(input),
common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: /^The "options" argument must be of type object$/
}));
});
}

0 comments on commit f6247a9

Please sign in to comment.