Skip to content

assert: improve error messages #19467

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.

Sign up for GitHub

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

Closed
BridgeAR wants to merge 3 commits into from
Closed

assert: improve error messages #19467

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d808ca2
util: inspect arguments properly
BridgeAR Mar 15, 2018
87a3657
assert: improve default error messages
BridgeAR Mar 12, 2018
611ddd2
assert: remove `errorDiff` property
BridgeAR Apr 2, 2018
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
Prev Previous commit
assert: remove errorDiff property 
The property is not necessary as it is possible to check for the
operator instead.
  • Loading branch information
@BridgeAR
BridgeAR committed Apr 13, 2018
commit 611ddd2698b10fcb93177c2ed06929f963085943
18 changes: 5 additions & 13 deletions lib/assert.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,6 @@ const meta = [

const escapeFn = (str) => meta[str.charCodeAt(0)];

const ERR_DIFF_NOT_EQUAL = 1;
const ERR_DIFF_EQUAL = 2;

let warned = false;

// The assert module provides functions that throw
Expand Down Expand Up @@ -321,8 +318,7 @@ assert.deepStrictEqual = function deepStrictEqual(actual, expected, message) {
expected,
message,
operator: 'deepStrictEqual',
stackStartFn: deepStrictEqual,
errorDiff: ERR_DIFF_EQUAL
stackStartFn: deepStrictEqual
});
}
};
Expand All @@ -335,8 +331,7 @@ function notDeepStrictEqual(actual, expected, message) {
expected,
message,
operator: 'notDeepStrictEqual',
stackStartFn: notDeepStrictEqual,
errorDiff: ERR_DIFF_NOT_EQUAL
stackStartFn: notDeepStrictEqual
});
}
}
Expand All @@ -348,8 +343,7 @@ assert.strictEqual = function strictEqual(actual, expected, message) {
expected,
message,
operator: 'strictEqual',
stackStartFn: strictEqual,
errorDiff: ERR_DIFF_EQUAL
stackStartFn: strictEqual
});
}
};
Expand All @@ -361,8 +355,7 @@ assert.notStrictEqual = function notStrictEqual(actual, expected, message) {
expected,
message,
operator: 'notStrictEqual',
stackStartFn: notStrictEqual,
errorDiff: ERR_DIFF_NOT_EQUAL
stackStartFn: notStrictEqual
});
}
};
Expand All @@ -389,8 +382,7 @@ function compareExceptionKey(actual, expected, key, message, keys) {
actual: a,
expected: b,
operator: 'deepStrictEqual',
stackStartFn: assert.throws,
errorDiff: ERR_DIFF_EQUAL
stackStartFn: assert.throws
});
Error.stackTraceLimit = tmpLimit;
message = err.message;
Expand Down
24 changes: 12 additions & 12 deletions lib/internal/errors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,8 +395,7 @@ class AssertionError extends Error {
expected,
message,
operator,
stackStartFn,
errorDiff = 0
stackStartFn
} = options;

if (message != null) {
Expand Down Expand Up @@ -427,15 +426,10 @@ class AssertionError extends Error {
expected = copyError(expected);
}
Copy link
Copy Markdown
Member Author

@BridgeAR BridgeAR Apr 2, 2018

Choose a reason for hiding this comment

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

I think it is best to actually show the stack trace in the comparison in case there is only a single error. That way it is clearer in what way the input is different from each other.


if (errorDiff === 0) {
let res = util.inspect(actual);
let other = util.inspect(expected);
if (res.length > 128)
res = `${res.slice(0, 125)}...`;
if (other.length > 128)
other = `${other.slice(0, 125)}...`;
super(`${res} ${operator} ${other}`);
} else if (errorDiff === 1) {
if (operator === 'deepStrictEqual' || operator === 'strictEqual') {
super(createErrDiff(actual, expected, operator));
} else if (operator === 'notDeepStrictEqual' ||
operator === 'notStrictEqual') {
// In case the objects are equal but the operator requires unequal, show
// the first object and say A equals B
const res = inspectValue(actual);
Expand All @@ -457,7 +451,13 @@ class AssertionError extends Error {
super(`${base}\n\n ${res.join('\n ')}\n`);
}
} else {
super(createErrDiff(actual, expected, operator));
let res = util.inspect(actual);
let other = util.inspect(expected);
if (res.length > 128)
res = `${res.slice(0, 125)}...`;
if (other.length > 128)
other = `${other.slice(0, 125)}...`;
super(`${res} ${operator} ${other}`);
}
}

Expand Down