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: multiple improvements #21628

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 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
129 changes: 83 additions & 46 deletions lib/internal/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,18 @@ let green = '';
let red = '';
let white = '';

const READABLE_OPERATOR = {
deepStrictEqual: 'Input A expected to strictly deep-equal input B',
notDeepStrictEqual: 'Input A expected to strictly not deep-equal input B',
strictEqual: 'Input A expected to strictly equal input B',
notStrictEqual: 'Input A expected to strictly not equal input B'
const kReadableOperator = {
deepStrictEqual: 'Expected inputs to be strictly deep-equal:',
notDeepStrictEqual: 'Expected "actual" not to be strictly deep-equal to:',
strictEqual: 'Expected inputs to be strictly equal:',
notStrictEqual: 'Expected "actual" to be strictly unequal to:',
notIdentical: 'Inputs identical but not reference equal:',
};

// Comparing short primitives should just show === / !== instead of using the
// diff.
const kMaxShortLength = 10;

function copyError(source) {
const keys = Object.keys(source);
const target = Object.create(Object.getPrototypeOf(source));
Expand Down Expand Up @@ -49,22 +54,58 @@ function inspectValue(val) {
}

function createErrDiff(actual, expected, operator) {
var other = '';
var res = '';
var lastPos = 0;
var end = '';
var skipped = false;
let other = '';
let res = '';
let lastPos = 0;
let end = '';
let skipped = false;
const actualLines = inspectValue(actual);
const expectedLines = inspectValue(expected);
const msg = READABLE_OPERATOR[operator] +
`:\n${green}+ expected${white} ${red}- actual${white}`;
const msg = kReadableOperator[operator] +
`\n${green}+ actual${white} ${red}- expected${white}`;
const skippedMsg = ` ${blue}...${white} Lines skipped`;

let i = 0;
let indicator = '';

// If "actual" and "expected" fit on a single line and they are not strictly
// equal, check further special handling.
if (actualLines.length === 1 && expectedLines.length === 1 &&
actualLines[0] !== expectedLines[0]) {
const inputLength = actualLines[0].length + expectedLines[0].length;
// If the character length of "actual" and "expected" together is less than
// kMaxShortLength and if neither is an object and at least one of them is
// not `zero`, use the strict equal comparison to visualize the output.
if (inputLength <= kMaxShortLength) {
if ((typeof actual !== 'object' || actual === null) &&
(typeof expected !== 'object' || expected === null) &&
(actual !== 0 || expected !== 0)) { // -0 === +0
return `${kReadableOperator[operator]}\n\n` +
`${actualLines[0]} !== ${expectedLines[0]}\n`;
}
Copy link
Member

Choose a reason for hiding this comment

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

Can you please add a comment explaining this block of logic?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

} else {
// If the stdout is a tty and the input length is lower than the current
// columns per line, add a mismatch indicator below the output. If it is
// not a tty, use a default value of 80 characters.
const maxLength = process.stdout.isTTY ? process.stdout.columns : 80;
if (inputLength < maxLength) {
while (actualLines[0][i] === expectedLines[0][i]) {
i++;
}
if (i !== 0) {
// Add position indicator for the first mismatch in case it is a
// single line and the input length is less than the column length.
indicator = `\n ${' '.repeat(i)}^`;
i = 0;
}
}
}
}

// Remove all ending lines that match (this optimizes the output for
// readability by reducing the number of total changed lines).
var a = actualLines[actualLines.length - 1];
var b = expectedLines[expectedLines.length - 1];
var i = 0;
let a = actualLines[actualLines.length - 1];
let b = expectedLines[expectedLines.length - 1];
while (a === b) {
if (i++ < 2) {
end = `\n ${a}${end}`;
Expand All @@ -78,6 +119,26 @@ function createErrDiff(actual, expected, operator) {
a = actualLines[actualLines.length - 1];
b = expectedLines[expectedLines.length - 1];
}

const maxLines = Math.max(actualLines.length, expectedLines.length);
// Strict equal with identical objects that are not identical by reference.
// E.g., assert.deepStrictEqual({ a: Symbol() }, { a: Symbol() })
if (maxLines === 0) {
// We have to get the result again. The lines were all removed before.
const actualLines = inspectValue(actual);

// Only remove lines in case it makes sense to collapse those.
// TODO: Accept env to always show the full error.
if (actualLines.length > 30) {
actualLines[26] = `${blue}...${white}`;
while (actualLines.length > 27) {
actualLines.pop();
}
}

return `${kReadableOperator.notIdentical}\n\n${actualLines.join('\n')}\n`;
}

if (i > 3) {
end = `\n${blue}...${white}${end}`;
skipped = true;
Expand All @@ -87,9 +148,7 @@ function createErrDiff(actual, expected, operator) {
other = '';
}

const maxLines = Math.max(actualLines.length, expectedLines.length);
var printedLines = 0;
var identical = 0;
let printedLines = 0;
for (i = 0; i < maxLines; i++) {
// Only extra expected lines exist
const cur = i - lastPos;
Expand All @@ -106,7 +165,7 @@ function createErrDiff(actual, expected, operator) {
printedLines++;
}
lastPos = i;
other += `\n${green}+${white} ${expectedLines[i]}`;
other += `\n${red}-${white} ${expectedLines[i]}`;
printedLines++;
// Only extra actual lines exist
} else if (expectedLines.length < i + 1) {
Expand All @@ -122,7 +181,7 @@ function createErrDiff(actual, expected, operator) {
printedLines++;
}
lastPos = i;
res += `\n${red}-${white} ${actualLines[i]}`;
res += `\n${green}+${white} ${actualLines[i]}`;
printedLines++;
// Lines diverge
} else if (actualLines[i] !== expectedLines[i]) {
Expand All @@ -138,8 +197,8 @@ function createErrDiff(actual, expected, operator) {
printedLines++;
}
lastPos = i;
res += `\n${red}-${white} ${actualLines[i]}`;
other += `\n${green}+${white} ${expectedLines[i]}`;
res += `\n${green}+${white} ${actualLines[i]}`;
other += `\n${red}-${white} ${expectedLines[i]}`;
printedLines += 2;
// Lines are identical
} else {
Expand All @@ -149,7 +208,6 @@ function createErrDiff(actual, expected, operator) {
res += `\n ${actualLines[i]}`;
printedLines++;
}
identical++;
}
// Inspected object to big (Show ~20 rows max)
if (printedLines > 20 && i < maxLines - 2) {
Expand All @@ -158,28 +216,7 @@ function createErrDiff(actual, expected, operator) {
}
}

// Strict equal with identical objects that are not identical by reference.
if (identical === maxLines) {
// E.g., assert.deepStrictEqual(Symbol(), Symbol())
const base = operator === 'strictEqual' ?
'Input objects identical but not reference equal:' :
'Input objects not identical:';

// We have to get the result again. The lines were all removed before.
const actualLines = inspectValue(actual);

// Only remove lines in case it makes sense to collapse those.
// TODO: Accept env to always show the full error.
if (actualLines.length > 30) {
actualLines[26] = `${blue}...${white}`;
while (actualLines.length > 27) {
actualLines.pop();
}
}

return `${base}\n\n${actualLines.join('\n')}\n`;
}
return `${msg}${skipped ? skippedMsg : ''}\n${res}${other}${end}`;
return `${msg}${skipped ? skippedMsg : ''}\n${res}${other}${end}${indicator}`;
}

class AssertionError extends Error {
Expand Down Expand Up @@ -231,7 +268,7 @@ class AssertionError extends Error {
// In case the objects are equal but the operator requires unequal, show
// the first object and say A equals B
const res = inspectValue(actual);
const base = `Identical input passed to ${operator}:`;
const base = kReadableOperator[operator];

// Only remove lines in case it makes sense to collapse those.
// TODO: Accept env to always show the full error.
Expand Down
12 changes: 6 additions & 6 deletions test/message/assert_throws_stack.out
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ assert.js:*
throw err;
^

AssertionError [ERR_ASSERTION]: Input A expected to strictly deep-equal input B:
+ expected - actual
AssertionError [ERR_ASSERTION]: Expected inputs to be strictly deep-equal:
+ actual - expected

- Comparison {}
+ Comparison {
+ bar: true
+ }
+ Comparison {}
- Comparison {
- bar: true
- }
at Object.<anonymous> (*assert_throws_stack.js:*:*)
at *
at *
Expand Down
2 changes: 1 addition & 1 deletion test/message/core_line_numbers.out
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ punycode.js:42
^

RangeError: Invalid input
at error (punycode.js:42:*)
at error (punycode.js:42:8)
at Object.decode (punycode.js:*:*)
at Object.<anonymous> (*test*message*core_line_numbers.js:*:*)
at Module._compile (internal/modules/cjs/loader.js:*:*)
Expand Down
7 changes: 3 additions & 4 deletions test/message/error_exit.out
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@ assert.js:*
throw new AssertionError(obj);
^

AssertionError [ERR_ASSERTION]: Input A expected to strictly equal input B:
+ expected - actual
AssertionError [ERR_ASSERTION]: Expected inputs to be strictly equal:

1 !== 2

- 1
+ 2
at Object.<anonymous> (*test*message*error_exit.js:*:*)
at Module._compile (internal/modules/cjs/loader.js:*:*)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*:*)
Expand Down
8 changes: 4 additions & 4 deletions test/parallel/test-assert-checktag.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@ if (process.stdout.isTTY)
assert.throws(
() => assert.deepStrictEqual(date, fake),
{
message: 'Input A expected to strictly deep-equal input B:\n' +
'+ expected - actual\n\n- 2016-01-01T00:00:00.000Z\n+ Date {}'
message: 'Expected inputs to be strictly deep-equal:\n' +
'+ actual - expected\n\n+ 2016-01-01T00:00:00.000Z\n- Date {}'
}
);
assert.throws(
() => assert.deepStrictEqual(fake, date),
{
message: 'Input A expected to strictly deep-equal input B:\n' +
'+ expected - actual\n\n- Date {}\n+ 2016-01-01T00:00:00.000Z'
message: 'Expected inputs to be strictly deep-equal:\n' +
'+ actual - expected\n\n+ Date {}\n- 2016-01-01T00:00:00.000Z'
}
);
}
Expand Down
Loading