From 7a2b3e2b6cc92b2d694041ab852e860f459dadcd Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sun, 2 Jul 2017 18:06:35 +0200 Subject: [PATCH 1/2] doc,assert: document stackStartFunction in fail PR-URL: https://github.com/nodejs/node/pull/13862 Reviewed-By: Refael Ackermann Reviewed-By: Joyee Cheung --- doc/api/assert.md | 44 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/doc/api/assert.md b/doc/api/assert.md index 502150fd8dc9c8..c92ae3f3f12e9f 100644 --- a/doc/api/assert.md +++ b/doc/api/assert.md @@ -257,7 +257,7 @@ property set equal to the value of the `message` parameter. If the `message` parameter is undefined, a default error message is assigned. ## assert.fail([message]) -## assert.fail(actual, expected, message, operator) +## assert.fail(actual, expected[, message[, operator[, stackStartFunction]]]) @@ -265,29 +265,54 @@ added: v0.1.21 * `expected` {any} * `message` {any} (default: 'Failed') * `operator` {string} (default: '!=') +* `stackStartFunction` {function} (default: `assert.fail`) Throws an `AssertionError`. If `message` is falsy, the error message is set as the values of `actual` and `expected` separated by the provided `operator`. -Otherwise, the error message is the value of `message`. -If no arguments are provided at all, a default message will be used instead. +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 stored as properties on the thrown object. +If `stackStartFunction` is provided, all stack frames above that function will +be removed from stacktrace (see [`Error.captureStackTrace`]). If no arguments +are given, the default message `Failed` will be used. ```js const assert = require('assert'); assert.fail(1, 2, undefined, '>'); -// AssertionError: 1 > 2 +// AssertionError [ERR_ASSERTION]: 1 > 2 + +assert.fail(1, 2, 'fail'); +// AssertionError [ERR_ASSERTION]: fail assert.fail(1, 2, 'whoops', '>'); -// AssertionError: whoops +// AssertionError [ERR_ASSERTION]: whoops +``` + +*Note*: Is the last two cases `actual`, `expected`, and `operator` have no +influence on the error message. + +```js +assert.fail(); +// AssertionError [ERR_ASSERTION]: Failed assert.fail('boom'); -// AssertionError: boom +// AssertionError [ERR_ASSERTION]: boom assert.fail('a', 'b'); -// AssertionError: 'a' != 'b' +// AssertionError [ERR_ASSERTION]: 'a' != 'b' +``` -assert.fail(); -// AssertionError: Failed +Example use of `stackStartFunction` for truncating the exception's stacktrace: +```js +function suppressFrame() { + assert.fail('a', 'b', undefined, '!==', suppressFrame); +} +suppressFrame(); +// AssertionError [ERR_ASSERTION]: 'a' !== 'b' +// at repl:1:1 +// at ContextifyScript.Script.runInThisContext (vm.js:44:33) +// ... ``` ## assert.ifError(value) @@ -594,6 +619,7 @@ For more information, see [MDN's guide on equality comparisons and sameness][mdn-equality-guide]. [`Error`]: errors.html#errors_class_error +[`Error.captureStackTrace`]: errors.html#errors_error_capturestacktrace_targetobject_constructoropt [`Map`]: https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Map [`Object.is()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/is [`RegExp`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions From 0455fff880b9a4d25ee24150aedafa51808926ee Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 27 Jun 2017 06:00:35 +0200 Subject: [PATCH 2/2] assert: refactor the code 1. Rename private functions 2. Use destructuring 3. Remove obsolete comments PR-URL: https://github.com/nodejs/node/pull/13862 Reviewed-By: Refael Ackermann Reviewed-By: Joyee Cheung --- lib/assert.js | 118 +++++++++++++----------------- test/parallel/test-assert-fail.js | 43 ++++++++--- 2 files changed, 83 insertions(+), 78 deletions(-) diff --git a/lib/assert.js b/lib/assert.js index ca7c28154a93a9..a73536b5145de1 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -20,12 +20,11 @@ 'use strict'; -// UTILITY -const compare = process.binding('buffer').compare; +const { compare } = process.binding('buffer'); const util = require('util'); const { isSet, isMap } = process.binding('util'); -const objectToString = require('internal/util').objectToString; -const Buffer = require('buffer').Buffer; +const { objectToString } = require('internal/util'); +const { Buffer } = require('buffer'); var errors; function lazyErrors() { @@ -47,10 +46,21 @@ const assert = module.exports = ok; // 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 +// may be undefined if not provided. All assertion methods provide // both the actual and expected values to the assertion error for // display purposes. +function innerFail(actual, expected, message, operator, stackStartFunction) { + const errors = lazyErrors(); + throw new errors.AssertionError({ + message, + actual, + expected, + operator, + stackStartFunction + }); +} + function fail(actual, expected, message, operator, stackStartFunction) { if (arguments.length === 0) { message = 'Failed'; @@ -59,19 +69,11 @@ function fail(actual, expected, message, operator, stackStartFunction) { message = actual; actual = undefined; } - if (arguments.length === 2) + if (arguments.length === 2) { operator = '!='; - const errors = lazyErrors(); - throw new errors.AssertionError({ - message: message, - actual: actual, - expected: expected, - operator: operator, - stackStartFunction: stackStartFunction - }); + } + innerFail(actual, expected, message, operator, stackStartFunction || fail); } - -// EXTENSION! allows for well behaved errors defined elsewhere. assert.fail = fail; // The AssertionError is defined in internal/error. @@ -82,50 +84,39 @@ assert.AssertionError = lazyErrors().AssertionError; // Pure assertion tests whether a value is truthy, as determined -// by !!guard. -// assert.ok(guard, message_opt); -// This statement is equivalent to assert.equal(true, !!guard, -// message_opt);. To test strictly for the value true, use -// assert.strictEqual(true, guard, message_opt);. - +// by !!value. function ok(value, message) { - if (!value) fail(value, true, message, '==', assert.ok); + if (!value) innerFail(value, true, message, '==', ok); } assert.ok = ok; -// The equality assertion tests shallow, coercive equality with -// ==. -// assert.equal(actual, expected, message_opt); +// The equality assertion tests shallow, coercive equality with ==. /* eslint-disable no-restricted-properties */ assert.equal = function equal(actual, expected, message) { // eslint-disable-next-line eqeqeq - if (actual != expected) fail(actual, expected, message, '==', assert.equal); + if (actual != expected) innerFail(actual, expected, message, '==', equal); }; // The non-equality assertion tests for whether two objects are not // equal with !=. -// assert.notEqual(actual, expected, message_opt); - assert.notEqual = function notEqual(actual, expected, message) { // eslint-disable-next-line eqeqeq if (actual == expected) { - fail(actual, expected, message, '!=', assert.notEqual); + innerFail(actual, expected, message, '!=', notEqual); } }; // The equivalence assertion tests a deep equality relation. -// assert.deepEqual(actual, expected, message_opt); - assert.deepEqual = function deepEqual(actual, expected, message) { - if (!_deepEqual(actual, expected, false)) { - fail(actual, expected, message, 'deepEqual', assert.deepEqual); + if (!innerDeepEqual(actual, expected, false)) { + innerFail(actual, expected, message, 'deepEqual', deepEqual); } }; /* eslint-enable */ assert.deepStrictEqual = function deepStrictEqual(actual, expected, message) { - if (!_deepEqual(actual, expected, true)) { - fail(actual, expected, message, 'deepStrictEqual', assert.deepStrictEqual); + if (!innerDeepEqual(actual, expected, true)) { + innerFail(actual, expected, message, 'deepStrictEqual', deepStrictEqual); } }; @@ -154,7 +145,7 @@ function isArguments(tag) { return tag === '[object Arguments]'; } -function _deepEqual(actual, expected, strict, memos) { +function innerDeepEqual(actual, expected, strict, memos) { // All identical values are equivalent, as determined by ===. if (actual === expected) { return true; @@ -307,7 +298,7 @@ function setHasSimilarElement(set, val1, usedEntries, strict, memo) { if (usedEntries && usedEntries.has(val2)) continue; - if (_deepEqual(val1, val2, strict, memo)) { + if (innerDeepEqual(val1, val2, strict, memo)) { if (usedEntries) usedEntries.add(val2); return true; @@ -364,7 +355,7 @@ function mapHasSimilarEntry(map, key1, item1, usedEntries, strict, memo) { // This check is not strictly necessary. The loop performs this check, but // doing it here improves performance of the common case when reference-equal // keys exist (which includes all primitive-valued keys). - if (map.has(key1) && _deepEqual(item1, map.get(key1), strict, memo)) { + if (map.has(key1) && innerDeepEqual(item1, map.get(key1), strict, memo)) { if (usedEntries) usedEntries.add(key1); return true; @@ -381,8 +372,8 @@ function mapHasSimilarEntry(map, key1, item1, usedEntries, strict, memo) { if (usedEntries && usedEntries.has(key2)) continue; - if (_deepEqual(key1, key2, strict, memo) && - _deepEqual(item1, item2, strict, memo)) { + if (innerDeepEqual(key1, key2, strict, memo) && + innerDeepEqual(item1, item2, strict, memo)) { if (usedEntries) usedEntries.add(key2); return true; @@ -459,44 +450,39 @@ function objEquiv(a, b, strict, actualVisitedObjects) { // Possibly expensive deep test: for (i = aKeys.length - 1; i >= 0; i--) { key = aKeys[i]; - if (!_deepEqual(a[key], b[key], strict, actualVisitedObjects)) + if (!innerDeepEqual(a[key], b[key], strict, actualVisitedObjects)) return false; } return true; } // The non-equivalence assertion tests for any deep inequality. -// assert.notDeepEqual(actual, expected, message_opt); - assert.notDeepEqual = function notDeepEqual(actual, expected, message) { - if (_deepEqual(actual, expected, false)) { - fail(actual, expected, message, 'notDeepEqual', assert.notDeepEqual); + if (innerDeepEqual(actual, expected, false)) { + innerFail(actual, expected, message, 'notDeepEqual', notDeepEqual); } }; assert.notDeepStrictEqual = notDeepStrictEqual; function notDeepStrictEqual(actual, expected, message) { - if (_deepEqual(actual, expected, true)) { - fail(actual, expected, message, 'notDeepStrictEqual', notDeepStrictEqual); + if (innerDeepEqual(actual, expected, true)) { + innerFail(actual, expected, message, 'notDeepStrictEqual', + notDeepStrictEqual); } } // The strict equality assertion tests strict equality, as determined by ===. -// assert.strictEqual(actual, expected, message_opt); - assert.strictEqual = function strictEqual(actual, expected, message) { if (actual !== expected) { - fail(actual, expected, message, '===', assert.strictEqual); + innerFail(actual, expected, message, '===', strictEqual); } }; // The strict non-equality assertion tests for strict inequality, as // determined by !==. -// assert.notStrictEqual(actual, expected, message_opt); - assert.notStrictEqual = function notStrictEqual(actual, expected, message) { if (actual === expected) { - fail(actual, expected, message, '!==', assert.notStrictEqual); + innerFail(actual, expected, message, '!==', notStrictEqual); } }; @@ -525,7 +511,7 @@ function expectedException(actual, expected) { return expected.call({}, actual) === true; } -function _tryBlock(block) { +function tryBlock(block) { var error; try { block(); @@ -535,7 +521,7 @@ function _tryBlock(block) { return error; } -function _throws(shouldThrow, block, expected, message) { +function innerThrows(shouldThrow, block, expected, message) { var actual; if (typeof block !== 'function') { @@ -549,13 +535,13 @@ function _throws(shouldThrow, block, expected, message) { expected = null; } - actual = _tryBlock(block); + actual = tryBlock(block); message = (expected && expected.name ? ' (' + expected.name + ')' : '') + (message ? ': ' + message : '.'); if (shouldThrow && !actual) { - fail(actual, expected, 'Missing expected exception' + message); + innerFail(actual, expected, 'Missing expected exception' + message, fail); } const userProvidedMessage = typeof message === 'string'; @@ -566,7 +552,7 @@ function _throws(shouldThrow, block, expected, message) { userProvidedMessage && expectedException(actual, expected)) || isUnexpectedException) { - fail(actual, expected, 'Got unwanted exception' + message); + innerFail(actual, expected, 'Got unwanted exception' + message, fail); } if ((shouldThrow && actual && expected && @@ -576,16 +562,12 @@ function _throws(shouldThrow, block, expected, message) { } // Expected to throw an error. -// assert.throws(block, Error_opt, message_opt); - -assert.throws = function throws(block, /*optional*/error, /*optional*/message) { - _throws(true, block, error, message); +assert.throws = function throws(block, error, message) { + innerThrows(true, block, error, message); }; -// EXTENSION! This is annoying to write outside this module. -assert.doesNotThrow = doesNotThrow; -function doesNotThrow(block, /*optional*/error, /*optional*/message) { - _throws(false, block, error, message); -} +assert.doesNotThrow = function doesNotThrow(block, error, message) { + innerThrows(false, block, error, message); +}; assert.ifError = function ifError(err) { if (err) throw err; }; diff --git a/test/parallel/test-assert-fail.js b/test/parallel/test-assert-fail.js index 1b72310a169933..99f7c4e3da8d5d 100644 --- a/test/parallel/test-assert-fail.js +++ b/test/parallel/test-assert-fail.js @@ -1,53 +1,76 @@ 'use strict'; + const common = require('../common'); const assert = require('assert'); -// no args +// No args assert.throws( () => { assert.fail(); }, common.expectsError({ code: 'ERR_ASSERTION', type: assert.AssertionError, - message: 'Failed' + message: 'Failed', + operator: undefined, + actual: undefined, + expected: undefined }) ); -// one arg = message +// One arg = message assert.throws( () => { assert.fail('custom message'); }, common.expectsError({ code: 'ERR_ASSERTION', type: assert.AssertionError, - message: 'custom message' + message: 'custom message', + operator: undefined, + actual: undefined, + expected: undefined }) ); -// two args only, operator defaults to '!=' +// Two args only, operator defaults to '!=' assert.throws( () => { assert.fail('first', 'second'); }, common.expectsError({ code: 'ERR_ASSERTION', type: assert.AssertionError, - message: '\'first\' != \'second\'' + message: '\'first\' != \'second\'', + operator: '!=', + actual: 'first', + expected: 'second' + }) ); -// three args +// Three args assert.throws( () => { assert.fail('ignored', 'ignored', 'another custom message'); }, common.expectsError({ code: 'ERR_ASSERTION', type: assert.AssertionError, - message: 'another custom message' + message: 'another custom message', + operator: undefined, + actual: 'ignored', + expected: 'ignored' }) ); -// no third arg (but a fourth arg) +// No third arg (but a fourth arg) assert.throws( () => { assert.fail('first', 'second', undefined, 'operator'); }, common.expectsError({ code: 'ERR_ASSERTION', type: assert.AssertionError, - message: '\'first\' operator \'second\'' + message: '\'first\' operator \'second\'', + operator: 'operator', + actual: 'first', + expected: 'second' }) ); + +// The stackFrameFunction should exclude the foo frame +assert.throws( + function foo() { assert.fail('first', 'second', 'message', '!==', foo); }, + (err) => !/foo/m.test(err.stack) +);