Skip to content

Commit

Permalink
assert: fix error diff
Browse files Browse the repository at this point in the history 
In some edge cases an identical line could be printed twice. This is
now fixed by changing the algorithm a bit. It will now verify how
many lines were identical before the current one.

PR-URL: nodejs#28058
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
@BridgeAR @Trott
BridgeAR authored and Trott committed Jun 13, 2019
1 parent a3ea54f commit e4ec4f2
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 44 deletions.
66 changes: 28 additions & 38 deletions lib/internal/assert/assertion_error.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,25 +52,18 @@ function inspectValue(val) {
maxArrayLength: Infinity,
// Assert compares only enumerable properties (with a few exceptions).
showHidden: false,
// Having a long line as error is better than wrapping the line for
// comparison for now.
// TODO(BridgeAR): `breakLength` should be limited as soon as soon as we
// have meta information about the inspected properties (i.e., know where
// in what line the property starts and ends).
breakLength: Infinity,
// Assert does not detect proxies currently.
showProxy: false,
sorted: true,
// Inspect getters as we also check them when comparing entries.
getters: true
getters: true,
}
);
}

function createErrDiff(actual, expected, operator) {
let other = '';
let res = '';
let lastPos = 0;
let end = '';
let skipped = false;
const actualInspected = inspectValue(actual);
Expand Down Expand Up @@ -171,51 +164,48 @@ function createErrDiff(actual, expected, operator) {
}

let printedLines = 0;
let identical = 0;
const msg = kReadableOperator[operator] +
`\n${green}+ actual${white} ${red}- expected${white}`;
const skippedMsg = ` ${blue}...${white} Lines skipped`;

for (i = 0; i < maxLines; i++) {
// Only extra expected lines exist
const cur = i - lastPos;
if (actualLines.length < i + 1) {
// If the last diverging line is more than one line above and the
// current line is at least line three, add some of the former lines and
// also add dots to indicate skipped entries.
if (cur > 1 && i > 2) {
if (cur > 4) {
// If more than one former line is identical, print that. Collapse those
// in case more than three lines before were identical.
if (identical > 1) {
if (identical > 3) {
res += `\n${blue}...${white}`;
skipped = true;
} else if (cur > 3) {
} else if (identical > 2) {
res += `\n ${expectedLines[i - 2]}`;
printedLines++;
}
res += `\n ${expectedLines[i - 1]}`;
printedLines++;
}
// Mark the current line as the last diverging one.
lastPos = i;
// No identical lines before.
identical = 0;
// Add the expected line to the cache.
other += `\n${red}-${white} ${expectedLines[i]}`;
printedLines++;
// Only extra actual lines exist
} else if (expectedLines.length < i + 1) {
// If the last diverging line is more than one line above and the
// current line is at least line three, add some of the former lines and
// also add dots to indicate skipped entries.
if (cur > 1 && i > 2) {
if (cur > 4) {
// If more than one former line is identical, print that. Collapse those
// in case more than three lines before were identical.
if (identical > 1) {
if (identical > 3) {
res += `\n${blue}...${white}`;
skipped = true;
} else if (cur > 3) {
} else if (identical > 2) {
res += `\n ${actualLines[i - 2]}`;
printedLines++;
}
res += `\n ${actualLines[i - 1]}`;
printedLines++;
}
// Mark the current line as the last diverging one.
lastPos = i;
// No identical lines before.
identical = 0;
// Add the actual line to the result.
res += `\n${green}+${white} ${actualLines[i]}`;
printedLines++;
Expand Down Expand Up @@ -245,22 +235,21 @@ function createErrDiff(actual, expected, operator) {
actualLine += ',';
}
if (divergingLines) {
// If the last diverging line is more than one line above and the
// current line is at least line three, add some of the former lines and
// also add dots to indicate skipped entries.
if (cur > 1 && i > 2) {
if (cur > 4) {
// If more than one former line is identical, print that. Collapse those
// in case more than three lines before were identical.
if (identical > 1) {
if (identical > 3) {
res += `\n${blue}...${white}`;
skipped = true;
} else if (cur > 3) {
} else if (identical > 2) {
res += `\n ${actualLines[i - 2]}`;
printedLines++;
}
res += `\n ${actualLines[i - 1]}`;
printedLines++;
}
// Mark the current line as the last diverging one.
lastPos = i;
// No identical lines before.
identical = 0;
// Add the actual line to the result and cache the expected diverging
// line so consecutive diverging lines show up as +++--- and not +-+-+-.
res += `\n${green}+${white} ${actualLine}`;
Expand All @@ -272,9 +261,10 @@ function createErrDiff(actual, expected, operator) {
// and reset the cache.
res += other;
other = '';
// If the last diverging line is exactly one line above or if it is the
// very first line, add the line to the result.
if (cur === 1 || i === 0) {
identical++;
// The very first identical line since the last diverging line is be
// added to the result.
if (identical === 1) {
res += `\n ${actualLine}`;
printedLines++;
}
Expand Down Expand Up @@ -316,7 +306,7 @@ class AssertionError extends Error {
if (process.stderr.isTTY) {
// Reset on each call to make sure we handle dynamically set environment
// variables correct.
if (process.stderr.getColorDepth() !== 1) {
if (process.stderr.hasColors()) {
blue = '\u001b[34m';
green = '\u001b[32m';
white = '\u001b[39m';
Expand Down
27 changes: 21 additions & 6 deletions test/parallel/test-assert-deep.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ assert.deepEqual(arr, buf);
code: 'ERR_ASSERTION',
message: `${defaultMsgStartFull} ... Lines skipped\n\n` +
' Buffer [Uint8Array] [\n' +
' 120,\n' +
'...\n' +
' 10,\n' +
'+ prop: 1\n' +
Expand All @@ -87,7 +86,6 @@ assert.deepEqual(arr, buf);
code: 'ERR_ASSERTION',
message: `${defaultMsgStartFull} ... Lines skipped\n\n` +
' Uint8Array [\n' +
' 120,\n' +
'...\n' +
' 10,\n' +
'- prop: 5\n' +
Expand Down Expand Up @@ -921,13 +919,30 @@ assert.deepStrictEqual(obj1, obj2);
const a = new TypeError('foo');
const b = new TypeError('foo');
a.foo = 'bar';
b.foo = 'baz';
b.foo = 'baz.';

assert.throws(
() => assert.deepStrictEqual(a, b),
() => assert.throws(
() => assert.deepStrictEqual(a, b),
{
operator: 'throws',
message: `${defaultMsgStartFull}\n\n` +
' [TypeError: foo] {\n+ foo: \'bar\'\n- foo: \'baz\'\n }',
}
),
{
message: `${defaultMsgStartFull}\n\n` +
' [TypeError: foo] {\n+ foo: \'bar\'\n- foo: \'baz\'\n }'
message: 'Expected values to be strictly deep-equal:\n' +
'+ actual - expected ... Lines skipped\n' +
'\n' +
' Comparison {\n' +
'...\n' +
" \"+ foo: 'bar'\\n\" +\n" +
"+ \"- foo: 'baz.'\\n\" +\n" +
"- \"- foo: 'baz'\\n\" +\n" +
" ' }',\n" +
"+ operator: 'deepStrictEqual'\n" +
"- operator: 'throws'\n" +
' }'
}
);
}
Expand Down

0 comments on commit e4ec4f2

Please sign in to comment.