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.

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

Jump to bottom

assert: reduce diff noise #23048

Closed
wants to merge 3 commits into from
+83 −30
Closed

assert: reduce diff noise #23048

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1e6bd03
assert: reduce diff noise
BridgeAR Sep 24, 2018
11236df
assert: add comments for diff algorithm
BridgeAR Sep 24, 2018
8b493b5
fixup: fix wrong wording
BridgeAR Sep 25, 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
89 changes: 67 additions & 22 deletions lib/internal/assert.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,9 @@ function createErrDiff(actual, expected, operator) {
// 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) {
res += `\n${blue}...${white}`;
Expand All @@ -171,11 +174,16 @@ function createErrDiff(actual, expected, operator) {
res += `\n ${expectedLines[i - 1]}`;
printedLines++;
}
// Mark the current line as the last diverging one.
lastPos = i;
// 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) {
res += `\n${blue}...${white}`;
Expand All @@ -187,39 +195,76 @@ function createErrDiff(actual, expected, operator) {
res += `\n ${actualLines[i - 1]}`;
printedLines++;
}
// Mark the current line as the last diverging one.
lastPos = i;
// Add the actual line to the result.
res += `\n${green}+${white} ${actualLines[i]}`;
printedLines++;
// Lines diverge
} else if (actualLines[i] !== expectedLines[i]) {
if (cur > 1 && i > 2) {
if (cur > 4) {
res += `\n${blue}...${white}`;
skipped = true;
} else if (cur > 3) {
res += `\n ${actualLines[i - 2]}`;
} else {
const expectedLine = expectedLines[i];
let actualLine = actualLines[i];
// If the lines diverge, specifically check for lines that only diverge by
// a trailing comma. In that case it is actually identical and we should
// mark it as such.
let divergingLines = actualLine !== expectedLine &&
(!actualLine.endsWith(',') ||
actualLine.slice(0, -1) !== expectedLine);
// If the expected line has a trailing comma but is otherwise identical,
// add a comma at the end of the actual line. Otherwise the output could
// look weird as in:
//
// [
// 1 // No comma at the end!
// + 2
// ]
//
if (divergingLines &&
expectedLine.endsWith(',') &&
expectedLine.slice(0, -1) === actualLine) {
divergingLines = false;
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) {
res += `\n${blue}...${white}`;
skipped = true;
} else if (cur > 3) {
res += `\n ${actualLines[i - 2]}`;
printedLines++;
}
res += `\n ${actualLines[i - 1]}`;
printedLines++;
}
BridgeAR marked this conversation as resolved.
Show resolved Hide resolved
// Mark the current line as the last diverging one.
lastPos = i;
// 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}`;
other += `\n${red}-${white} ${expectedLine}`;
printedLines += 2;
// Lines are identical
} else {
// Add all cached information to the result before adding other things
// 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) {
res += `\n ${actualLine}`;
printedLines++;
}
res += `\n ${actualLines[i - 1]}`;
printedLines++;
}
lastPos = i;
res += `\n${green}+${white} ${actualLines[i]}`;
other += `\n${red}-${white} ${expectedLines[i]}`;
printedLines += 2;
// Lines are identical
} else {
res += other;
other = '';
if (cur === 1 || i === 0) {
res += `\n ${actualLines[i]}`;
printedLines++;
}
}
// Inspected object to big (Show ~20 rows max)
if (printedLines > 20 && i < maxLines - 2) {
return `${msg}${skippedMsg}\n${res}\n${blue}...${white}${other}\n` +
`${blue}...${white}`;
`${blue}...${white}`;
}
}

Expand Down
24 changes: 16 additions & 8 deletions test/parallel/test-assert-deep.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,13 @@ assert.deepEqual(arr, buf);
() => assert.deepStrictEqual(buf2, buf),
{
code: 'ERR_ASSERTION',
message: `${defaultMsgStartFull}\n\n` +
' Buffer [Uint8Array] [\n 120,\n 121,\n 122,\n' +
'+ 10,\n+ prop: 1\n- 10\n ]'
message: `${defaultMsgStartFull} ... Lines skipped\n\n` +
' Buffer [Uint8Array] [\n' +
' 120,\n' +
'...\n' +
' 10,\n' +
'+ prop: 1\n' +
' ]'
}
);
assert.deepEqual(buf2, buf);
Expand All @@ -80,9 +84,13 @@ assert.deepEqual(arr, buf);
() => assert.deepStrictEqual(arr, arr2),
{
code: 'ERR_ASSERTION',
message: `${defaultMsgStartFull}\n\n` +
' Uint8Array [\n 120,\n 121,\n 122,\n' +
'+ 10\n- 10,\n- prop: 5\n ]'
message: `${defaultMsgStartFull} ... Lines skipped\n\n` +
' Uint8Array [\n' +
' 120,\n' +
'...\n' +
' 10,\n' +
'- prop: 5\n' +
' ]'
}
);
assert.deepEqual(arr, arr2);
Expand Down Expand Up @@ -822,7 +830,7 @@ assert.throws(
code: 'ERR_ASSERTION',
name: 'AssertionError [ERR_ASSERTION]',
message: `${defaultMsgStartFull}\n\n ` +
'{\n+ a: 4\n- a: 4,\n- b: true\n }'
'{\n a: 4,\n- b: true\n }'
});
assert.throws(
() => assert.deepStrictEqual(['a'], { 0: 'a' }),
Expand Down Expand Up @@ -891,7 +899,7 @@ assert.deepStrictEqual(obj1, obj2);
assert.throws(
() => assert.deepStrictEqual(arrProxy, [1, 2, 3]),
{ message: `${defaultMsgStartFull}\n\n` +
' [\n 1,\n+ 2\n- 2,\n- 3\n ]' }
' [\n 1,\n 2,\n- 3\n ]' }
);
util.inspect.defaultOptions = tmp;

Expand Down