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: use error diffs in throws messages #19463

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
53 changes: 38 additions & 15 deletions lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -365,13 +365,38 @@ assert.notStrictEqual = function notStrictEqual(actual, expected, message) {
}
};

function compareExceptionKey(actual, expected, key, msg) {
if (!isDeepStrictEqual(actual[key], expected[key])) {
class Comparison {
constructor(obj, keys) {
for (const key of keys) {
if (key in obj)
this[key] = obj[key];
}
}
}

function compareExceptionKey(actual, expected, key, message, keys) {
if (!(key in actual) || !isDeepStrictEqual(actual[key], expected[key])) {
if (!message) {
// Create placeholder objects to create a nice output.
const a = new Comparison(actual, keys);
const b = new Comparison(expected, keys);

const tmpLimit = Error.stackTraceLimit;
Error.stackTraceLimit = 0;
const err = new AssertionError({
actual: a,
expected: b,
operator: 'deepStrictEqual',
stackStartFn: assert.throws,
errorDiff: ERR_DIFF_EQUAL
});
Error.stackTraceLimit = tmpLimit;
message = err.message;
}
innerFail({
actual: actual[key],
expected: expected[key],
message: msg || `${key}: expected ${inspect(expected[key])}, ` +
`not ${inspect(actual[key])}`,
actual,
expected,
message,
operator: 'throws',
stackStartFn: assert.throws
});
Expand All @@ -388,16 +413,14 @@ function expectedException(actual, expected, msg) {
'expected', ['Function', 'RegExp'], expected
);
}
// The name and message could be non enumerable. Therefore test them
// explicitly.
if ('name' in expected) {
compareExceptionKey(actual, expected, 'name', msg);
}
if ('message' in expected) {
compareExceptionKey(actual, expected, 'message', msg);
const keys = Object.keys(expected);
// Special handle errors to make sure the name and the message are compared
// as well.
if (expected instanceof Error) {
keys.push('name', 'message');
}
for (const key of Object.keys(expected)) {
compareExceptionKey(actual, expected, key, msg);
for (const key of keys) {
compareExceptionKey(actual, expected, key, msg, keys);
}
return true;
}
Expand Down
8 changes: 7 additions & 1 deletion test/message/assert_throws_stack.out
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@ assert.js:*
throw new AssertionError(obj);
^

AssertionError [ERR_ASSERTION]: bar: expected true, not undefined
AssertionError [ERR_ASSERTION]: Input A expected to deepStrictEqual input B:
+ expected - actual

- Comparison {}
+ Comparison {
+ bar: true
+ }
at Object.<anonymous> (*assert_throws_stack.js:*:*)
at *
at *
Expand Down
6 changes: 1 addition & 5 deletions test/parallel/test-assert-fail-deprecation.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,7 @@ assert.throws(() => {
assert.fail(typeof 1, 'object', new TypeError('another custom message'));
}, {
name: 'TypeError',
message: 'another custom message',
operator: undefined,
actual: undefined,
expected: undefined,
code: undefined
message: 'another custom message'
});

// No third arg (but a fourth arg)
Expand Down
5 changes: 1 addition & 4 deletions test/parallel/test-assert-fail.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,5 @@ assert.throws(() => {
assert.fail(new TypeError('custom message'));
}, {
name: 'TypeError',
message: 'custom message',
operator: undefined,
actual: undefined,
expected: undefined
message: 'custom message'
});
53 changes: 33 additions & 20 deletions test/parallel/test-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ const { writeFileSync, unlinkSync } = require('fs');
const { inspect } = require('util');
const a = assert;

const colors = process.stdout.isTTY && process.stdout.getColorDepth() > 1;
const start = 'Input A expected to deepStrictEqual input B:';
const actExp = colors ?
'\u001b[32m+ expected\u001b[39m \u001b[31m- actual\u001b[39m' :
'+ expected - actual';

assert.ok(a.AssertionError.prototype instanceof Error,
'a.AssertionError instanceof Error');

Expand Down Expand Up @@ -440,11 +446,6 @@ common.expectsError(
Error.stackTraceLimit = tmpLimit;

// Test error diffs.
const colors = process.stdout.isTTY && process.stdout.getColorDepth() > 1;
const start = 'Input A expected to deepStrictEqual input B:';
const actExp = colors ?
'\u001b[32m+ expected\u001b[39m \u001b[31m- actual\u001b[39m' :
'+ expected - actual';
const plus = colors ? '\u001b[32m+\u001b[39m' : '+';
const minus = colors ? '\u001b[31m-\u001b[39m' : '-';
let message = [
Expand Down Expand Up @@ -769,24 +770,32 @@ common.expectsError(
errObj.code = 404;
assert.throws(errFn, errObj);

errObj.code = '404';
common.expectsError(
// Fail in case a expected property is undefined and not existent on the
// error.
errObj.foo = undefined;
assert.throws(
() => assert.throws(errFn, errObj),
{
code: 'ERR_ASSERTION',
type: assert.AssertionError,
message: 'code: expected \'404\', not 404'
name: 'AssertionError [ERR_ASSERTION]',
message: `${start}\n${actExp}\n\n` +
" Comparison {\n name: 'TypeError',\n" +
" message: 'Wrong value',\n- code: 404\n" +
'+ code: 404,\n+ foo: undefined\n }'
}
);

errObj.code = 404;
errObj.foo = 'bar';
common.expectsError(
// Show multiple wrong properties at the same time.
errObj.code = '404';
assert.throws(
() => assert.throws(errFn, errObj),
{
code: 'ERR_ASSERTION',
type: assert.AssertionError,
message: 'foo: expected \'bar\', not undefined'
name: 'AssertionError [ERR_ASSERTION]',
message: `${start}\n${actExp}\n\n` +
" Comparison {\n name: 'TypeError',\n" +
" message: 'Wrong value',\n- code: 404\n" +
"+ code: '404',\n+ foo: undefined\n }"
}
);

Expand All @@ -810,20 +819,24 @@ common.expectsError(
);

assert.throws(() => { throw new Error('e'); }, new Error('e'));
common.expectsError(
assert.throws(
() => assert.throws(() => { throw new TypeError('e'); }, new Error('e')),
{
type: assert.AssertionError,
name: 'AssertionError [ERR_ASSERTION]',
code: 'ERR_ASSERTION',
message: "name: expected 'Error', not 'TypeError'"
message: `${start}\n${actExp}\n\n` +
" Comparison {\n- name: 'TypeError',\n+ name: 'Error'," +
"\n message: 'e'\n }"
}
);
common.expectsError(
assert.throws(
() => assert.throws(() => { throw new Error('foo'); }, new Error('')),
{
type: assert.AssertionError,
name: 'AssertionError [ERR_ASSERTION]',
code: 'ERR_ASSERTION',
message: "message: expected '', not 'foo'"
message: `${start}\n${actExp}\n\n` +
" Comparison {\n name: 'Error',\n- message: 'foo'" +
"\n+ message: ''\n }"
}
);

Expand Down