assert: improve assert.throws
Throw a TypeError in case a error message is provided in the second argument and a third argument is present as well. This is clearly a mistake and should not be done. PR-URL: https://github.com/nodejs/node/pull/17585 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Evan Lucas <evanlucas@me.com>
This commit is contained in:
parent
dc2e266647
commit
da5c7d68cd
@ -767,17 +767,42 @@ assert.throws(
|
|||||||
|
|
||||||
Note that `error` can not be a string. If a string is provided as the second
|
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
|
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:
|
`message` instead. This can lead to easy-to-miss mistakes. Please read the
|
||||||
|
example below carefully if using a string as the second argument gets
|
||||||
|
considered:
|
||||||
|
|
||||||
<!-- eslint-disable no-restricted-syntax -->
|
<!-- eslint-disable no-restricted-syntax -->
|
||||||
```js
|
```js
|
||||||
// THIS IS A MISTAKE! DO NOT DO THIS!
|
function throwingFirst() {
|
||||||
assert.throws(myFunction, 'missing foo', 'did not throw with expected message');
|
throw new Error('First');
|
||||||
|
}
|
||||||
|
function throwingSecond() {
|
||||||
|
throw new Error('Second');
|
||||||
|
}
|
||||||
|
function notThrowing() {}
|
||||||
|
|
||||||
// Do this instead.
|
// The second argument is a string and the input function threw an Error.
|
||||||
assert.throws(myFunction, /missing foo/, 'did not throw with expected message');
|
// In that case both cases do not throw as neither is going to try to
|
||||||
|
// match for the error message thrown by the input function!
|
||||||
|
assert.throws(throwingFirst, 'Second');
|
||||||
|
assert.throws(throwingSecond, 'Second');
|
||||||
|
|
||||||
|
// The string is only used (as message) in case the function does not throw:
|
||||||
|
assert.throws(notThrowing, 'Second');
|
||||||
|
// AssertionError [ERR_ASSERTION]: Missing expected exception: Second
|
||||||
|
|
||||||
|
// If it was intended to match for the error message do this instead:
|
||||||
|
assert.throws(throwingSecond, /Second$/);
|
||||||
|
// Does not throw because the error messages match.
|
||||||
|
assert.throws(throwingFirst, /Second$/);
|
||||||
|
// Throws a error:
|
||||||
|
// Error: First
|
||||||
|
// at throwingFirst (repl:2:9)
|
||||||
```
|
```
|
||||||
|
|
||||||
|
Due to the confusing notation, it is recommended not to use a string as the
|
||||||
|
second argument. This might lead to difficult-to-spot errors.
|
||||||
|
|
||||||
[`Error.captureStackTrace`]: errors.html#errors_error_capturestacktrace_targetobject_constructoropt
|
[`Error.captureStackTrace`]: errors.html#errors_error_capturestacktrace_targetobject_constructoropt
|
||||||
[`Map`]: https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Map
|
[`Map`]: https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Map
|
||||||
[`Object.is()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/is
|
[`Object.is()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/is
|
||||||
|
@ -208,7 +208,11 @@ function expectedException(actual, expected) {
|
|||||||
return expected.call({}, actual) === true;
|
return expected.call({}, actual) === true;
|
||||||
}
|
}
|
||||||
|
|
||||||
function tryBlock(block) {
|
function getActual(block) {
|
||||||
|
if (typeof block !== 'function') {
|
||||||
|
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'block', 'Function',
|
||||||
|
block);
|
||||||
|
}
|
||||||
try {
|
try {
|
||||||
block();
|
block();
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
@ -216,60 +220,61 @@ function tryBlock(block) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
function innerThrows(shouldThrow, block, expected, message) {
|
// Expected to throw an error.
|
||||||
var details = '';
|
assert.throws = function throws(block, error, message) {
|
||||||
|
const actual = getActual(block);
|
||||||
|
|
||||||
if (typeof block !== 'function') {
|
if (typeof error === 'string') {
|
||||||
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'block', 'Function',
|
if (arguments.length === 3)
|
||||||
block);
|
throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
|
||||||
|
'error',
|
||||||
|
['Function', 'RegExp'],
|
||||||
|
error);
|
||||||
|
|
||||||
|
message = error;
|
||||||
|
error = null;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (typeof expected === 'string') {
|
|
||||||
message = expected;
|
|
||||||
expected = null;
|
|
||||||
}
|
|
||||||
|
|
||||||
const actual = tryBlock(block);
|
|
||||||
|
|
||||||
if (shouldThrow === true) {
|
|
||||||
if (actual === undefined) {
|
if (actual === undefined) {
|
||||||
if (expected && expected.name) {
|
let details = '';
|
||||||
details += ` (${expected.name})`;
|
if (error && error.name) {
|
||||||
|
details += ` (${error.name})`;
|
||||||
}
|
}
|
||||||
details += message ? `: ${message}` : '.';
|
details += message ? `: ${message}` : '.';
|
||||||
innerFail({
|
innerFail({
|
||||||
actual,
|
actual,
|
||||||
expected,
|
expected: error,
|
||||||
operator: 'throws',
|
operator: 'throws',
|
||||||
message: `Missing expected exception${details}`,
|
message: `Missing expected exception${details}`,
|
||||||
stackStartFn: assert.throws
|
stackStartFn: throws
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
if (expected && expectedException(actual, expected) === false) {
|
if (error && expectedException(actual, error) === false) {
|
||||||
throw actual;
|
throw actual;
|
||||||
}
|
}
|
||||||
} else if (actual !== undefined) {
|
|
||||||
if (!expected || expectedException(actual, expected)) {
|
|
||||||
details = message ? `: ${message}` : '.';
|
|
||||||
innerFail({
|
|
||||||
actual,
|
|
||||||
expected,
|
|
||||||
operator: 'doesNotThrow',
|
|
||||||
message: `Got unwanted exception${details}\n${actual.message}`,
|
|
||||||
stackStartFn: assert.doesNotThrow
|
|
||||||
});
|
|
||||||
}
|
|
||||||
throw actual;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Expected to throw an error.
|
|
||||||
assert.throws = function throws(block, error, message) {
|
|
||||||
innerThrows(true, block, error, message);
|
|
||||||
};
|
};
|
||||||
|
|
||||||
assert.doesNotThrow = function doesNotThrow(block, error, message) {
|
assert.doesNotThrow = function doesNotThrow(block, error, message) {
|
||||||
innerThrows(false, block, error, message);
|
const actual = getActual(block);
|
||||||
|
if (actual === undefined)
|
||||||
|
return;
|
||||||
|
|
||||||
|
if (typeof error === 'string') {
|
||||||
|
message = error;
|
||||||
|
error = null;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!error || expectedException(actual, error)) {
|
||||||
|
const details = message ? `: ${message}` : '.';
|
||||||
|
innerFail({
|
||||||
|
actual,
|
||||||
|
expected: error,
|
||||||
|
operator: 'doesNotThrow',
|
||||||
|
message: `Got unwanted exception${details}\n${actual.message}`,
|
||||||
|
stackStartFn: doesNotThrow
|
||||||
|
});
|
||||||
|
}
|
||||||
|
throw actual;
|
||||||
};
|
};
|
||||||
|
|
||||||
assert.ifError = function ifError(err) { if (err) throw err; };
|
assert.ifError = function ifError(err) { if (err) throw err; };
|
||||||
|
@ -773,3 +773,14 @@ common.expectsError(
|
|||||||
message: 'null == true'
|
message: 'null == true'
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
|
|
||||||
|
common.expectsError(
|
||||||
|
// eslint-disable-next-line no-restricted-syntax
|
||||||
|
() => assert.throws(() => {}, 'Error message', 'message'),
|
||||||
|
{
|
||||||
|
code: 'ERR_INVALID_ARG_TYPE',
|
||||||
|
type: TypeError,
|
||||||
|
message: 'The "error" argument must be one of type Function or RegExp. ' +
|
||||||
|
'Received type string'
|
||||||
|
}
|
||||||
|
);
|
||||||
|
Loading…
x
Reference in New Issue
Block a user