From a1c96f8e07115d3b6ff1498e7e9f530d2e0f1b6b Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 20 Mar 2018 01:49:29 +0100 Subject: [PATCH] assert: improve assert.throws This switches the assert.throws output to the one used in strict mode if a error object is used for comparison. From now on it will show the complete difference between two objects instead of only showing the first failing property. It also fixes detecting properties with a undefined value and fails in case the thrown error does not contain the value at all. PR-URL: https://github.com/nodejs/node/pull/19463 Reviewed-By: Matteo Collina Reviewed-By: James M Snell --- lib/assert.js | 53 +++++++++++++------ test/message/assert_throws_stack.out | 8 ++- test/parallel/test-assert-fail-deprecation.js | 6 +-- test/parallel/test-assert-fail.js | 5 +- test/parallel/test-assert.js | 53 ++++++++++++------- 5 files changed, 80 insertions(+), 45 deletions(-) diff --git a/lib/assert.js b/lib/assert.js index 35132bfb03f..2fc3cf33e80 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -366,13 +366,38 @@ assert.notStrictEqual = function notStrictEqual(actual, expected, message) { } }; -function compareExceptionKey(actual, expected, key, msg) { - if (!isDeepStrictEqual(actual[key], expected[key])) { +class Comparison { + constructor(obj, keys) { + for (const key of keys) { + if (key in obj) + this[key] = obj[key]; + } + } +} + +function compareExceptionKey(actual, expected, key, message, keys) { + if (!(key in actual) || !isDeepStrictEqual(actual[key], expected[key])) { + if (!message) { + // Create placeholder objects to create a nice output. + const a = new Comparison(actual, keys); + const b = new Comparison(expected, keys); + + const tmpLimit = Error.stackTraceLimit; + Error.stackTraceLimit = 0; + const err = new AssertionError({ + actual: a, + expected: b, + operator: 'deepStrictEqual', + stackStartFn: assert.throws, + errorDiff: ERR_DIFF_EQUAL + }); + Error.stackTraceLimit = tmpLimit; + message = err.message; + } innerFail({ - actual: actual[key], - expected: expected[key], - message: msg || `${key}: expected ${inspect(expected[key])}, ` + - `not ${inspect(actual[key])}`, + actual, + expected, + message, operator: 'throws', stackStartFn: assert.throws }); @@ -389,16 +414,14 @@ function expectedException(actual, expected, msg) { 'expected', ['Function', 'RegExp'], expected ); } - // The name and message could be non enumerable. Therefore test them - // explicitly. - if ('name' in expected) { - compareExceptionKey(actual, expected, 'name', msg); + const keys = Object.keys(expected); + // Special handle errors to make sure the name and the message are compared + // as well. + if (expected instanceof Error) { + keys.push('name', 'message'); } - if ('message' in expected) { - compareExceptionKey(actual, expected, 'message', msg); - } - for (const key of Object.keys(expected)) { - compareExceptionKey(actual, expected, key, msg); + for (const key of keys) { + compareExceptionKey(actual, expected, key, msg, keys); } return true; } diff --git a/test/message/assert_throws_stack.out b/test/message/assert_throws_stack.out index d34bdd24798..71d06a461e1 100644 --- a/test/message/assert_throws_stack.out +++ b/test/message/assert_throws_stack.out @@ -2,7 +2,13 @@ assert.js:* throw new AssertionError(obj); ^ -AssertionError [ERR_ASSERTION]: bar: expected true, not undefined +AssertionError [ERR_ASSERTION]: Input A expected to deepStrictEqual input B: ++ expected - actual + +- Comparison {} ++ Comparison { ++ bar: true ++ } at Object. (*assert_throws_stack.js:*:*) at * at * diff --git a/test/parallel/test-assert-fail-deprecation.js b/test/parallel/test-assert-fail-deprecation.js index 9da26dab3fd..aab26d42725 100644 --- a/test/parallel/test-assert-fail-deprecation.js +++ b/test/parallel/test-assert-fail-deprecation.js @@ -38,11 +38,7 @@ assert.throws(() => { assert.fail(typeof 1, 'object', new TypeError('another custom message')); }, { name: 'TypeError', - message: 'another custom message', - operator: undefined, - actual: undefined, - expected: undefined, - code: undefined + message: 'another custom message' }); // No third arg (but a fourth arg) diff --git a/test/parallel/test-assert-fail.js b/test/parallel/test-assert-fail.js index fc2b6cc03c1..e8336e8f216 100644 --- a/test/parallel/test-assert-fail.js +++ b/test/parallel/test-assert-fail.js @@ -33,8 +33,5 @@ assert.throws(() => { assert.fail(new TypeError('custom message')); }, { name: 'TypeError', - message: 'custom message', - operator: undefined, - actual: undefined, - expected: undefined + message: 'custom message' }); diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index ac56b409d05..195d9bbc801 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -34,6 +34,12 @@ 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'; + assert.ok(a.AssertionError.prototype instanceof Error, 'a.AssertionError instanceof Error'); @@ -436,11 +442,6 @@ common.expectsError( Error.stackTraceLimit = tmpLimit; // Test error diffs. - 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 plus = colors ? '\u001b[32m+\u001b[39m' : '+'; const minus = colors ? '\u001b[31m-\u001b[39m' : '-'; let message = [ @@ -765,24 +766,32 @@ common.expectsError( errObj.code = 404; assert.throws(errFn, errObj); - errObj.code = '404'; - common.expectsError( + // Fail in case a expected property is undefined and not existent on the + // error. + errObj.foo = undefined; + assert.throws( () => assert.throws(errFn, errObj), { code: 'ERR_ASSERTION', - type: assert.AssertionError, - message: 'code: expected \'404\', not 404' + name: 'AssertionError [ERR_ASSERTION]', + message: `${start}\n${actExp}\n\n` + + " Comparison {\n name: 'TypeError',\n" + + " message: 'Wrong value',\n- code: 404\n" + + '+ code: 404,\n+ foo: undefined\n }' } ); - errObj.code = 404; - errObj.foo = 'bar'; - common.expectsError( + // Show multiple wrong properties at the same time. + errObj.code = '404'; + assert.throws( () => assert.throws(errFn, errObj), { code: 'ERR_ASSERTION', - type: assert.AssertionError, - message: 'foo: expected \'bar\', not undefined' + name: 'AssertionError [ERR_ASSERTION]', + message: `${start}\n${actExp}\n\n` + + " Comparison {\n name: 'TypeError',\n" + + " message: 'Wrong value',\n- code: 404\n" + + "+ code: '404',\n+ foo: undefined\n }" } ); @@ -806,20 +815,24 @@ common.expectsError( ); assert.throws(() => { throw new Error('e'); }, new Error('e')); - common.expectsError( + assert.throws( () => assert.throws(() => { throw new TypeError('e'); }, new Error('e')), { - type: assert.AssertionError, + name: 'AssertionError [ERR_ASSERTION]', code: 'ERR_ASSERTION', - message: "name: expected 'Error', not 'TypeError'" + message: `${start}\n${actExp}\n\n` + + " Comparison {\n- name: 'TypeError',\n+ name: 'Error'," + + "\n message: 'e'\n }" } ); - common.expectsError( + assert.throws( () => assert.throws(() => { throw new Error('foo'); }, new Error('')), { - type: assert.AssertionError, + name: 'AssertionError [ERR_ASSERTION]', code: 'ERR_ASSERTION', - message: "message: expected '', not 'foo'" + message: `${start}\n${actExp}\n\n` + + " Comparison {\n name: 'Error',\n- message: 'foo'" + + "\n+ message: ''\n }" } );