From 627ce64cc8d8034be00a534274ec0116a8b70af1 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 27 Jun 2017 06:00:35 +0200 Subject: [PATCH] assert: refactor the code 1. Rename private functions 2. Use destructuring 3. Remove obsolete comments Backport-PR-URL: https://github.com/nodejs/node/pull/15516 PR-URL: https://github.com/nodejs/node/pull/13862 Reviewed-By: Refael Ackermann Reviewed-By: Joyee Cheung --- lib/assert.js | 84 +++++++++++-------------------- test/parallel/test-assert-fail.js | 10 ++++ 2 files changed, 40 insertions(+), 54 deletions(-) create mode 100644 test/parallel/test-assert-fail.js diff --git a/lib/assert.js b/lib/assert.js index 219842f953a12c..cfc7addbcb0fcc 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -20,8 +20,7 @@ 'use strict'; -// UTILITY -const compare = process.binding('buffer').compare; +const { compare } = process.binding('buffer'); const util = require('util'); const Buffer = require('buffer').Buffer; const pToString = (obj) => Object.prototype.toString.call(obj); @@ -49,7 +48,7 @@ assert.AssertionError = function AssertionError(options) { this.message = getMessage(this); this.generatedMessage = true; } - var stackStartFunction = options.stackStartFunction || fail; + const stackStartFunction = options.stackStartFunction || fail; Error.captureStackTrace(this, stackStartFunction); }; @@ -73,7 +72,7 @@ function getMessage(self) { // 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. @@ -86,25 +85,16 @@ function fail(actual, expected, message, operator, stackStartFunction) { stackStartFunction: stackStartFunction }); } - -// EXTENSION! allows for well behaved errors defined elsewhere. assert.fail = fail; // 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) fail(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) { if (actual != expected) fail(actual, expected, message, '==', assert.equal); @@ -112,31 +102,27 @@ assert.equal = function equal(actual, expected, message) { // 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) { if (actual == expected) { - fail(actual, expected, message, '!=', assert.notEqual); + fail(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)) { + fail(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)) { + fail(actual, expected, message, 'deepStrictEqual', deepStrictEqual); } }; -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; @@ -247,45 +233,40 @@ function objEquiv(a, b, strict, actualVisitedObjects) { // Possibly expensive deep test: for (i = ka.length - 1; i >= 0; i--) { key = ka[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)) { + fail(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)) { + fail(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); + fail(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); + fail(actual, expected, message, '!==', notStrictEqual); } }; @@ -314,7 +295,7 @@ function expectedException(actual, expected) { return expected.call({}, actual) === true; } -function _tryBlock(block) { +function tryBlock(block) { var error; try { block(); @@ -324,9 +305,7 @@ function _tryBlock(block) { return error; } -function _throws(shouldThrow, block, expected, message) { - var actual; - +function innerThrows(shouldThrow, block, expected, message) { if (typeof block !== 'function') { throw new TypeError('"block" argument must be a function'); } @@ -336,13 +315,13 @@ function _throws(shouldThrow, block, expected, message) { expected = null; } - actual = _tryBlock(block); + const actual = tryBlock(block); message = (expected && expected.name ? ' (' + expected.name + ').' : '.') + (message ? ' ' + message : '.'); if (shouldThrow && !actual) { - fail(actual, expected, 'Missing expected exception' + message); + fail(actual, expected, 'Missing expected exception' + message, fail); } const userProvidedMessage = typeof message === 'string'; @@ -353,7 +332,7 @@ function _throws(shouldThrow, block, expected, message) { userProvidedMessage && expectedException(actual, expected)) || isUnexpectedException) { - fail(actual, expected, 'Got unwanted exception' + message); + fail(actual, expected, 'Got unwanted exception' + message, fail); } if ((shouldThrow && actual && expected && @@ -363,15 +342,12 @@ function _throws(shouldThrow, block, expected, message) { } // Expected to throw an error. -// assert.throws(block, Error_opt, message_opt); - -assert.throws = function(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 = function(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(err) { if (err) throw err; }; +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 new file mode 100644 index 00000000000000..0423b35ec4d901 --- /dev/null +++ b/test/parallel/test-assert-fail.js @@ -0,0 +1,10 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); + +// The stackFrameFunction should exclude the foo frame +assert.throws( + function foo() { assert.fail('first', 'second', 'message', '!==', foo); }, + (err) => !/foo/m.test(err.stack) +);