From c6aa5ff7e2b701039f803626af1c3263b0c2beb2 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 13 Nov 2017 16:44:30 -0200 Subject: [PATCH 01/21] doc: improve assert documentation 1) Separate all loose and strict functions. 2) Stronger outline the used comparison rules in (not)deepStrictEqual 3) Fix SameValue comparison info PR-URL: https://github.com/nodejs/node/pull/17002 Reviewed-By: Refael Ackermann Reviewed-By: James M Snell Reviewed-By: Vse Mozhet Byt Reviewed-By: Anna Henningsen --- doc/api/assert.md | 91 +++++++++++++++++++++++------------------------ 1 file changed, 44 insertions(+), 47 deletions(-) diff --git a/doc/api/assert.md b/doc/api/assert.md index 6935a9db4bda64..058aff858be8e5 100644 --- a/doc/api/assert.md +++ b/doc/api/assert.md @@ -101,7 +101,7 @@ assert.deepEqual(obj1, obj4); If the values are not equal, an `AssertionError` is thrown with a `message` property set equal to the value of the `message` parameter. If the `message` parameter is undefined, a default error message is assigned. If the `message` -parameter is an instance of an `Error` then it will be thrown instead of the +parameter is an instance of an [`Error`][] then it will be thrown instead of the `AssertionError`. ## assert.deepStrictEqual(actual, expected[, message]) @@ -136,50 +136,50 @@ changes: * `expected` {any} * `message` {any} -Identical to [`assert.deepEqual()`][] with the following exceptions: +Tests for deep equality between the `actual` and `expected` parameters. +"Deep" equality means that the enumerable "own" properties of child objects +are recursively evaluated also by the following rules. -1. Primitive values besides `NaN` are compared using the [Strict Equality - Comparison][] ( `===` ). Set and Map values, Map keys and `NaN` are compared - using the [SameValueZero][] comparison (which means they are free of the - [caveats][]). -2. [`[[Prototype]]`][prototype-spec] of objects are compared using +### Comparison details + +* Primitive values are compared using the [SameValue Comparison][], used by + [`Object.is()`][]. +* [Type tags][Object.prototype.toString()] of objects should be the same. +* [`[[Prototype]]`][prototype-spec] of objects are compared using the [Strict Equality Comparison][] too. -3. [Type tags][Object.prototype.toString()] of objects should be the same. -4. [Object wrappers][] are compared both as objects and unwrapped values. -5. `0` and `-0` are not considered equal. -6. Enumerable own [`Symbol`][] properties are compared as well. +* Only [enumerable "own" properties][] are considered. +* [`Error`][] names and messages are always compared, even if these are not + enumerable properties. +* Enumerable own [`Symbol`][] properties are compared as well. +* [Object wrappers][] are compared both as objects and unwrapped values. +* Object properties are compared unordered. +* Map keys and Set items are compared unordered. +* Recursion stops when both sides differ or both sides encounter a circular + reference. ```js const assert = require('assert'); -assert.deepEqual({ a: 1 }, { a: '1' }); -// OK, because 1 == '1' - assert.deepStrictEqual({ a: 1 }, { a: '1' }); // AssertionError: { a: 1 } deepStrictEqual { a: '1' } -// because 1 !== '1' using strict equality +// because 1 !== '1' using SameValue comparison // The following objects don't have own properties const date = new Date(); const object = {}; const fakeDate = {}; - Object.setPrototypeOf(fakeDate, Date.prototype); -assert.deepEqual(object, fakeDate); -// OK, doesn't check [[Prototype]] assert.deepStrictEqual(object, fakeDate); // AssertionError: {} deepStrictEqual Date {} // Different [[Prototype]] -assert.deepEqual(date, fakeDate); -// OK, doesn't check type tags assert.deepStrictEqual(date, fakeDate); // AssertionError: 2017-03-11T14:25:31.849Z deepStrictEqual Date {} // Different type tags assert.deepStrictEqual(NaN, NaN); -// OK, because of the SameValueZero comparison +// OK, because of the SameValue comparison assert.deepStrictEqual(new Number(1), new Number(2)); // Fails because the wrapped number is unwrapped and compared as well. @@ -202,7 +202,7 @@ assert.deepStrictEqual({ [symbol1]: 1 }, { [symbol2]: 1 }); If the values are not equal, an `AssertionError` is thrown with a `message` property set equal to the value of the `message` parameter. If the `message` parameter is undefined, a default error message is assigned. If the `message` -parameter is an instance of an `Error` then it will be thrown instead of the +parameter is an instance of an [`Error`][] then it will be thrown instead of the `AssertionError`. ## assert.doesNotThrow(block[, error][, message]) @@ -298,7 +298,7 @@ assert.equal({ a: { b: 1 } }, { a: { b: 1 } }); If the values are not equal, an `AssertionError` is thrown with a `message` property set equal to the value of the `message` parameter. If the `message` parameter is undefined, a default error message is assigned. If the `message` -parameter is an instance of an `Error` then it will be thrown instead of the +parameter is an instance of an [`Error`][] then it will be thrown instead of the `AssertionError`. ## assert.fail([message]) @@ -314,7 +314,7 @@ added: v0.1.21 Throws an `AssertionError`. If `message` is falsy, the error message is set as the values of `actual` and `expected` separated by the provided `operator`. If -the `message` parameter is an instance of an `Error` then it will be thrown +the `message` parameter is an instance of an [`Error`][] then it will be thrown instead of the `AssertionError`. If just the two `actual` and `expected` arguments are provided, `operator` will default to `'!='`. If `message` is provided only it will be used as the error message, the other arguments will be @@ -451,7 +451,7 @@ assert.notDeepEqual(obj1, obj4); If the values are deeply equal, an `AssertionError` is thrown with a `message` property set equal to the value of the `message` parameter. If the `message` parameter is undefined, a default error message is assigned. If the `message` -parameter is an instance of an `Error` then it will be thrown instead of the +parameter is an instance of an [`Error`][] then it will be thrown instead of the `AssertionError`. ## assert.notDeepStrictEqual(actual, expected[, message]) @@ -491,9 +491,6 @@ Tests for deep strict inequality. Opposite of [`assert.deepStrictEqual()`][]. ```js const assert = require('assert'); -assert.notDeepEqual({ a: 1 }, { a: '1' }); -// AssertionError: { a: 1 } notDeepEqual { a: '1' } - assert.notDeepStrictEqual({ a: 1 }, { a: '1' }); // OK ``` @@ -501,8 +498,8 @@ assert.notDeepStrictEqual({ a: 1 }, { a: '1' }); If the values are deeply and strictly equal, an `AssertionError` is thrown with a `message` property set equal to the value of the `message` parameter. If the `message` parameter is undefined, a default error message is assigned. If the -`message` parameter is an instance of an `Error` then it will be thrown instead -of the `AssertionError`. +`message` parameter is an instance of an [`Error`][] then it will be thrown +instead of the `AssertionError`. ## assert.notEqual(actual, expected[, message]) + +When using the `strict mode`, any `assert` function will use the equality used in +the strict function mode. So [`assert.deepEqual()`][] will, for example, work the +same as [`assert.deepStrictEqual()`][]. + +It can be accessed using: + +```js +const assert = require('assert').strict; +``` + +## Legacy mode + +> Stability: 0 - Deprecated: Use strict mode instead. + +When accessing `assert` directly instead of using the `strict` property, the +[Abstract Equality Comparison][] will be used for any function without a +"strict" in its name (e.g. [`assert.deepEqual()`][]). + +It can be accessed using: + +```js +const assert = require('assert'); +``` + +It is recommended to use the [`strict mode`][] instead as the +[Abstract Equality Comparison][] can often have surprising results. Especially +in case of [`assert.deepEqual()`][] as the used comparison rules there are very +lax. + +E.g. + +```js +// WARNING: This does not throw an AssertionError! +assert.deepEqual(/a/gi, new Date()); +``` + ## assert(value[, message]) ```js -// THIS IS A MISTAKE! DO NOT DO THIS! -assert.throws(myFunction, 'missing foo', 'did not throw with expected message'); - -// Do this instead. -assert.throws(myFunction, /missing foo/, 'did not throw with expected message'); +function throwingFirst() { + throw new Error('First'); +} +function throwingSecond() { + throw new Error('Second'); +} +function notThrowing() {} + +// The second argument is a string and the input function threw an Error. +// In that case both cases do not throw as neither is going to try to +// match for the error message thrown by the input function! +assert.throws(throwingFirst, 'Second'); +assert.throws(throwingSecond, 'Second'); + +// The string is only used (as message) in case the function does not throw: +assert.throws(notThrowing, 'Second'); +// AssertionError [ERR_ASSERTION]: Missing expected exception: Second + +// If it was intended to match for the error message do this instead: +assert.throws(throwingSecond, /Second$/); +// Does not throw because the error messages match. +assert.throws(throwingFirst, /Second$/); +// Throws a error: +// Error: First +// at throwingFirst (repl:2:9) ``` +Due to the confusing notation, it is recommended not to use a string as the +second argument. This might lead to difficult-to-spot errors. + ## Caveats For the following cases, consider using ES2015 [`Object.is()`][], diff --git a/lib/assert.js b/lib/assert.js index cb590ca8af0c2b..6aae1a33a428be 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -211,7 +211,11 @@ function expectedException(actual, expected) { return expected.call({}, actual) === true; } -function tryBlock(block) { +function getActual(block) { + if (typeof block !== 'function') { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'block', 'Function', + block); + } try { block(); } catch (e) { @@ -219,60 +223,61 @@ function tryBlock(block) { } } -function innerThrows(shouldThrow, block, expected, message) { - var details = ''; +// Expected to throw an error. +assert.throws = function throws(block, error, message) { + const actual = getActual(block); - if (typeof block !== 'function') { - throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'block', 'Function', - block); - } + if (typeof error === 'string') { + if (arguments.length === 3) + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', + 'error', + ['Function', 'RegExp'], + error); - if (typeof expected === 'string') { - message = expected; - expected = null; + message = error; + error = null; } - const actual = tryBlock(block); - - if (shouldThrow === true) { - if (actual === undefined) { - if (expected && expected.name) { - details += ` (${expected.name})`; - } - details += message ? `: ${message}` : '.'; - innerFail({ - actual, - expected, - operator: 'throws', - message: `Missing expected exception${details}`, - stackStartFn: assert.throws - }); - } - if (expected && expectedException(actual, expected) === false) { - throw actual; - } - } else if (actual !== undefined) { - if (!expected || expectedException(actual, expected)) { - details = message ? `: ${message}` : '.'; - innerFail({ - actual, - expected, - operator: 'doesNotThrow', - message: `Got unwanted exception${details}\n${actual.message}`, - stackStartFn: assert.doesNotThrow - }); + if (actual === undefined) { + let details = ''; + if (error && error.name) { + details += ` (${error.name})`; } + details += message ? `: ${message}` : '.'; + innerFail({ + actual, + expected: error, + operator: 'throws', + message: `Missing expected exception${details}`, + stackStartFn: throws + }); + } + if (error && expectedException(actual, error) === false) { throw actual; } -} - -// Expected to throw an error. -assert.throws = function throws(block, error, message) { - innerThrows(true, block, error, message); }; assert.doesNotThrow = function doesNotThrow(block, error, message) { - innerThrows(false, block, error, message); + const actual = getActual(block); + if (actual === undefined) + return; + + if (typeof error === 'string') { + message = error; + error = null; + } + + if (!error || expectedException(actual, error)) { + const details = message ? `: ${message}` : '.'; + innerFail({ + actual, + expected: error, + operator: 'doesNotThrow', + message: `Got unwanted exception${details}\n${actual.message}`, + stackStartFn: doesNotThrow + }); + } + throw actual; }; assert.ifError = function ifError(err) { if (err) throw err; }; diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index dd9b0906097643..c260eabe2e3e8d 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -773,3 +773,14 @@ common.expectsError( message: 'null == true' } ); + +common.expectsError( + // eslint-disable-next-line no-restricted-syntax + () => assert.throws(() => {}, 'Error message', 'message'), + { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: 'The "error" argument must be one of type Function or RegExp. ' + + 'Received type string' + } +); From f38be4193ee20a20cbe6ed29e5c7713d92e9e970 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 11 Dec 2017 05:04:17 -0200 Subject: [PATCH 07/21] doc: improve .throws RegExp info It was not clear why the error name is actually also tested for when using a regular expression. This is now clarified. PR-URL: https://github.com/nodejs/node/pull/17585 Reviewed-By: Rich Trott Reviewed-By: Benjamin Gruenbaum Reviewed-By: Matteo Collina Reviewed-By: James M Snell Reviewed-By: Gireesh Punathil Reviewed-By: Evan Lucas --- doc/api/assert.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/doc/api/assert.md b/doc/api/assert.md index 1bbb2c1f367027..5e4d0e75f53e60 100644 --- a/doc/api/assert.md +++ b/doc/api/assert.md @@ -738,12 +738,15 @@ assert.throws( Validate error message using [`RegExp`][]: +Using a regular expression runs `.toString` on the error object, and will +therefore also include the error name. + ```js assert.throws( () => { throw new Error('Wrong value'); }, - /value/ + /^Error: Wrong value$/ ); ``` From 7f558d55db79b352e3294b130621f56f23c15b63 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 9 Dec 2017 22:54:44 -0200 Subject: [PATCH 08/21] assert: .throws accept objects From now on it is possible to use a validation object in throws instead of the other possibilites. PR-URL: https://github.com/nodejs/node/pull/17584 Refs: https://github.com/nodejs/node/pull/17557 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Luigi Pinca Reviewed-By: James M Snell Reviewed-By: Matteo Collina Reviewed-By: Ron Korving Reviewed-By: Yuta Hiroto --- doc/api/assert.md | 26 +++++++++-- lib/assert.js | 43 ++++++++++++++++-- test/parallel/test-assert.js | 86 +++++++++++++++++++++++++++++++----- 3 files changed, 138 insertions(+), 17 deletions(-) diff --git a/doc/api/assert.md b/doc/api/assert.md index 5e4d0e75f53e60..d53bccd21a998d 100644 --- a/doc/api/assert.md +++ b/doc/api/assert.md @@ -709,18 +709,21 @@ instead of the `AssertionError`. * `block` {Function} -* `error` {RegExp|Function} +* `error` {RegExp|Function|object} * `message` {any} Expects the function `block` to throw an error. -If specified, `error` can be a constructor, [`RegExp`][], or validation -function. +If specified, `error` can be a constructor, [`RegExp`][], a validation +function, or an object where each property will be tested for. If specified, `message` will be the message provided by the `AssertionError` if the block fails to throw. @@ -766,6 +769,23 @@ assert.throws( ); ``` +Custom error object / error instance: + +```js +assert.throws( + () => { + const err = new TypeError('Wrong value'); + err.code = 404; + throw err; + }, + { + name: 'TypeError', + message: 'Wrong value' + // Note that only properties on the error object will be tested! + } +); +``` + Note that `error` can not be a string. If a string is provided as the second argument, then `error` is assumed to be omitted and the string will be used for `message` instead. This can lead to easy-to-miss mistakes. Please read the diff --git a/lib/assert.js b/lib/assert.js index 6aae1a33a428be..4781f170e359da 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -23,6 +23,7 @@ const { isDeepEqual, isDeepStrictEqual } = require('internal/util/comparisons'); const errors = require('internal/errors'); +const { inspect } = require('util'); // The assert module provides functions that throw // AssertionError's when particular conditions are not met. The @@ -196,10 +197,44 @@ assert.notStrictEqual = function notStrictEqual(actual, expected, message) { } }; -function expectedException(actual, expected) { +function createMsg(msg, key, actual, expected) { + if (msg) + return msg; + return `${key}: expected ${inspect(expected[key])}, ` + + `not ${inspect(actual[key])}`; +} + +function expectedException(actual, expected, msg) { if (typeof expected !== 'function') { - // Should be a RegExp, if not fail hard - return expected.test(actual); + if (expected instanceof RegExp) + return expected.test(actual); + // assert.doesNotThrow does not accept objects. + if (arguments.length === 2) { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'expected', + ['Function', 'RegExp'], expected); + } + // The name and message could be non enumerable. Therefore test them + // explicitly. + if ('name' in expected) { + assert.strictEqual( + actual.name, + expected.name, + createMsg(msg, 'name', actual, expected)); + } + if ('message' in expected) { + assert.strictEqual( + actual.message, + expected.message, + createMsg(msg, 'message', actual, expected)); + } + const keys = Object.keys(expected); + for (const key of keys) { + assert.deepStrictEqual( + actual[key], + expected[key], + createMsg(msg, key, actual, expected)); + } + return true; } // Guard instanceof against arrow functions as they don't have a prototype. if (expected.prototype !== undefined && actual instanceof expected) { @@ -252,7 +287,7 @@ assert.throws = function throws(block, error, message) { stackStartFn: throws }); } - if (error && expectedException(actual, error) === false) { + if (error && expectedException(actual, error, message) === false) { throw actual; } }; diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index c260eabe2e3e8d..949318d65bd8e2 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -39,16 +39,6 @@ assert.ok(a.AssertionError.prototype instanceof Error, assert.throws(makeBlock(a, false), a.AssertionError, 'ok(false)'); -// Using a object as second arg results in a failure -assert.throws( - () => { assert.throws(() => { throw new Error(); }, { foo: 'bar' }); }, - common.expectsError({ - type: TypeError, - message: 'expected.test is not a function' - }) -); - - assert.doesNotThrow(makeBlock(a, true), a.AssertionError, 'ok(true)'); assert.doesNotThrow(makeBlock(a, 'test', 'ok(\'test\')')); @@ -784,3 +774,79 @@ common.expectsError( 'Received type string' } ); + +{ + const errFn = () => { + const err = new TypeError('Wrong value'); + err.code = 404; + throw err; + }; + const errObj = { + name: 'TypeError', + message: 'Wrong value' + }; + assert.throws(errFn, errObj); + + errObj.code = 404; + assert.throws(errFn, errObj); + + errObj.code = '404'; + common.expectsError( + // eslint-disable-next-line no-restricted-syntax + () => assert.throws(errFn, errObj), + { + code: 'ERR_ASSERTION', + type: assert.AssertionError, + message: 'code: expected \'404\', not 404' + } + ); + + errObj.code = 404; + errObj.foo = 'bar'; + common.expectsError( + // eslint-disable-next-line no-restricted-syntax + () => assert.throws(errFn, errObj), + { + code: 'ERR_ASSERTION', + type: assert.AssertionError, + message: 'foo: expected \'bar\', not undefined' + } + ); + + common.expectsError( + () => assert.throws(() => { throw new Error(); }, { foo: 'bar' }, 'foobar'), + { + type: assert.AssertionError, + code: 'ERR_ASSERTION', + message: 'foobar' + } + ); + + common.expectsError( + () => assert.doesNotThrow(() => { throw new Error(); }, { foo: 'bar' }), + { + type: TypeError, + code: 'ERR_INVALID_ARG_TYPE', + message: 'The "expected" argument must be one of type Function or ' + + 'RegExp. Received type object' + } + ); + + assert.throws(() => { throw new Error('e'); }, new Error('e')); + common.expectsError( + () => assert.throws(() => { throw new TypeError('e'); }, new Error('e')), + { + type: assert.AssertionError, + code: 'ERR_ASSERTION', + message: "name: expected 'Error', not 'TypeError'" + } + ); + common.expectsError( + () => assert.throws(() => { throw new Error('foo'); }, new Error('')), + { + type: assert.AssertionError, + code: 'ERR_ASSERTION', + message: "message: expected '', not 'foo'" + } + ); +} From fe9f098df2b877a06dcf587dca5092997e48da25 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 28 Dec 2017 18:58:08 +0100 Subject: [PATCH 09/21] assert: fix strict regression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using `assert()` or `assert.ok()` resulted in a error since a refactoring. Refs: https://github.com/nodejs/node/pull/17582 PR-URL: https://github.com/nodejs/node/pull/17903 Refs: https://github.com/nodejs/node/pull/17582 Reviewed-By: Anatoli Papirovski Reviewed-By: Tobias Nießen Reviewed-By: Colin Ihrig Reviewed-By: James M Snell --- lib/assert.js | 10 +++++++++- test/parallel/test-assert.js | 8 ++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/assert.js b/lib/assert.js index 4781f170e359da..05daf6d05ee55a 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -319,7 +319,15 @@ assert.ifError = function ifError(err) { if (err) throw err; }; // Expose a strict only variant of assert function strict(value, message) { - if (!value) innerFail(value, true, message, '==', strict); + if (!value) { + innerFail({ + actual: value, + expected: true, + message, + operator: '==', + stackStartFn: strict + }); + } } assert.strict = Object.assign(strict, assert, { equal: assert.strictEqual, diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index 949318d65bd8e2..b2debe1d8001ac 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -753,6 +753,14 @@ common.expectsError( assert.equal(Object.keys(assert).length, Object.keys(a).length); /* eslint-enable no-restricted-properties */ assert(7); + common.expectsError( + () => assert(), + { + code: 'ERR_ASSERTION', + type: assert.AssertionError, + message: 'undefined == true' + } + ); } common.expectsError( From 49dc778ae0d6ece0f575b0a14b9f5a26d8427115 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Sun, 7 Jan 2018 23:17:29 +0100 Subject: [PATCH 10/21] lib: handle `throw undefined` in assert.throws() And make `assert.doesNotThrow()` handle it as well. PR-URL: https://github.com/nodejs/node/pull/18029 Fixes: https://github.com/nodejs/node/issues/18027 Reviewed-By: Anna Henningsen Reviewed-By: Ruben Bridgewater Reviewed-By: Khaidi Chu Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: Daniel Bevenius Reviewed-By: Colin Ihrig Reviewed-By: James M Snell --- lib/assert.js | 9 ++++++--- test/parallel/test-assert.js | 12 ++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/lib/assert.js b/lib/assert.js index 05daf6d05ee55a..c3570afeb38a3a 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -31,6 +31,8 @@ const { inspect } = require('util'); const assert = module.exports = ok; +const NO_EXCEPTION_SENTINEL = {}; + // All of the following functions must throw an AssertionError // when a corresponding condition is not met, with a message that // may be undefined if not provided. All assertion methods provide @@ -256,6 +258,7 @@ function getActual(block) { } catch (e) { return e; } + return NO_EXCEPTION_SENTINEL; } // Expected to throw an error. @@ -273,7 +276,7 @@ assert.throws = function throws(block, error, message) { error = null; } - if (actual === undefined) { + if (actual === NO_EXCEPTION_SENTINEL) { let details = ''; if (error && error.name) { details += ` (${error.name})`; @@ -294,7 +297,7 @@ assert.throws = function throws(block, error, message) { assert.doesNotThrow = function doesNotThrow(block, error, message) { const actual = getActual(block); - if (actual === undefined) + if (actual === NO_EXCEPTION_SENTINEL) return; if (typeof error === 'string') { @@ -308,7 +311,7 @@ assert.doesNotThrow = function doesNotThrow(block, error, message) { actual, expected: error, operator: 'doesNotThrow', - message: `Got unwanted exception${details}\n${actual.message}`, + message: `Got unwanted exception${details}\n${actual && actual.message}`, stackStartFn: doesNotThrow }); } diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index b2debe1d8001ac..462c01425dab5b 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -857,4 +857,16 @@ common.expectsError( message: "message: expected '', not 'foo'" } ); + + // eslint-disable-next-line no-throw-literal + assert.throws(() => { throw undefined; }, /undefined/); + common.expectsError( + // eslint-disable-next-line no-throw-literal + () => assert.doesNotThrow(() => { throw undefined; }), + { + type: assert.AssertionError, + code: 'ERR_ASSERTION', + message: 'Got unwanted exception.\nundefined' + } + ); } From 5d8855739d8cc150c7163993773920acadd60055 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 9 Dec 2017 05:34:16 -0200 Subject: [PATCH 11/21] util: fix custom inspect description Using a custom inspect function on the inspected object is deprecated. Remove the reference from the option description to make sure the user will read about the deprecation in the more detailed description. PR-URL: https://github.com/nodejs/node/pull/17576 Reviewed-By: Anna Henningsen --- doc/api/util.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/doc/api/util.md b/doc/api/util.md index 976621ddb383e3..a001124824f0a5 100644 --- a/doc/api/util.md +++ b/doc/api/util.md @@ -342,8 +342,7 @@ changes: codes. Defaults to `false`. Colors are customizable, see [Customizing `util.inspect` colors][]. * `customInspect` {boolean} If `false`, then custom `inspect(depth, opts)` - functions exported on the `object` being inspected will not be called. - Defaults to `true`. + functions will not be called. Defaults to `true`. * `showProxy` {boolean} If `true`, then objects and functions that are `Proxy` objects will be introspected to show their `target` and `handler` objects. Defaults to `false`. From f77f91d6f93857e991ff92f679c38886887f4f1d Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 9 Dec 2017 05:33:00 -0200 Subject: [PATCH 12/21] util: rename util.inspect argument util.inspect can actually receive any property and the description was wrong so far. This fixes it by renaming the argument to value and also updating the description. PR-URL: https://github.com/nodejs/node/pull/17576 Reviewed-By: Anna Henningsen --- lib/util.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/util.js b/lib/util.js index cd6321cfe5270e..bb805a082b27d6 100644 --- a/lib/util.js +++ b/lib/util.js @@ -257,14 +257,14 @@ function debuglog(set) { } /** - * Echos the value of a value. Tries to print the value out + * Echos the value of any input. Tries to print the value out * in the best way possible given the different types. * - * @param {Object} obj The object to print out. + * @param {any} value The value to print out. * @param {Object} opts Optional options object that alters the output. */ -/* Legacy: obj, showHidden, depth, colors*/ -function inspect(obj, opts) { +/* Legacy: value, showHidden, depth, colors*/ +function inspect(value, opts) { // Default options const ctx = { seen: [], @@ -298,7 +298,7 @@ function inspect(obj, opts) { } if (ctx.colors) ctx.stylize = stylizeWithColor; if (ctx.maxArrayLength === null) ctx.maxArrayLength = Infinity; - return formatValue(ctx, obj, ctx.depth); + return formatValue(ctx, value, ctx.depth); } inspect.custom = customInspectSymbol; From fb85597aa4efdadf7bd94c67475875e7ac1f942e Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 9 Dec 2017 05:27:21 -0200 Subject: [PATCH 13/21] util: add util.inspect compact option The current default formatting is not ideal and this improves the situation by formatting the output more intuitiv. 1) All object keys are now indented by 2 characters instead of sometimes 2 and sometimes 3 characters. 2) Each object key will now use an individual line instead of sharing a line potentially with multiple object keys. 3) Long strings will now be split into multiple lines in case they exceed the "lineBreak" option length (including the current indentation). 4) Opening braces are now directly behind a object property instead of using a new line. 5) Switch inspect "base" order. In case the compact option is set to `false`, inspect will now print "[Function: foo] {\n property: 'data'\n}" instead of "{ [Function: foo]\n property: 'data'\n}". PR-URL: https://github.com/nodejs/node/pull/17576 Reviewed-By: Anna Henningsen --- doc/api/util.md | 67 ++++++++++++++ lib/util.js | 84 +++++++++++++---- test/parallel/test-util-inspect.js | 143 +++++++++++++++++++++++++++++ 3 files changed, 277 insertions(+), 17 deletions(-) diff --git a/doc/api/util.md b/doc/api/util.md index a001124824f0a5..bc1d3ae262ae50 100644 --- a/doc/api/util.md +++ b/doc/api/util.md @@ -316,6 +316,9 @@ stream.write('With ES6'); diff --git a/lib/util.js b/lib/util.js index bb805a082b27d6..70fd1a05564389 100644 --- a/lib/util.js +++ b/lib/util.js @@ -68,7 +68,8 @@ const inspectDefaultOptions = Object.seal({ customInspect: true, showProxy: false, maxArrayLength: 100, - breakLength: 60 + breakLength: 60, + compact: true }); const propertyIsEnumerable = Object.prototype.propertyIsEnumerable; @@ -86,6 +87,10 @@ const strEscapeSequencesReplacer = /[\x00-\x1f\x27\x5c]/g; const keyStrRegExp = /^[a-zA-Z_][a-zA-Z_0-9]*$/; const numberRegExp = /^(0|[1-9][0-9]*)$/; +const readableRegExps = {}; + +const MIN_LINE_LENGTH = 16; + // Escaped special characters. Use empty strings to fill up unused entries. const meta = [ '\\u0000', '\\u0001', '\\u0002', '\\u0003', '\\u0004', @@ -276,7 +281,8 @@ function inspect(value, opts) { showProxy: inspectDefaultOptions.showProxy, maxArrayLength: inspectDefaultOptions.maxArrayLength, breakLength: inspectDefaultOptions.breakLength, - indentationLvl: 0 + indentationLvl: 0, + compact: inspectDefaultOptions.compact }; // Legacy... if (arguments.length > 2) { @@ -374,7 +380,7 @@ function ensureDebugIsInitialized() { function formatValue(ctx, value, recurseTimes, ln) { // Primitive types cannot have properties if (typeof value !== 'object' && typeof value !== 'function') { - return formatPrimitive(ctx.stylize, value); + return formatPrimitive(ctx.stylize, value, ctx); } if (value === null) { return ctx.stylize('null', 'null'); @@ -481,10 +487,10 @@ function formatValue(ctx, value, recurseTimes, ln) { } catch (e) { /* ignore */ } if (typeof raw === 'string') { - const formatted = formatPrimitive(stylizeNoColor, raw); + const formatted = formatPrimitive(stylizeNoColor, raw, ctx); if (keyLength === raw.length) return ctx.stylize(`[String: ${formatted}]`, 'string'); - base = ` [String: ${formatted}]`; + base = `[String: ${formatted}]`; // For boxed Strings, we have to remove the 0-n indexed entries, // since they just noisy up the output and are redundant // Make boxed primitive Strings look like such @@ -505,12 +511,12 @@ function formatValue(ctx, value, recurseTimes, ln) { const name = `${constructor.name}${value.name ? `: ${value.name}` : ''}`; if (keyLength === 0) return ctx.stylize(`[${name}]`, 'special'); - base = ` [${name}]`; + base = `[${name}]`; } else if (isRegExp(value)) { // Make RegExps say that they are RegExps if (keyLength === 0 || recurseTimes < 0) return ctx.stylize(regExpToString.call(value), 'regexp'); - base = ` ${regExpToString.call(value)}`; + base = `${regExpToString.call(value)}`; } else if (isDate(value)) { if (keyLength === 0) { if (Number.isNaN(value.getTime())) @@ -518,12 +524,12 @@ function formatValue(ctx, value, recurseTimes, ln) { return ctx.stylize(dateToISOString.call(value), 'date'); } // Make dates with properties first say the date - base = ` ${dateToISOString.call(value)}`; + base = `${dateToISOString.call(value)}`; } else if (isError(value)) { // Make error with message first say the error if (keyLength === 0) return formatError(value); - base = ` ${formatError(value)}`; + base = `${formatError(value)}`; } else if (isAnyArrayBuffer(value)) { // Fast path for ArrayBuffer and SharedArrayBuffer. // Can't do the same for DataView because it has a non-primitive @@ -553,13 +559,13 @@ function formatValue(ctx, value, recurseTimes, ln) { const formatted = formatPrimitive(stylizeNoColor, raw); if (keyLength === 0) return ctx.stylize(`[Number: ${formatted}]`, 'number'); - base = ` [Number: ${formatted}]`; + base = `[Number: ${formatted}]`; } else if (typeof raw === 'boolean') { // Make boxed primitive Booleans look like such const formatted = formatPrimitive(stylizeNoColor, raw); if (keyLength === 0) return ctx.stylize(`[Boolean: ${formatted}]`, 'boolean'); - base = ` [Boolean: ${formatted}]`; + base = `[Boolean: ${formatted}]`; } else if (typeof raw === 'symbol') { const formatted = formatPrimitive(stylizeNoColor, raw); return ctx.stylize(`[Symbol: ${formatted}]`, 'symbol'); @@ -603,9 +609,42 @@ function formatNumber(fn, value) { return fn(`${value}`, 'number'); } -function formatPrimitive(fn, value) { - if (typeof value === 'string') +function formatPrimitive(fn, value, ctx) { + if (typeof value === 'string') { + if (ctx.compact === false && + value.length > MIN_LINE_LENGTH && + ctx.indentationLvl + value.length > ctx.breakLength) { + // eslint-disable-next-line max-len + const minLineLength = Math.max(ctx.breakLength - ctx.indentationLvl, MIN_LINE_LENGTH); + // eslint-disable-next-line max-len + const averageLineLength = Math.ceil(value.length / Math.ceil(value.length / minLineLength)); + const divisor = Math.max(averageLineLength, MIN_LINE_LENGTH); + var res = ''; + if (readableRegExps[divisor] === undefined) { + // Build a new RegExp that naturally breaks text into multiple lines. + // + // Rules + // 1. Greedy match all text up the max line length that ends with a + // whitespace or the end of the string. + // 2. If none matches, non-greedy match any text up to a whitespace or + // the end of the string. + // + // eslint-disable-next-line max-len, no-unescaped-regexp-dot + readableRegExps[divisor] = new RegExp(`(.|\\n){1,${divisor}}(\\s|$)|(\\n|.)+?(\\s|$)`, 'gm'); + } + const indent = ' '.repeat(ctx.indentationLvl); + const matches = value.match(readableRegExps[divisor]); + if (matches.length > 1) { + res += `${fn(strEscape(matches[0]), 'string')} +\n`; + for (var i = 1; i < matches.length - 1; i++) { + res += `${indent} ${fn(strEscape(matches[i]), 'string')} +\n`; + } + res += `${indent} ${fn(strEscape(matches[i]), 'string')}`; + return res; + } + } return fn(strEscape(value), 'string'); + } if (typeof value === 'number') return formatNumber(fn, value); if (typeof value === 'boolean') @@ -806,7 +845,7 @@ function formatProperty(ctx, value, recurseTimes, key, array) { const desc = Object.getOwnPropertyDescriptor(value, key) || { value: value[key], enumerable: true }; if (desc.value !== undefined) { - const diff = array === 0 ? 3 : 2; + const diff = array !== 0 || ctx.compact === false ? 2 : 3; ctx.indentationLvl += diff; str = formatValue(ctx, desc.value, recurseTimes, array === 0); ctx.indentationLvl -= diff; @@ -839,9 +878,19 @@ function formatProperty(ctx, value, recurseTimes, key, array) { function reduceToSingleString(ctx, output, base, braces, addLn) { const breakLength = ctx.breakLength; + var i = 0; + if (ctx.compact === false) { + const indentation = ' '.repeat(ctx.indentationLvl); + var res = `${base ? `${base} ` : ''}${braces[0]}\n${indentation} `; + for (; i < output.length - 1; i++) { + res += `${output[i]},\n${indentation} `; + } + res += `${output[i]}\n${indentation}${braces[1]}`; + return res; + } if (output.length * 2 <= breakLength) { var length = 0; - for (var i = 0; i < output.length && length <= breakLength; i++) { + for (; i < output.length && length <= breakLength; i++) { if (ctx.colors) { length += removeColors(output[i]).length + 1; } else { @@ -849,7 +898,8 @@ function reduceToSingleString(ctx, output, base, braces, addLn) { } } if (length <= breakLength) - return `${braces[0]}${base} ${join(output, ', ')} ${braces[1]}`; + return `${braces[0]}${base ? ` ${base}` : ''} ${join(output, ', ')} ` + + braces[1]; } // If the opening "brace" is too large, like in the case of "Set {", // we need to force the first item to be on the next line or the @@ -857,7 +907,7 @@ function reduceToSingleString(ctx, output, base, braces, addLn) { const indentation = ' '.repeat(ctx.indentationLvl); const extraLn = addLn === true ? `\n${indentation}` : ''; const ln = base === '' && braces[0].length === 1 ? - ' ' : `${base}\n${indentation} `; + ' ' : `${base ? ` ${base}` : base}\n${indentation} `; const str = join(output, `,\n${indentation} `); return `${extraLn}${braces[0]}${ln}${str} ${braces[1]}`; } diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js index a99583d454b3f4..83f6b469d68055 100644 --- a/test/parallel/test-util-inspect.js +++ b/test/parallel/test-util-inspect.js @@ -1160,3 +1160,146 @@ if (typeof Symbol !== 'undefined') { assert.doesNotThrow(() => util.inspect(process)); /* eslint-enable accessor-pairs */ + +// Setting custom inspect property to a non-function should do nothing. +{ + const obj = { inspect: 'fhqwhgads' }; + assert.strictEqual(util.inspect(obj), "{ inspect: 'fhqwhgads' }"); +} + +{ + const o = { + a: [1, 2, [[ + 'Lorem ipsum dolor\nsit amet,\tconsectetur adipiscing elit, sed do ' + + 'eiusmod tempor incididunt ut labore et dolore magna aliqua.', + 'test', + 'foo']], 4], + b: new Map([['za', 1], ['zb', 'test']]) + }; + + let out = util.inspect(o, { compact: true, depth: 5, breakLength: 80 }); + let expect = [ + '{ a: ', + ' [ 1,', + ' 2,', + " [ [ 'Lorem ipsum dolor\\nsit amet,\\tconsectetur adipiscing elit, " + + "sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.',", + " 'test',", + " 'foo' ] ],", + ' 4 ],', + " b: Map { 'za' => 1, 'zb' => 'test' } }", + ].join('\n'); + assert.strictEqual(out, expect); + + out = util.inspect(o, { compact: false, depth: 5, breakLength: 60 }); + expect = [ + '{', + ' a: [', + ' 1,', + ' 2,', + ' [', + ' [', + ' \'Lorem ipsum dolor\\nsit amet,\\tconsectetur \' +', + ' \'adipiscing elit, sed do eiusmod tempor \' +', + ' \'incididunt ut labore et dolore magna \' +', + ' \'aliqua.\',', + ' \'test\',', + ' \'foo\'', + ' ]', + ' ],', + ' 4', + ' ],', + ' b: Map {', + ' \'za\' => 1,', + ' \'zb\' => \'test\'', + ' }', + '}' + ].join('\n'); + assert.strictEqual(out, expect); + + out = util.inspect(o.a[2][0][0], { compact: false, breakLength: 30 }); + expect = [ + '\'Lorem ipsum dolor\\nsit \' +', + ' \'amet,\\tconsectetur \' +', + ' \'adipiscing elit, sed do \' +', + ' \'eiusmod tempor incididunt \' +', + ' \'ut labore et dolore magna \' +', + ' \'aliqua.\'' + ].join('\n'); + assert.strictEqual(out, expect); + + out = util.inspect( + '12345678901234567890123456789012345678901234567890', + { compact: false, breakLength: 3 }); + expect = '\'12345678901234567890123456789012345678901234567890\''; + assert.strictEqual(out, expect); + + out = util.inspect( + '12 45 78 01 34 67 90 23 56 89 123456789012345678901234567890', + { compact: false, breakLength: 3 }); + expect = [ + '\'12 45 78 01 34 \' +', + ' \'67 90 23 56 89 \' +', + ' \'123456789012345678901234567890\'' + ].join('\n'); + assert.strictEqual(out, expect); + + out = util.inspect( + '12 45 78 01 34 67 90 23 56 89 1234567890123 0', + { compact: false, breakLength: 3 }); + expect = [ + '\'12 45 78 01 34 \' +', + ' \'67 90 23 56 89 \' +', + ' \'1234567890123 0\'' + ].join('\n'); + assert.strictEqual(out, expect); + + out = util.inspect( + '12 45 78 01 34 67 90 23 56 89 12345678901234567 0', + { compact: false, breakLength: 3 }); + expect = [ + '\'12 45 78 01 34 \' +', + ' \'67 90 23 56 89 \' +', + ' \'12345678901234567 \' +', + ' \'0\'' + ].join('\n'); + assert.strictEqual(out, expect); + + o.a = () => {}; + o.b = new Number(3); + out = util.inspect(o, { compact: false, breakLength: 3 }); + expect = [ + '{', + ' a: [Function],', + ' b: [Number: 3]', + '}' + ].join('\n'); + assert.strictEqual(out, expect); + + out = util.inspect(o, { compact: false, breakLength: 3, showHidden: true }); + expect = [ + '{', + ' a: [Function] {', + ' [length]: 0,', + " [name]: ''", + ' },', + ' b: [Number: 3]', + '}' + ].join('\n'); + assert.strictEqual(out, expect); + + o[util.inspect.custom] = () => 42; + out = util.inspect(o, { compact: false, breakLength: 3 }); + expect = '42'; + assert.strictEqual(out, expect); + + o[util.inspect.custom] = () => '12 45 78 01 34 67 90 23'; + out = util.inspect(o, { compact: false, breakLength: 3 }); + expect = '12 45 78 01 34 67 90 23'; + assert.strictEqual(out, expect); + + o[util.inspect.custom] = () => ({ a: '12 45 78 01 34 67 90 23' }); + out = util.inspect(o, { compact: false, breakLength: 3 }); + expect = '{\n a: \'12 45 78 01 34 \' +\n \'67 90 23\'\n}'; + assert.strictEqual(out, expect); +} From 70ac31896bda9f47225ec7d029c09b0fc3915879 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 9 Dec 2017 19:38:20 -0200 Subject: [PATCH 14/21] assert: improve error messages From now on all error messages produced by `assert` in strict mode will produce a error diff. PR-URL: https://github.com/nodejs/node/pull/17615 Reviewed-By: James M Snell Reviewed-By: Matteo Collina --- doc/api/assert.md | 33 ++++++++ lib/assert.js | 16 +++- lib/internal/errors.js | 150 ++++++++++++++++++++++++++++++++++- test/parallel/test-assert.js | 143 ++++++++++++++++++++++++++++++++- 4 files changed, 332 insertions(+), 10 deletions(-) diff --git a/doc/api/assert.md b/doc/api/assert.md index d53bccd21a998d..68b01350dfdee6 100644 --- a/doc/api/assert.md +++ b/doc/api/assert.md @@ -17,6 +17,9 @@ For more information about the used equality comparisons see + +* `env` {object} A object containing the environment variables to check. + Defaults to `process.env`. +* Returns: {number} + +Returns: +* 1 for 2, +* 4 for 16, +* 8 for 256, +* 24 for 16,777,216 +colors supported. + +Use this to determine what colors the terminal supports. Due to the nature of +colors in terminals it is possible to either have false positives or false +negatives. It depends on process information and the environment variables that +may lie about what terminal is used. +To enforce a specific behavior without relying on `process.env` it is possible +to pass in an object with different settings. + +Use the `NODE_DISABLE_COLORS` environment variable to enforce this function to +always return 1. + ## tty.isatty(fd)