From 0757b9b292e11144213fd91ae4939b28161fcb2f Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 26 Apr 2018 03:14:47 +0200 Subject: [PATCH 1/3] assert: minor error message improvements Adjust indentations and fix a typo. --- lib/internal/errors.js | 15 ++++++--------- test/parallel/test-assert.js | 7 +++---- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index e7eb3bd133813a..3dfd661abe997a 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -360,13 +360,10 @@ function createErrDiff(actual, expected, operator) { // Strict equal with identical objects that are not identical by reference. if (identical === maxLines) { - let base = 'Input object identical but not reference equal:'; - - if (operator !== 'strictEqual') { - // This code path should not be possible to reach. - // The output is identical but it is not clear why. - base = 'Input objects not identical:'; - } + // 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); @@ -380,7 +377,7 @@ function createErrDiff(actual, expected, operator) { } } - return `${base}\n\n ${actualLines.join('\n ')}\n`; + return `${base}\n\n${actualLines.join('\n')}\n`; } return `${msg}${skipped ? skippedMsg : ''}\n${res}${other}${end}`; } @@ -448,7 +445,7 @@ class AssertionError extends Error { if (res.length === 1) { super(`${base} ${res[0]}`); } else { - super(`${base}\n\n ${res.join('\n ')}\n`); + super(`${base}\n\n${res.join('\n')}\n`); } } else { let res = util.inspect(actual); diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index b871cd7265d288..d892122ed62ff2 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -599,14 +599,13 @@ assert.throws( }); // notDeepEqual tests - message = 'Identical input passed to notDeepStrictEqual:\n\n' + - ' [\n 1\n ]\n'; + message = 'Identical input passed to notDeepStrictEqual:\n\n[\n 1\n]\n'; assert.throws( () => assert.notDeepEqual([1], [1]), { message }); message = 'Identical input passed to notDeepStrictEqual:' + - `\n\n [${'\n 1,'.repeat(25)}\n ...\n`; + `\n\n[${'\n 1,'.repeat(25)}\n...\n`; const data = Array(31).fill(1); assert.throws( () => assert.notDeepEqual(data, data), @@ -901,7 +900,7 @@ assert.throws(() => { throw null; }, 'foo'); assert.throws( () => assert.strictEqual([], []), { - message: 'Input object identical but not reference equal:\n\n []\n' + message: 'Input objects identical but not reference equal:\n\n[]\n' } ); From bef57a603de6f141862eadbcd6c8d2d3de0aba9a Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 26 Apr 2018 03:24:45 +0200 Subject: [PATCH 2/3] assert: make skipping indicator blue If lines gets skipped, they are marked with three dots. This makes sure they are better visualized to distinguish them from everything else. --- lib/internal/errors.js | 20 ++++++++++++-------- test/pseudo-tty/test-assert-colors.js | 7 +++++-- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 3dfd661abe997a..b8f91ab6fca604 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -15,6 +15,7 @@ const kInfo = Symbol('info'); const messages = new Map(); const codes = {}; +let blue = ''; let green = ''; let red = ''; let white = ''; @@ -259,7 +260,7 @@ function createErrDiff(actual, expected, operator) { const expectedLines = inspectValue(expected); const msg = READABLE_OPERATOR[operator] + `:\n${green}+ expected${white} ${red}- actual${white}`; - const skippedMsg = ' ... Lines skipped'; + const skippedMsg = ` ${blue}...${white} Lines skipped`; // Remove all ending lines that match (this optimizes the output for // readability by reducing the number of total changed lines). @@ -280,7 +281,7 @@ function createErrDiff(actual, expected, operator) { b = expectedLines[expectedLines.length - 1]; } if (i > 3) { - end = `\n...${end}`; + end = `\n${blue}...${white}${end}`; skipped = true; } if (other !== '') { @@ -297,7 +298,7 @@ function createErrDiff(actual, expected, operator) { if (actualLines.length < i + 1) { if (cur > 1 && i > 2) { if (cur > 4) { - res += '\n...'; + res += `\n${blue}...${white}`; skipped = true; } else if (cur > 3) { res += `\n ${expectedLines[i - 2]}`; @@ -313,7 +314,7 @@ function createErrDiff(actual, expected, operator) { } else if (expectedLines.length < i + 1) { if (cur > 1 && i > 2) { if (cur > 4) { - res += '\n...'; + res += `\n${blue}...${white}`; skipped = true; } else if (cur > 3) { res += `\n ${actualLines[i - 2]}`; @@ -329,7 +330,7 @@ function createErrDiff(actual, expected, operator) { } else if (actualLines[i] !== expectedLines[i]) { if (cur > 1 && i > 2) { if (cur > 4) { - res += '\n...'; + res += `\n${blue}...${white}`; skipped = true; } else if (cur > 3) { res += `\n ${actualLines[i - 2]}`; @@ -354,7 +355,8 @@ function createErrDiff(actual, expected, operator) { } // Inspected object to big (Show ~20 rows max) if (printedLines > 20 && i < maxLines - 2) { - return `${msg}${skippedMsg}\n${res}\n...${other}\n...`; + return `${msg}${skippedMsg}\n${res}\n${blue}...${white}${other}\n` + + `${blue}...${white}`; } } @@ -371,7 +373,7 @@ function createErrDiff(actual, expected, operator) { // 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] = '...'; + actualLines[26] = `${blue}...${white}`; while (actualLines.length > 27) { actualLines.pop(); } @@ -402,10 +404,12 @@ class AssertionError extends Error { // Reset on each call to make sure we handle dynamically set environment // variables correct. if (process.stdout.getColorDepth() !== 1) { + blue = '\u001b[34m'; green = '\u001b[32m'; white = '\u001b[39m'; red = '\u001b[31m'; } else { + blue = ''; green = ''; white = ''; red = ''; @@ -435,7 +439,7 @@ class AssertionError extends Error { // Only remove lines in case it makes sense to collapse those. // TODO: Accept env to always show the full error. if (res.length > 30) { - res[26] = '...'; + res[26] = `${blue}...${white}`; while (res.length > 27) { res.pop(); } diff --git a/test/pseudo-tty/test-assert-colors.js b/test/pseudo-tty/test-assert-colors.js index 39bee740d2ea15..b6f980695622dc 100644 --- a/test/pseudo-tty/test-assert-colors.js +++ b/test/pseudo-tty/test-assert-colors.js @@ -5,13 +5,16 @@ const assert = require('assert').strict; try { // Activate colors even if the tty does not support colors. process.env.COLORTERM = '1'; - assert.deepStrictEqual([1, 2], [2, 2]); + assert.deepStrictEqual([1, 2, 2, 2], [2, 2, 2, 2]); } catch (err) { const expected = 'Input A expected to strictly deep-equal input B:\n' + - '\u001b[32m+ expected\u001b[39m \u001b[31m- actual\u001b[39m\n\n' + + '\u001b[32m+ expected\u001b[39m \u001b[31m- actual\u001b[39m' + + ' \u001b[34m...\u001b[39m Lines skipped\n\n' + ' [\n' + '\u001b[31m-\u001b[39m 1,\n' + '\u001b[32m+\u001b[39m 2,\n' + + ' 2\n,' + + '\u001b[34m...\u001b[39m' + ' 2\n' + ' ]'; assert.strictEqual(err.message, expected); From 52eb7e6631e29e515876ecb96969a5bcfe80abd8 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 26 Apr 2018 05:48:54 +0200 Subject: [PATCH 3/3] fixup ... --- test/pseudo-tty/test-assert-colors.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/pseudo-tty/test-assert-colors.js b/test/pseudo-tty/test-assert-colors.js index b6f980695622dc..75d3af55796e1b 100644 --- a/test/pseudo-tty/test-assert-colors.js +++ b/test/pseudo-tty/test-assert-colors.js @@ -13,8 +13,8 @@ try { ' [\n' + '\u001b[31m-\u001b[39m 1,\n' + '\u001b[32m+\u001b[39m 2,\n' + - ' 2\n,' + - '\u001b[34m...\u001b[39m' + + ' 2,\n' + + '\u001b[34m...\u001b[39m\n' + ' 2\n' + ' ]'; assert.strictEqual(err.message, expected);