From 2160a77f798d071c6eec0409423c8bf0cb69036b Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 3 Sep 2018 15:15:03 +0200 Subject: [PATCH 1/2] assert: combine auto generated message with user message So far it was difficult to tell what the actual error was about if there was only the user error. To overcome this, the user message and the auto-generated message will now be visible at the same time. --- doc/api/assert.md | 155 +++++++++------ doc/api/deprecations.md | 10 + lib/assert.js | 80 ++++---- lib/internal/assert.js | 228 ++++++++++++++--------- test/message/assert_throws_stack.out | 4 +- test/parallel/test-assert-if-error.js | 2 + test/parallel/test-assert.js | 47 ++++- test/parallel/test-stream-inheritance.js | 9 +- 8 files changed, 339 insertions(+), 196 deletions(-) diff --git a/doc/api/assert.md b/doc/api/assert.md index 149c4161ec2bb9..758e6a2233bc72 100644 --- a/doc/api/assert.md +++ b/doc/api/assert.md @@ -21,10 +21,23 @@ thrown by the `assert` module will be instances of the `AssertionError` class. ### new assert.AssertionError(options) * `options` {Object} - * `message` {string} If provided, the error message is going to be set to this - value. + * `message` {any} If provided and not `undefined`, it is going to be + appended to auto-generated message and the `userMessage` will be set to this + value. If the `message` is not of type string, `util.inspect()` is called + upon that value before being appended. If no message is auto-generated, the + error message will be set to this value. * `actual` {any} The `actual` property on the error instance is going to contain this value. Internally used for the `actual` error input in case e.g., [`assert.strictEqual()`] is used. @@ -46,11 +59,17 @@ and: [`assert.strictEqual()`] is used. * `expected` {any} Set to the expected value in case e.g., [`assert.strictEqual()`] is used. -* `generatedMessage` {boolean} Indicates if the message was auto-generated - (`true`) or not. * `code` {string} This is always set to the string `ERR_ASSERTION` to indicate that the error is actually an assertion error. +* `generatedMessage` {boolean} Deprecated. Indicates if the message was + auto-generated (`true`) or not. * `operator` {string} Set to the passed in operator value. +* `userMessage` {any} Contains the actual passed through message by the user. + It will not be set in case the user does not provide a error message. + +All error messages thrown by the `assert` module are auto-generated. If the user +provides an individual error message, it will be added to the auto-generated one +to provide extra feedback to the user. ```js const assert = require('assert'); @@ -74,6 +93,7 @@ try { assert.strictEqual(err.code, 'ERR_ASSERTION'); assert.strictEqual(err.operator, 'strictEqual'); assert.strictEqual(err.generatedMessage, true); + assert.strictEuqal('userMessage' in err, false); } ``` @@ -155,7 +175,7 @@ assert.deepEqual(/a/gi, new Date()); added: v0.5.9 --> * `value` {any} The input that is checked for being truthy. -* `message` {string|Error} +* `message` {any} An alias of [`assert.ok()`][]. @@ -163,6 +183,9 @@ An alias of [`assert.ok()`][]. * `actual` {any} * `expected` {any} -* `message` {string|Error} +* `message` {any} **Strict mode** @@ -249,16 +272,17 @@ assert.deepEqual(obj1, obj4); // AssertionError: { a: { b: 1 } } deepEqual {} ``` -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 -`AssertionError`. +If the values are not deep-equal and the `message` parameter is set to an +instance of an [`Error`][] then that error will be thrown. In all other cases an +`AssertionError` is thrown, passing through the `message` parameter. ## assert.deepStrictEqual(actual, expected[, message]) * `actual` {any} * `expected` {any} -* `message` {string|Error} +* `message` {any} Tests for deep equality between the `actual` and `expected` parameters. "Deep" equality means that the enumerable "own" properties of child objects @@ -402,11 +426,9 @@ assert.deepStrictEqual(weakMap1, weakMap3); // } ``` -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 -`AssertionError`. +If the values are not strictly deep-equal and the `message` parameter is set to +an instance of an [`Error`][] then that error will be thrown. In all other cases +an `AssertionError` is thrown, passing through the `message` parameter. ## assert.doesNotReject(block[, error][, message]) * `actual` {any} * `expected` {any} -* `message` {string|Error} +* `message` {any} **Strict mode** @@ -563,11 +589,9 @@ assert.equal({ a: { b: 1 } }, { a: { b: 1 } }); // AssertionError: { 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 -`AssertionError`. +If the values are not equal and the `message` parameter is set to an instance of +an [`Error`][] then that error will be thrown. In all other cases an +`AssertionError` is thrown, passing through the `message` parameter. ## assert.fail([message]) * `actual` {any} * `expected` {any} -* `message` {string|Error} +* `message` {any} **Strict mode** @@ -770,16 +797,17 @@ assert.notDeepEqual(obj1, obj4); // OK ``` -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 -`AssertionError`. +If the values are deeply equal and the `message` parameter is set to an instance +of an [`Error`][] then that error will be thrown. In all other cases an +`AssertionError` is thrown, passing through the `message` parameter. ## assert.notDeepStrictEqual(actual, expected[, message]) * `actual` {any} * `expected` {any} -* `message` {string|Error} +* `message` {any} Tests for deep strict inequality. Opposite of [`assert.deepStrictEqual()`][]. @@ -817,19 +845,21 @@ assert.notDeepStrictEqual({ a: 1 }, { a: '1' }); // OK ``` -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`. +If the values are strictly deep-equal and the `message` parameter is set to an +instance of an [`Error`][] then that error will be thrown. In all other cases an +`AssertionError` is thrown, passing through the `message` parameter. ## assert.notEqual(actual, expected[, message]) * `actual` {any} * `expected` {any} -* `message` {string|Error} +* `message` {any} **Strict mode** @@ -855,23 +885,24 @@ assert.notEqual(1, '1'); // AssertionError: 1 != '1' ``` -If the values are 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`. +If the values are equal and the `message` parameter is set to an instance of an +[`Error`][] then that error will be thrown. In all other cases an +`AssertionError` is thrown, passing through the `message` parameter. ## assert.notStrictEqual(actual, expected[, message]) * `actual` {any} * `expected` {any} -* `message` {string|Error} +* `message` {any} Tests strict inequality between the `actual` and `expected` parameters as determined by the [SameValue Comparison][]. @@ -891,32 +922,32 @@ assert.notStrictEqual(1, '1'); // OK ``` -If the values are 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`. +If the values are strictly equal and the `message` parameter is set to an +instance of an [`Error`][] then that error will be thrown. In all other cases an +`AssertionError` is thrown, passing through the `message` parameter. ## assert.ok(value[, message]) * `value` {any} -* `message` {string|Error} +* `message` {any} Tests if `value` is truthy. It is equivalent to `assert.equal(!!value, true, message)`. -If `value` is not truthy, 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`. +If `value` is not truthy and the `message` parameter is set to an instance of an +[`Error`][] then that error will be thrown. In all other cases an +`AssertionError` is thrown, passing through the `message` parameter. + If no arguments are passed in at all `message` will be set to the string: ``'No value argument passed to `assert.ok()`'``. @@ -990,8 +1021,9 @@ an object where each property will be tested for, or an instance of error where each property will be tested for including the non-enumerable `message` and `name` properties. -If specified, `message` will be the message provided by the `AssertionError` if -the block fails to reject. +If specified, `message` will be appended to the message provided by the +`AssertionError`, if the block call fails to reject or in case the error +validation fails. ```js (async () => { @@ -1026,13 +1058,16 @@ argument gets considered. * `actual` {any} * `expected` {any} -* `message` {string|Error} +* `message` {any} Tests strict equality between the `actual` and `expected` parameters as determined by the [SameValue Comparison][]. @@ -1057,11 +1092,9 @@ assert.strictEqual('Hello foobar', 'Hello World!'); // ^ ``` -If the values are not 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`. +If the values are not strictly equal and the `message` parameter is set to an +instance of an [`Error`][] then that error will be thrown. In all other cases an +`AssertionError` is thrown, passing through the `message` parameter. ## assert.throws(block[, error][, message]) * `options` {Object} - * `message` {any} If provided and not `undefined`, it is going to be - appended to auto-generated message and the `userMessage` will be set to this - value. If the `message` is not of type string, `util.inspect()` is called - upon that value before being appended. If no message is auto-generated, the - error message will be set to this value. + * `message` {any} If provided, it is going to be appended to auto-generated + message and the `userMessage` will be set to this value. If the `message` is + not of type string, `util.inspect()` is called upon that value before being + appended. If no message is auto-generated, the error message will be set to + this value. * `actual` {any} The `actual` property on the error instance is going to contain this value. Internally used for the `actual` error input in case e.g., [`assert.strictEqual()`] is used. diff --git a/lib/assert.js b/lib/assert.js index 6a52014b8cd782..8b4b3d77501224 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -340,13 +340,16 @@ function innerOk(fn, argLen, value, message) { } } - const err = new AssertionError({ + const errArgs = { actual: value, expected: true, - message, operator: '==', stackStartFn: fn - }); + }; + if (argLen >= 2) { + errArgs.message = message; + } + const err = new AssertionError(errArgs); err.generatedMessage = generatedMessage; if (newMessage) { err.message = newMessage; @@ -367,13 +370,16 @@ assert.ok = ok; assert.equal = function equal(actual, expected, message) { // eslint-disable-next-line eqeqeq if (actual != expected) { - innerFail({ + const errArgs = { actual, expected, - message, operator: '==', stackStartFn: equal - }); + }; + if (arguments.length >= 3) { + errArgs.message = message; + } + innerFail(errArgs); } }; @@ -382,13 +388,16 @@ assert.equal = function equal(actual, expected, message) { assert.notEqual = function notEqual(actual, expected, message) { // eslint-disable-next-line eqeqeq if (actual == expected) { - innerFail({ + const errArgs = { actual, expected, - message, operator: '!=', stackStartFn: notEqual - }); + }; + if (arguments.length >= 3) { + errArgs.message = message; + } + innerFail(errArgs); } }; @@ -396,13 +405,16 @@ assert.notEqual = function notEqual(actual, expected, message) { assert.deepEqual = function deepEqual(actual, expected, message) { if (isDeepEqual === undefined) lazyLoadComparison(); if (!isDeepEqual(actual, expected)) { - innerFail({ + const errArgs = { actual, expected, - message, operator: 'deepEqual', stackStartFn: deepEqual - }); + }; + if (arguments.length >= 3) { + errArgs.message = message; + } + innerFail(errArgs); } }; @@ -410,13 +422,16 @@ assert.deepEqual = function deepEqual(actual, expected, message) { assert.notDeepEqual = function notDeepEqual(actual, expected, message) { if (isDeepEqual === undefined) lazyLoadComparison(); if (isDeepEqual(actual, expected)) { - innerFail({ + const errArgs = { actual, expected, - message, operator: 'notDeepEqual', stackStartFn: notDeepEqual - }); + }; + if (arguments.length >= 3) { + errArgs.message = message; + } + innerFail(errArgs); } }; /* eslint-enable */ @@ -424,13 +439,16 @@ assert.notDeepEqual = function notDeepEqual(actual, expected, message) { assert.deepStrictEqual = function deepStrictEqual(actual, expected, message) { if (isDeepEqual === undefined) lazyLoadComparison(); if (!isDeepStrictEqual(actual, expected)) { - innerFail({ + const errArgs = { actual, expected, - message, operator: 'deepStrictEqual', stackStartFn: deepStrictEqual - }); + }; + if (arguments.length >= 3) { + errArgs.message = message; + } + innerFail(errArgs); } }; @@ -438,37 +456,46 @@ assert.notDeepStrictEqual = notDeepStrictEqual; function notDeepStrictEqual(actual, expected, message) { if (isDeepEqual === undefined) lazyLoadComparison(); if (isDeepStrictEqual(actual, expected)) { - innerFail({ + const errArgs = { actual, expected, - message, operator: 'notDeepStrictEqual', stackStartFn: notDeepStrictEqual - }); + }; + if (arguments.length >= 3) { + errArgs.message = message; + } + innerFail(errArgs); } } assert.strictEqual = function strictEqual(actual, expected, message) { if (!Object.is(actual, expected)) { - innerFail({ + const errArgs = { actual, expected, - message, operator: 'strictEqual', stackStartFn: strictEqual - }); + }; + if (arguments.length >= 3) { + errArgs.message = message; + } + innerFail(errArgs); } }; assert.notStrictEqual = function notStrictEqual(actual, expected, message) { if (Object.is(actual, expected)) { - innerFail({ + const errArgs = { actual, expected, - message, operator: 'notStrictEqual', stackStartFn: notStrictEqual - }); + }; + if (arguments.length >= 3) { + errArgs.message = message; + } + innerFail(errArgs); } }; @@ -495,13 +522,16 @@ function compareExceptionKey(actual, expected, key, message, keys) { const a = new Comparison(actual, keys); const b = new Comparison(expected, keys, actual); - const err = new AssertionError({ + const errArgs = { actual: a, expected: b, - message, operator: 'deepStrictEqual', stackStartFn: assert.throws - }); + }; + if (message !== undefined) { + errArgs.message = message; + } + const err = new AssertionError(errArgs); err.actual = actual; err.expected = expected; err.operator = 'throws'; @@ -522,13 +552,16 @@ function expectedException(actual, expected, message) { // Handle primitives properly. if (typeof actual !== 'object' || actual === null) { - const err = new AssertionError({ + const errArgs = { actual, expected, - message, operator: 'deepStrictEqual', stackStartFn: assert.throws - }); + }; + if (message !== undefined) { + errArgs.message = message; + } + const err = new AssertionError(errArgs); err.operator = 'throws'; throw err; } diff --git a/lib/internal/assert.js b/lib/internal/assert.js index 4caf33cc7aa792..1107b66f84e549 100644 --- a/lib/internal/assert.js +++ b/lib/internal/assert.js @@ -324,7 +324,7 @@ class AssertionError extends Error { let userMessage = false; - if (message !== undefined) { + if ('message' in options) { userMessage = true; let isStringMessage = true; diff --git a/test/parallel/test-assert-deep.js b/test/parallel/test-assert-deep.js index e857ddf50b49f1..e9a44768fd796d 100644 --- a/test/parallel/test-assert-deep.js +++ b/test/parallel/test-assert-deep.js @@ -871,10 +871,11 @@ assert.deepStrictEqual(obj1, obj2); b.foo = 'baz'; assert.throws( - () => assert.deepStrictEqual(a, b), + () => assert.deepStrictEqual(a, b, undefined), { message: `${defaultMsgStartFull}\n\n` + - ' [TypeError: foo] {\n+ foo: \'bar\'\n- foo: \'baz\'\n }' + ' [TypeError: foo] {\n+ foo: \'bar\'\n- foo: \'baz\'\n }' + + '\n\nValue passed through as message:\n\nundefined' } ); } diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index fbc8ae27216fcf..a1f7da9c885514 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -301,15 +301,14 @@ testAssertionMessage({ a: NaN, b: Infinity, c: -Infinity }, '{\n+ a: NaN,\n+ b: Infinity,\n+ c: -Infinity\n+ }'); // https://github.com/nodejs/node-v0.x-archive/issues/5292 -try { - assert.strictEqual(1, 2); -} catch (e) { - assert.strictEqual( - e.message, - `${strictEqualMessageStart}\n1 !== 2\n` - ); - assert.ok(e.generatedMessage, 'Message not marked as generated'); -} +assert.throws( + () => assert.strictEqual(1, 2, undefined), + { + message: `${strictEqualMessageStart}\n1 !== 2\n\n` + + 'Value passed through as message:\n\nundefined', + generatedMessage: true + } +); try { assert.strictEqual(1, 2, 'oh no');