Skip to content

Commit 8217441

Browse files
committed
assert: fix error diff
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: #28058 Reviewed-By: Rich Trott <rtrott@gmail.com>
1 parent 1ee7ce6 commit 8217441

File tree

2 files changed

+49
-44
lines changed

2 files changed

+49
-44
lines changed

lib/internal/assert/assertion_error.js

Lines changed: 28 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -52,25 +52,18 @@ function inspectValue(val) {
5252
maxArrayLength: Infinity,
5353
// Assert compares only enumerable properties (with a few exceptions).
5454
showHidden: false,
55-
// Having a long line as error is better than wrapping the line for
56-
// comparison for now.
57-
// TODO(BridgeAR): `breakLength` should be limited as soon as soon as we
58-
// have meta information about the inspected properties (i.e., know where
59-
// in what line the property starts and ends).
60-
breakLength: Infinity,
6155
// Assert does not detect proxies currently.
6256
showProxy: false,
6357
sorted: true,
6458
// Inspect getters as we also check them when comparing entries.
65-
getters: true
59+
getters: true,
6660
}
6761
);
6862
}
6963

7064
function createErrDiff(actual, expected, operator) {
7165
let other = '';
7266
let res = '';
73-
let lastPos = 0;
7467
let end = '';
7568
let skipped = false;
7669
const actualInspected = inspectValue(actual);
@@ -171,51 +164,48 @@ function createErrDiff(actual, expected, operator) {
171164
}
172165

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

178172
for (i = 0; i < maxLines; i++) {
179-
// Only extra expected lines exist
180-
const cur = i - lastPos;
181173
if (actualLines.length < i + 1) {
182-
// If the last diverging line is more than one line above and the
183-
// current line is at least line three, add some of the former lines and
184-
// also add dots to indicate skipped entries.
185-
if (cur > 1 && i > 2) {
186-
if (cur > 4) {
174+
// If more than one former line is identical, print that. Collapse those
175+
// in case more than three lines before were identical.
176+
if (identical > 1) {
177+
if (identical > 3) {
187178
res += `\n${blue}...${white}`;
188179
skipped = true;
189-
} else if (cur > 3) {
180+
} else if (identical > 2) {
190181
res += `\n ${expectedLines[i - 2]}`;
191182
printedLines++;
192183
}
193184
res += `\n ${expectedLines[i - 1]}`;
194185
printedLines++;
195186
}
196-
// Mark the current line as the last diverging one.
197-
lastPos = i;
187+
// No identical lines before.
188+
identical = 0;
198189
// Add the expected line to the cache.
199190
other += `\n${red}-${white} ${expectedLines[i]}`;
200191
printedLines++;
201192
// Only extra actual lines exist
202193
} else if (expectedLines.length < i + 1) {
203-
// If the last diverging line is more than one line above and the
204-
// current line is at least line three, add some of the former lines and
205-
// also add dots to indicate skipped entries.
206-
if (cur > 1 && i > 2) {
207-
if (cur > 4) {
194+
// If more than one former line is identical, print that. Collapse those
195+
// in case more than three lines before were identical.
196+
if (identical > 1) {
197+
if (identical > 3) {
208198
res += `\n${blue}...${white}`;
209199
skipped = true;
210-
} else if (cur > 3) {
200+
} else if (identical > 2) {
211201
res += `\n ${actualLines[i - 2]}`;
212202
printedLines++;
213203
}
214204
res += `\n ${actualLines[i - 1]}`;
215205
printedLines++;
216206
}
217-
// Mark the current line as the last diverging one.
218-
lastPos = i;
207+
// No identical lines before.
208+
identical = 0;
219209
// Add the actual line to the result.
220210
res += `\n${green}+${white} ${actualLines[i]}`;
221211
printedLines++;
@@ -245,22 +235,21 @@ function createErrDiff(actual, expected, operator) {
245235
actualLine += ',';
246236
}
247237
if (divergingLines) {
248-
// If the last diverging line is more than one line above and the
249-
// current line is at least line three, add some of the former lines and
250-
// also add dots to indicate skipped entries.
251-
if (cur > 1 && i > 2) {
252-
if (cur > 4) {
238+
// If more than one former line is identical, print that. Collapse those
239+
// in case more than three lines before were identical.
240+
if (identical > 1) {
241+
if (identical > 3) {
253242
res += `\n${blue}...${white}`;
254243
skipped = true;
255-
} else if (cur > 3) {
244+
} else if (identical > 2) {
256245
res += `\n ${actualLines[i - 2]}`;
257246
printedLines++;
258247
}
259248
res += `\n ${actualLines[i - 1]}`;
260249
printedLines++;
261250
}
262-
// Mark the current line as the last diverging one.
263-
lastPos = i;
251+
// No identical lines before.
252+
identical = 0;
264253
// Add the actual line to the result and cache the expected diverging
265254
// line so consecutive diverging lines show up as +++--- and not +-+-+-.
266255
res += `\n${green}+${white} ${actualLine}`;
@@ -272,9 +261,10 @@ function createErrDiff(actual, expected, operator) {
272261
// and reset the cache.
273262
res += other;
274263
other = '';
275-
// If the last diverging line is exactly one line above or if it is the
276-
// very first line, add the line to the result.
277-
if (cur === 1 || i === 0) {
264+
identical++;
265+
// The very first identical line since the last diverging line is be
266+
// added to the result.
267+
if (identical === 1) {
278268
res += `\n ${actualLine}`;
279269
printedLines++;
280270
}
@@ -316,7 +306,7 @@ class AssertionError extends Error {
316306
if (process.stderr.isTTY) {
317307
// Reset on each call to make sure we handle dynamically set environment
318308
// variables correct.
319-
if (process.stderr.getColorDepth() !== 1) {
309+
if (process.stderr.hasColors()) {
320310
blue = '\u001b[34m';
321311
green = '\u001b[32m';
322312
white = '\u001b[39m';

test/parallel/test-assert-deep.js

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@ assert.deepEqual(arr, buf);
6868
code: 'ERR_ASSERTION',
6969
message: `${defaultMsgStartFull} ... Lines skipped\n\n` +
7070
' Buffer [Uint8Array] [\n' +
71-
' 120,\n' +
7271
'...\n' +
7372
' 10,\n' +
7473
'+ prop: 1\n' +
@@ -87,7 +86,6 @@ assert.deepEqual(arr, buf);
8786
code: 'ERR_ASSERTION',
8887
message: `${defaultMsgStartFull} ... Lines skipped\n\n` +
8988
' Uint8Array [\n' +
90-
' 120,\n' +
9189
'...\n' +
9290
' 10,\n' +
9391
'- prop: 5\n' +
@@ -921,13 +919,30 @@ assert.deepStrictEqual(obj1, obj2);
921919
const a = new TypeError('foo');
922920
const b = new TypeError('foo');
923921
a.foo = 'bar';
924-
b.foo = 'baz';
922+
b.foo = 'baz.';
925923

926924
assert.throws(
927-
() => assert.deepStrictEqual(a, b),
925+
() => assert.throws(
926+
() => assert.deepStrictEqual(a, b),
927+
{
928+
operator: 'throws',
929+
message: `${defaultMsgStartFull}\n\n` +
930+
' [TypeError: foo] {\n+ foo: \'bar\'\n- foo: \'baz\'\n }',
931+
}
932+
),
928933
{
929-
message: `${defaultMsgStartFull}\n\n` +
930-
' [TypeError: foo] {\n+ foo: \'bar\'\n- foo: \'baz\'\n }'
934+
message: 'Expected values to be strictly deep-equal:\n' +
935+
'+ actual - expected ... Lines skipped\n' +
936+
'\n' +
937+
' Comparison {\n' +
938+
'...\n' +
939+
" \"+ foo: 'bar'\\n\" +\n" +
940+
"+ \"- foo: 'baz.'\\n\" +\n" +
941+
"- \"- foo: 'baz'\\n\" +\n" +
942+
" ' }',\n" +
943+
"+ operator: 'deepStrictEqual'\n" +
944+
"- operator: 'throws'\n" +
945+
' }'
931946
}
932947
);
933948
}

0 commit comments

Comments
 (0)