diff options
author | Ruben Bridgewater <ruben@bridgewater.de> | 2018-03-20 02:25:22 +0100 |
---|---|---|
committer | Ruben Bridgewater <ruben@bridgewater.de> | 2018-03-27 01:20:19 +0100 |
commit | 2e6dd93aaa40e9f205a2e84920213effab81bea1 (patch) | |
tree | 8e389efa2af112a5a07fa2d182b3552e0b752aa0 | |
parent | d74919cc1a47f4f40766ba7f37ab434db246e700 (diff) | |
download | android-node-v8-2e6dd93aaa40e9f205a2e84920213effab81bea1.tar.gz android-node-v8-2e6dd93aaa40e9f205a2e84920213effab81bea1.tar.bz2 android-node-v8-2e6dd93aaa40e9f205a2e84920213effab81bea1.zip |
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: https://github.com/nodejs/node/pull/19464
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
-rw-r--r-- | lib/internal/errors.js | 66 | ||||
-rw-r--r-- | test/parallel/test-assert.js | 49 | ||||
-rw-r--r-- | test/pseudo-tty/test-assert-colors.js | 18 | ||||
-rw-r--r-- | test/pseudo-tty/test-assert-colors.out | 0 |
4 files changed, 74 insertions, 59 deletions
diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 16b4cd2416..e79041d91d 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -15,9 +15,9 @@ const kInfo = Symbol('info'); const messages = new Map(); const codes = {}; -var green = ''; -var red = ''; -var white = ''; +let green = ''; +let red = ''; +let white = ''; const { errmap, @@ -29,16 +29,9 @@ const { kMaxLength } = process.binding('buffer'); const { defineProperty } = Object; // Lazily loaded -var util_ = null; +var util; var buffer; -function lazyUtil() { - if (!util_) { - util_ = require('util'); - } - return util_; -} - var internalUtil = null; function lazyInternalUtil() { if (!internalUtil) { @@ -47,6 +40,13 @@ function lazyInternalUtil() { return internalUtil; } +function inspectValue(val) { + return util.inspect( + val, + { compact: false, customInspect: false } + ).split('\n'); +} + function makeNodeError(Base) { return class NodeError extends Base { constructor(key, ...args) { @@ -210,11 +210,9 @@ function createErrDiff(actual, expected, operator) { var lastPos = 0; var end = ''; var skipped = false; - const util = lazyUtil(); - const actualLines = util - .inspect(actual, { compact: false, customInspect: false }).split('\n'); - const expectedLines = util - .inspect(expected, { compact: false, customInspect: false }).split('\n'); + if (util === undefined) util = require('util'); + const actualLines = inspectValue(actual); + const expectedLines = inspectValue(expected); const msg = `Input A expected to ${operator} input B:\n` + `${green}+ expected${white} ${red}- actual${white}`; const skippedMsg = ' ... Lines skipped'; @@ -333,14 +331,20 @@ class AssertionError extends Error { if (message != null) { super(message); } else { - if (util_ === null && - process.stdout.isTTY && - process.stdout.getColorDepth() !== 1) { - green = '\u001b[32m'; - white = '\u001b[39m'; - red = '\u001b[31m'; + if (process.stdout.isTTY) { + // Reset on each call to make sure we handle dynamically set environment + // variables correct. + if (process.stdout.getColorDepth() !== 1) { + green = '\u001b[32m'; + white = '\u001b[39m'; + red = '\u001b[31m'; + } else { + green = ''; + white = ''; + red = ''; + } } - const util = lazyUtil(); + if (util === undefined) util = require('util'); if (typeof actual === 'object' && actual !== null && 'stack' in actual && actual instanceof Error) { actual = `${actual.name}: ${actual.message}`; @@ -361,10 +365,7 @@ class AssertionError extends Error { } else if (errorDiff === 1) { // In case the objects are equal but the operator requires unequal, show // the first object and say A equals B - const res = util.inspect( - actual, - { compact: false, customInspect: false } - ).split('\n'); + const res = inspectValue(actual); if (res.length > 20) { res[19] = '...'; @@ -406,10 +407,10 @@ function message(key, args) { const msg = messages.get(key); internalAssert(msg, `An invalid error message key was used: ${key}.`); let fmt; + if (util === undefined) util = require('util'); if (typeof msg === 'function') { fmt = msg; } else { - const util = lazyUtil(); fmt = util.format; if (args === undefined || args.length === 0) return msg; @@ -479,7 +480,8 @@ function errnoException(err, syscall, original) { // getSystemErrorName(err) to guard against invalid arguments from users. // This can be replaced with [ code ] = errmap.get(err) when this method // is no longer exposed to user land. - const code = lazyUtil().getSystemErrorName(err); + if (util === undefined) util = require('util'); + const code = util.getSystemErrorName(err); const message = original ? `${syscall} ${code} ${original}` : `${syscall} ${code}`; @@ -508,7 +510,8 @@ function exceptionWithHostPort(err, syscall, address, port, additional) { // getSystemErrorName(err) to guard against invalid arguments from users. // This can be replaced with [ code ] = errmap.get(err) when this method // is no longer exposed to user land. - const code = lazyUtil().getSystemErrorName(err); + if (util === undefined) util = require('util'); + const code = util.getSystemErrorName(err); let details = ''; if (port && port > 0) { details = ` ${address}:${port}`; @@ -768,10 +771,9 @@ E('ERR_INSPECTOR_NOT_CONNECTED', 'Session is not connected', Error); E('ERR_INVALID_ADDRESS_FAMILY', 'Invalid address family: %s', RangeError); E('ERR_INVALID_ARG_TYPE', invalidArgType, TypeError); E('ERR_INVALID_ARG_VALUE', (name, value, reason = 'is invalid') => { - const util = lazyUtil(); let inspected = util.inspect(value); if (inspected.length > 128) { - inspected = inspected.slice(0, 128) + '...'; + inspected = `${inspected.slice(0, 128)}...`; } return `The argument '${name}' ${reason}. Received ${inspected}`; }, TypeError, RangeError); diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index 195d9bbc80..54972657e9 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -34,11 +34,8 @@ const { writeFileSync, unlinkSync } = require('fs'); const { inspect } = require('util'); const a = assert; -const colors = process.stdout.isTTY && process.stdout.getColorDepth() > 1; const start = 'Input A expected to deepStrictEqual input B:'; -const actExp = colors ? - '\u001b[32m+ expected\u001b[39m \u001b[31m- actual\u001b[39m' : - '+ expected - actual'; +const actExp = '+ expected - actual'; assert.ok(a.AssertionError.prototype instanceof Error, 'a.AssertionError instanceof Error'); @@ -442,8 +439,6 @@ common.expectsError( Error.stackTraceLimit = tmpLimit; // Test error diffs. - const plus = colors ? '\u001b[32m+\u001b[39m' : '+'; - const minus = colors ? '\u001b[31m-\u001b[39m' : '-'; let message = [ start, `${actExp} ... Lines skipped`, @@ -452,8 +447,8 @@ common.expectsError( ' [', '...', ' 2,', - `${minus} 3`, - `${plus} '3'`, + '- 3', + "+ '3'", ' ]', '...', ' 5', @@ -470,7 +465,7 @@ common.expectsError( ' 1,', '...', ' 0,', - `${plus} 1,`, + '+ 1,', ' 1,', '...', ' 1', @@ -490,7 +485,7 @@ common.expectsError( ' 1,', '...', ' 0,', - `${minus} 1,`, + '- 1,', ' 1,', '...', ' 1', @@ -508,12 +503,12 @@ common.expectsError( '', ' [', ' 1,', - `${minus} 2,`, - `${plus} 1,`, + '- 2,', + '+ 1,', ' 1,', ' 1,', ' 0,', - `${minus} 1,`, + '- 1,', ' 1', ' ]' ].join('\n'); @@ -527,12 +522,12 @@ common.expectsError( start, actExp, '', - `${minus} [`, - `${minus} 1,`, - `${minus} 2,`, - `${minus} 1`, - `${minus} ]`, - `${plus} undefined`, + '- [', + '- 1,', + '- 2,', + '- 1', + '- ]', + '+ undefined', ].join('\n'); assert.throws( () => assert.deepEqual([1, 2, 1]), @@ -543,7 +538,7 @@ common.expectsError( actExp, '', ' [', - `${minus} 1,`, + '- 1,', ' 2,', ' 1', ' ]' @@ -556,9 +551,9 @@ common.expectsError( `${actExp} ... Lines skipped\n` + '\n' + ' [\n' + - `${minus} 1,\n`.repeat(10) + + '- 1,\n'.repeat(10) + '...\n' + - `${plus} 2,\n`.repeat(10) + + '+ 2,\n'.repeat(10) + '...'; assert.throws( () => assert.deepEqual(Array(12).fill(1), Array(12).fill(2)), @@ -572,11 +567,11 @@ common.expectsError( message: `${start}\n` + `${actExp}\n` + '\n' + - `${minus} {}\n` + - `${plus} {\n` + - `${plus} loop: 'forever',\n` + - `${plus} [Symbol(util.inspect.custom)]: [Function]\n` + - `${plus} }` + '- {}\n' + + '+ {\n' + + "+ loop: 'forever',\n" + + '+ [Symbol(util.inspect.custom)]: [Function]\n' + + '+ }' }); // notDeepEqual tests diff --git a/test/pseudo-tty/test-assert-colors.js b/test/pseudo-tty/test-assert-colors.js new file mode 100644 index 0000000000..7c5845bdaa --- /dev/null +++ b/test/pseudo-tty/test-assert-colors.js @@ -0,0 +1,18 @@ +'use strict'; +require('../common'); +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]); +} catch (err) { + const expected = 'Input A expected to deepStrictEqual input B:\n' + + '\u001b[32m+ expected\u001b[39m \u001b[31m- actual\u001b[39m\n\n' + + ' [\n' + + '\u001b[31m-\u001b[39m 1,\n' + + '\u001b[32m+\u001b[39m 2,\n' + + ' 2\n' + + ' ]'; + assert.strictEqual(err.message, expected); +} diff --git a/test/pseudo-tty/test-assert-colors.out b/test/pseudo-tty/test-assert-colors.out new file mode 100644 index 0000000000..e69de29bb2 --- /dev/null +++ b/test/pseudo-tty/test-assert-colors.out |