Skip to content

Commit 4f7a25b

Browse files
BridgeARBridgeAR
committed
assert: fix diff color output
The color output was broken at some point and that was not detected because it was not tested for properly so far. This makes sure the colors work again and it adds a regression test as well. PR-URL: nodejs#19464 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 979167c commit 4f7a25b

4 files changed

Lines changed: 77 additions & 60 deletions

File tree

‎lib/internal/errors.js‎

Lines changed: 34 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@ const kCode = Symbol('code');
1414
const messages = new Map();
1515
const codes = {};
1616

17-
var green = '';
18-
var red = '';
19-
var white = '';
17+
let green = '';
18+
let red = '';
19+
let white = '';
2020

2121
const {
2222
UV_EAI_MEMORY,
@@ -27,13 +27,7 @@ const { kMaxLength } = process.binding('buffer');
2727
const { defineProperty } = Object;
2828

2929
// Lazily loaded
30-
var util_ = null;
31-
function lazyUtil() {
32-
if (!util_) {
33-
util_ = require('util');
34-
}
35-
return util_;
36-
}
30+
var util;
3731

3832
var internalUtil = null;
3933
function lazyInternalUtil() {
@@ -43,6 +37,13 @@ function lazyInternalUtil() {
4337
return internalUtil;
4438
}
4539

40+
function inspectValue(val) {
41+
return util.inspect(
42+
val,
43+
{ compact: false, customInspect: false }
44+
).split('\n');
45+
}
46+
4647
function makeNodeError(Base) {
4748
return class NodeError extends Base {
4849
constructor(key, ...args) {
@@ -89,11 +90,9 @@ function createErrDiff(actual, expected, operator) {
8990
var lastPos = 0;
9091
var end = '';
9192
var skipped = false;
92-
const util = lazyUtil();
93-
const actualLines = util
94-
.inspect(actual, { compact: false, customInspect: false }).split('\n');
95-
const expectedLines = util
96-
.inspect(expected, { compact: false, customInspect: false }).split('\n');
93+
if (util === undefined) util = require('util');
94+
const actualLines = inspectValue(actual);
95+
const expectedLines = inspectValue(expected);
9796
const msg = `Input A expected to ${operator} input B:\n` +
9897
`${green}+ expected${white} ${red}- actual${white}`;
9998
const skippedMsg = ' ... Lines skipped';
@@ -260,14 +259,20 @@ class AssertionError extends Error {
260259
if (message != null) {
261260
super(message);
262261
} else {
263-
if (util_ === null &&
264-
process.stdout.isTTY &&
265-
process.stdout.getColorDepth() !== 1) {
266-
green = '\u001b[32m';
267-
white = '\u001b[39m';
268-
red = '\u001b[31m';
262+
if (process.stdout.isTTY) {
263+
// Reset on each call to make sure we handle dynamically set environment
264+
// variables correct.
265+
if (process.stdout.getColorDepth() !== 1) {
266+
green = '\u001b[32m';
267+
white = '\u001b[39m';
268+
red = '\u001b[31m';
269+
} else {
270+
green = '';
271+
white = '';
272+
red = '';
273+
}
269274
}
270-
const util = lazyUtil();
275+
if (util === undefined) util = require('util');
271276
if (typeof actual === 'object' && actual !== null &&
272277
'stack' in actual && actual instanceof Error) {
273278
actual = `${actual.name}: ${actual.message}`;
@@ -288,10 +293,7 @@ class AssertionError extends Error {
288293
} else if (errorDiff === 1) {
289294
// In case the objects are equal but the operator requires unequal, show
290295
// the first object and say A equals B
291-
const res = util.inspect(
292-
actual,
293-
{ compact: false, customInspect: false }
294-
).split('\n');
296+
const res = inspectValue(actual);
295297

296298
if (res.length > 20) {
297299
res[19] = '...';
@@ -333,10 +335,10 @@ function message(key, args) {
333335
const msg = messages.get(key);
334336
internalAssert(msg, `An invalid error message key was used: ${key}.`);
335337
let fmt;
338+
if (util === undefined) util = require('util');
336339
if (typeof msg === 'function') {
337340
fmt = msg;
338341
} else {
339-
const util = lazyUtil();
340342
fmt = util.format;
341343
if (args === undefined || args.length === 0)
342344
return msg;
@@ -358,7 +360,8 @@ function errnoException(err, syscall, original) {
358360
// getSystemErrorName(err) to guard against invalid arguments from users.
359361
// This can be replaced with [ code ] = errmap.get(err) when this method
360362
// is no longer exposed to user land.
361-
const code = lazyUtil().getSystemErrorName(err);
363+
if (util === undefined) util = require('util');
364+
const code = util.getSystemErrorName(err);
362365
const message = original ?
363366
`${syscall} ${code} ${original}` : `${syscall} ${code}`;
364367

@@ -386,7 +389,8 @@ function exceptionWithHostPort(err, syscall, address, port, additional) {
386389
// getSystemErrorName(err) to guard against invalid arguments from users.
387390
// This can be replaced with [ code ] = errmap.get(err) when this method
388391
// is no longer exposed to user land.
389-
const code = lazyUtil().getSystemErrorName(err);
392+
if (util === undefined) util = require('util');
393+
const code = util.getSystemErrorName(err);
390394
let details = '';
391395
if (port && port > 0) {
392396
details = ` ${address}:${port}`;
@@ -626,10 +630,9 @@ E('ERR_INSPECTOR_NOT_CONNECTED', 'Session is not connected', Error);
626630
E('ERR_INVALID_ADDRESS_FAMILY', 'Invalid address family: %s', RangeError);
627631
E('ERR_INVALID_ARG_TYPE', invalidArgType, TypeError);
628632
E('ERR_INVALID_ARG_VALUE', (name, value, reason = 'is invalid') => {
629-
const util = lazyUtil();
630633
let inspected = util.inspect(value);
631634
if (inspected.length > 128) {
632-
inspected = inspected.slice(0, 128) + '...';
635+
inspected = `${inspected.slice(0, 128)}...`;
633636
}
634637
return `The argument '${name}' ${reason}. Received ${inspected}`;
635638
}, TypeError, RangeError); // Some are currently falsy implemented as "Error"

‎test/parallel/test-assert.js‎

Lines changed: 25 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ const assert = require('assert');
2828
const { inspect } = require('util');
2929
const a = assert;
3030

31+
const start = 'Input A expected to deepStrictEqual input B:';
32+
const actExp = '+ expected - actual';
33+
3134
assert.ok(a.AssertionError.prototype instanceof Error,
3235
'a.AssertionError instanceof Error');
3336

@@ -430,14 +433,7 @@ common.expectsError(
430433
}
431434
);
432435

433-
// Test error diffs
434-
const colors = process.stdout.isTTY && process.stdout.getColorDepth() > 1;
435-
const start = 'Input A expected to deepStrictEqual input B:';
436-
const actExp = colors ?
437-
'\u001b[32m+ expected\u001b[39m \u001b[31m- actual\u001b[39m' :
438-
'+ expected - actual';
439-
const plus = colors ? '\u001b[32m+\u001b[39m' : '+';
440-
const minus = colors ? '\u001b[31m-\u001b[39m' : '-';
436+
// Test error diffs.
441437
let message = [
442438
start,
443439
`${actExp} ... Lines skipped`,
@@ -446,8 +442,8 @@ common.expectsError(
446442
' [',
447443
'...',
448444
' 2,',
449-
`${minus} 3`,
450-
`${plus} '3'`,
445+
'- 3',
446+
"+ '3'",
451447
' ]',
452448
'...',
453449
' 5',
@@ -464,7 +460,7 @@ common.expectsError(
464460
' 1,',
465461
'...',
466462
' 0,',
467-
`${plus} 1,`,
463+
'+ 1,',
468464
' 1,',
469465
'...',
470466
' 1',
@@ -484,7 +480,7 @@ common.expectsError(
484480
' 1,',
485481
'...',
486482
' 0,',
487-
`${minus} 1,`,
483+
'- 1,',
488484
' 1,',
489485
'...',
490486
' 1',
@@ -502,12 +498,12 @@ common.expectsError(
502498
'',
503499
' [',
504500
' 1,',
505-
`${minus} 2,`,
506-
`${plus} 1,`,
501+
'- 2,',
502+
'+ 1,',
507503
' 1,',
508504
' 1,',
509505
' 0,',
510-
`${minus} 1,`,
506+
'- 1,',
511507
' 1',
512508
' ]'
513509
].join('\n');
@@ -521,12 +517,12 @@ common.expectsError(
521517
start,
522518
actExp,
523519
'',
524-
`${minus} [`,
525-
`${minus} 1,`,
526-
`${minus} 2,`,
527-
`${minus} 1`,
528-
`${minus} ]`,
529-
`${plus} undefined`,
520+
'- [',
521+
'- 1,',
522+
'- 2,',
523+
'- 1',
524+
'- ]',
525+
'+ undefined',
530526
].join('\n');
531527
assert.throws(
532528
() => assert.deepEqual([1, 2, 1]),
@@ -537,7 +533,7 @@ common.expectsError(
537533
actExp,
538534
'',
539535
' [',
540-
`${minus} 1,`,
536+
'- 1,',
541537
' 2,',
542538
' 1',
543539
' ]'
@@ -550,9 +546,9 @@ common.expectsError(
550546
`${actExp} ... Lines skipped\n` +
551547
'\n' +
552548
' [\n' +
553-
`${minus} 1,\n`.repeat(10) +
549+
'- 1,\n'.repeat(10) +
554550
'...\n' +
555-
`${plus} 2,\n`.repeat(10) +
551+
'+ 2,\n'.repeat(10) +
556552
'...';
557553
assert.throws(
558554
() => assert.deepEqual(Array(12).fill(1), Array(12).fill(2)),
@@ -566,11 +562,11 @@ common.expectsError(
566562
message: `${start}\n` +
567563
`${actExp}\n` +
568564
'\n' +
569-
`${minus} {}\n` +
570-
`${plus} {\n` +
571-
`${plus} loop: 'forever',\n` +
572-
`${plus} [Symbol(util.inspect.custom)]: [Function]\n` +
573-
`${plus} }`
565+
'- {}\n' +
566+
'+ {\n' +
567+
"+ loop: 'forever',\n" +
568+
'+ [Symbol(util.inspect.custom)]: [Function]\n' +
569+
'+ }'
574570
});
575571

576572
// notDeepEqual tests

‎test/pseudo-tty/test-assert-colors.js‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
'use strict';
2+
require('../common');
3+
const assert = require('assert').strict;
4+
5+
try {
6+
// Activate colors even if the tty does not support colors.
7+
process.env.COLORTERM = '1';
8+
assert.deepStrictEqual([1, 2], [2, 2]);
9+
} catch (err) {
10+
const expected = 'Input A expected to deepStrictEqual input B:\n' +
11+
'\u001b[32m+ expected\u001b[39m \u001b[31m- actual\u001b[39m\n\n' +
12+
' [\n' +
13+
'\u001b[31m-\u001b[39m 1,\n' +
14+
'\u001b[32m+\u001b[39m 2,\n' +
15+
' 2\n' +
16+
' ]';
17+
assert.strictEqual(err.message, expected);
18+
}

‎test/pseudo-tty/test-assert-colors.out‎

Whitespace-only changes.

0 commit comments

Comments
 (0)