From 1b2733f272b77fb2beaa4b1f5ee600e8b9c36e14 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 5 Jul 2017 15:04:24 +0200 Subject: [PATCH] test: common.expectsError should be a must call Wrap expectsError in mustCall to make sure it's really called as expected. PR-URL: https://github.com/nodejs/node/pull/14088 Reviewed-By: Refael Ackermann Reviewed-By: Benjamin Gruenbaum --- test/common/README.md | 9 +++-- test/common/index.js | 13 +++--- test/parallel/test-assert.js | 6 +-- test/parallel/test-buffer-compare-offset.js | 2 +- .../test-child-process-send-after-close.js | 6 +-- ...ild-process-spawnsync-validation-errors.js | 2 +- .../test-child-process-validate-stdio.js | 2 +- test/parallel/test-fs-whatwg-url.js | 40 ++++++++----------- .../test-internal-util-assertCrypto.js | 9 ++--- test/parallel/test-path-parse-format.js | 2 +- test/parallel/test-process-cpuUsage.js | 4 +- test/parallel/test-process-emitwarning.js | 2 +- test/parallel/test-process-kill-pid.js | 2 +- test/parallel/test-url-format-whatwg.js | 2 +- test/parallel/test-whatwg-url-domainto.js | 2 +- test/parallel/test-whatwg-url-parsing.js | 2 +- ...est-whatwg-url-searchparams-constructor.js | 2 +- test/parallel/test-whatwg-url-searchparams.js | 2 +- 18 files changed, 49 insertions(+), 60 deletions(-) diff --git a/test/common/README.md b/test/common/README.md index 21d89591ae453e..80cf271853cb63 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -50,7 +50,7 @@ Platform normalizes the `dd` command Check if there is more than 1gb of total memory. -### expectsError([fn, ]settings) +### expectsError([fn, ]settings[, exact]) * `fn` [<Function>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function) * `settings` [<Object>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object) with the following optional properties: @@ -63,9 +63,12 @@ Check if there is more than 1gb of total memory. if a string is provided for `message`, expected error must have it for its `message` property; if a regular expression is provided for `message`, the regular expression must match the `message` property of the expected error +* `exact` [<Number>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Number_type) default = 1 * return function suitable for use as a validation function passed as the second - argument to `assert.throws()` + argument to e.g. `assert.throws()`. If the returned function has not been called + exactly `exact` number of times when the test is complete, then the test will + fail. If `fn` is provided, it will be passed to `assert.throws` as first argument. @@ -217,7 +220,7 @@ Array of IPV6 hosts. * return [<Function>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function) Returns a function that calls `fn`. If the returned function has not been called -exactly `expected` number of times when the test is complete, then the test will +exactly `exact` number of times when the test is complete, then the test will fail. If `fn` is not provided, an empty function will be used. diff --git a/test/common/index.js b/test/common/index.js index 308696fba5faf1..9b8d46bb7e4fe9 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -488,7 +488,7 @@ exports.mustCallAtLeast = function(fn, minimum) { return _mustCallInner(fn, minimum, 'minimum'); }; -function _mustCallInner(fn, criteria, field) { +function _mustCallInner(fn, criteria = 1, field) { if (typeof fn === 'number') { criteria = fn; fn = noop; @@ -496,9 +496,7 @@ function _mustCallInner(fn, criteria, field) { fn = noop; } - if (criteria === undefined) - criteria = 1; - else if (typeof criteria !== 'number') + if (typeof criteria !== 'number') throw new TypeError(`Invalid ${field} value: ${criteria}`); const context = { @@ -702,13 +700,14 @@ Object.defineProperty(exports, 'hasSmallICU', { }); // Useful for testing expected internal/error objects -exports.expectsError = function expectsError(fn, options) { +exports.expectsError = function expectsError(fn, options, exact) { if (typeof fn !== 'function') { + exact = options; options = fn; fn = undefined; } const { code, type, message } = options; - function innerFn(error) { + const innerFn = exports.mustCall(function(error) { assert.strictEqual(error.code, code); if (type !== undefined) { assert(error instanceof type, @@ -721,7 +720,7 @@ exports.expectsError = function expectsError(fn, options) { assert.strictEqual(error.message, message); } return true; - } + }, exact); if (fn) { assert.throws(fn, innerFn); return; diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index d35a6d27355df8..d4a1b277bcc195 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -153,11 +153,7 @@ assert.throws(makeBlock(a.deepEqual, /a/igm, /a/im), { const re1 = /a/g; re1.lastIndex = 3; - assert.doesNotThrow(makeBlock(a.deepEqual, re1, /a/g), - common.expectsError({ - code: 'ERR_ASSERTION', - message: /^\/a\/g deepEqual \/a\/g$/ - })); + assert.doesNotThrow(makeBlock(a.deepEqual, re1, /a/g)); } assert.doesNotThrow(makeBlock(a.deepEqual, 4, '4'), 'deepEqual(4, \'4\')'); diff --git a/test/parallel/test-buffer-compare-offset.js b/test/parallel/test-buffer-compare-offset.js index e5c7a691102cbc..265bd05026a658 100644 --- a/test/parallel/test-buffer-compare-offset.js +++ b/test/parallel/test-buffer-compare-offset.js @@ -57,7 +57,7 @@ assert.strictEqual(1, a.compare(b, Infinity, -Infinity)); // zero length target because default for targetEnd <= targetSource assert.strictEqual(1, a.compare(b, '0xff')); -const oor = common.expectsError({code: 'ERR_INDEX_OUT_OF_RANGE'}); +const oor = common.expectsError({code: 'ERR_INDEX_OUT_OF_RANGE'}, 7); assert.throws(() => a.compare(b, 0, 100, 0), oor); assert.throws(() => a.compare(b, 0, 1, 0, 100), oor); diff --git a/test/parallel/test-child-process-send-after-close.js b/test/parallel/test-child-process-send-after-close.js index 5039767aeb7660..b497965708e6dc 100644 --- a/test/parallel/test-child-process-send-after-close.js +++ b/test/parallel/test-child-process-send-after-close.js @@ -14,9 +14,9 @@ child.on('close', common.mustCall((code, signal) => { type: Error, message: 'Channel closed', code: 'ERR_IPC_CHANNEL_CLOSED' - }); + }, 2); - child.on('error', common.mustCall(testError)); + child.on('error', testError); { const result = child.send('ping'); @@ -24,7 +24,7 @@ child.on('close', common.mustCall((code, signal) => { } { - const result = child.send('pong', common.mustCall(testError)); + const result = child.send('pong', testError); assert.strictEqual(result, false); } })); diff --git a/test/parallel/test-child-process-spawnsync-validation-errors.js b/test/parallel/test-child-process-spawnsync-validation-errors.js index 98a947825f297b..e586037e1d28ff 100644 --- a/test/parallel/test-child-process-spawnsync-validation-errors.js +++ b/test/parallel/test-child-process-spawnsync-validation-errors.js @@ -186,7 +186,7 @@ if (!common.isWindows) { // Validate the killSignal option const typeErr = /^TypeError: "killSignal" must be a string or number$/; const unknownSignalErr = - common.expectsError({ code: 'ERR_UNKNOWN_SIGNAL', type: TypeError }); + common.expectsError({ code: 'ERR_UNKNOWN_SIGNAL', type: TypeError }, 17); pass('killSignal', undefined); pass('killSignal', null); diff --git a/test/parallel/test-child-process-validate-stdio.js b/test/parallel/test-child-process-validate-stdio.js index c6a9bd8e19129c..43f7d02c9bd966 100644 --- a/test/parallel/test-child-process-validate-stdio.js +++ b/test/parallel/test-child-process-validate-stdio.js @@ -6,7 +6,7 @@ const assert = require('assert'); const _validateStdio = require('internal/child_process')._validateStdio; const expectedError = - common.expectsError({code: 'ERR_INVALID_OPT_VALUE', type: TypeError}); + common.expectsError({code: 'ERR_INVALID_OPT_VALUE', type: TypeError}, 2); // should throw if string and not ignore, pipe, or inherit assert.throws(() => _validateStdio('foo'), expectedError); diff --git a/test/parallel/test-fs-whatwg-url.js b/test/parallel/test-fs-whatwg-url.js index 3f7be7f7809beb..c80fb5ca9e1512 100644 --- a/test/parallel/test-fs-whatwg-url.js +++ b/test/parallel/test-fs-whatwg-url.js @@ -28,12 +28,10 @@ fs.readFile(url, common.mustCall((err, data) => { // Check that using a non file:// URL reports an error const httpUrl = new URL('http://example.org'); -fs.readFile(httpUrl, common.mustCall((err) => { - common.expectsError({ - code: 'ERR_INVALID_URL_SCHEME', - type: TypeError, - message: 'The URL must be of scheme file' - })(err); +fs.readFile(httpUrl, common.expectsError({ + code: 'ERR_INVALID_URL_SCHEME', + type: TypeError, + message: 'The URL must be of scheme file' })); // pct-encoded characters in the path will be decoded and checked @@ -46,31 +44,25 @@ fs.readFile(new URL('file:///c:/tmp/%00test'), common.mustCall((err) => { if (common.isWindows) { // encoded back and forward slashes are not permitted on windows ['%2f', '%2F', '%5c', '%5C'].forEach((i) => { - fs.readFile(new URL(`file:///c:/tmp/${i}`), common.mustCall((err) => { - common.expectsError({ - code: 'ERR_INVALID_FILE_URL_PATH', - type: TypeError, - message: 'File URL path must not include encoded \\ or / characters' - })(err); + fs.readFile(new URL(`file:///c:/tmp/${i}`), common.expectsError({ + code: 'ERR_INVALID_FILE_URL_PATH', + type: TypeError, + message: 'File URL path must not include encoded \\ or / characters' })); }); } else { // encoded forward slashes are not permitted on other platforms ['%2f', '%2F'].forEach((i) => { - fs.readFile(new URL(`file:///c:/tmp/${i}`), common.mustCall((err) => { - common.expectsError({ - code: 'ERR_INVALID_FILE_URL_PATH', - type: TypeError, - message: 'File URL path must not include encoded / characters' - })(err); + fs.readFile(new URL(`file:///c:/tmp/${i}`), common.expectsError({ + code: 'ERR_INVALID_FILE_URL_PATH', + type: TypeError, + message: 'File URL path must not include encoded / characters' })); }); - fs.readFile(new URL('file://hostname/a/b/c'), common.mustCall((err) => { - common.expectsError({ - code: 'ERR_INVALID_FILE_URL_HOST', - type: TypeError, - message: `File URL host must be "localhost" or empty on ${os.platform()}` - })(err); + fs.readFile(new URL('file://hostname/a/b/c'), common.expectsError({ + code: 'ERR_INVALID_FILE_URL_HOST', + type: TypeError, + message: `File URL host must be "localhost" or empty on ${os.platform()}` })); } diff --git a/test/parallel/test-internal-util-assertCrypto.js b/test/parallel/test-internal-util-assertCrypto.js index d1387e489e1dfc..d01eab3a237ded 100644 --- a/test/parallel/test-internal-util-assertCrypto.js +++ b/test/parallel/test-internal-util-assertCrypto.js @@ -4,12 +4,11 @@ const common = require('../common'); const assert = require('assert'); const util = require('internal/util'); -const expectedError = common.expectsError({ - code: 'ERR_NO_CRYPTO', - type: Error -}); - if (!process.versions.openssl) { + const expectedError = common.expectsError({ + code: 'ERR_NO_CRYPTO', + type: Error + }); assert.throws(() => util.assertCrypto(), expectedError); } else { assert.doesNotThrow(() => util.assertCrypto()); diff --git a/test/parallel/test-path-parse-format.js b/test/parallel/test-path-parse-format.js index 1c3bdff130f55d..b253249472cb08 100644 --- a/test/parallel/test-path-parse-format.js +++ b/test/parallel/test-path-parse-format.js @@ -91,7 +91,7 @@ const unixSpecialCaseFormatTests = [ const expectedMessage = common.expectsError({ code: 'ERR_INVALID_ARG_TYPE', type: TypeError -}); +}, 18); const errors = [ {method: 'parse', input: [null], message: expectedMessage}, diff --git a/test/parallel/test-process-cpuUsage.js b/test/parallel/test-process-cpuUsage.js index 5f51f0ac839a8b..a6e799ca1b3213 100644 --- a/test/parallel/test-process-cpuUsage.js +++ b/test/parallel/test-process-cpuUsage.js @@ -35,13 +35,13 @@ const invalidUserArgument = common.expectsError({ code: 'ERR_INVALID_ARG_TYPE', type: TypeError, message: 'The "preValue.user" property must be of type Number' -}); +}, 8); const invalidSystemArgument = common.expectsError({ code: 'ERR_INVALID_ARG_TYPE', type: TypeError, message: 'The "preValue.system" property must be of type Number' -}); +}, 2); // Ensure that an invalid shape for the previous value argument throws an error. diff --git a/test/parallel/test-process-emitwarning.js b/test/parallel/test-process-emitwarning.js index d2d090eae339ee..0349f30cd9f4db 100644 --- a/test/parallel/test-process-emitwarning.js +++ b/test/parallel/test-process-emitwarning.js @@ -58,7 +58,7 @@ warningThrowToString.toString = function() { process.emitWarning(warningThrowToString); const expectedError = - common.expectsError({code: 'ERR_INVALID_ARG_TYPE', type: TypeError}); + common.expectsError({code: 'ERR_INVALID_ARG_TYPE', type: TypeError}, 11); // TypeError is thrown on invalid input assert.throws(() => process.emitWarning(1), expectedError); diff --git a/test/parallel/test-process-kill-pid.js b/test/parallel/test-process-kill-pid.js index 74491710d2646b..994a19f5ccea8b 100644 --- a/test/parallel/test-process-kill-pid.js +++ b/test/parallel/test-process-kill-pid.js @@ -42,7 +42,7 @@ const invalidPidArgument = common.expectsError({ code: 'ERR_INVALID_ARG_TYPE', type: TypeError, message: 'The "pid" argument must be of type Number' -}); +}, 6); assert.throws(function() { process.kill('SIGTERM'); }, invalidPidArgument); diff --git a/test/parallel/test-url-format-whatwg.js b/test/parallel/test-url-format-whatwg.js index d484760c808584..d6afe46426b2ac 100644 --- a/test/parallel/test-url-format-whatwg.js +++ b/test/parallel/test-url-format-whatwg.js @@ -25,7 +25,7 @@ assert.strictEqual( code: 'ERR_INVALID_ARG_TYPE', type: TypeError, message: 'The "options" argument must be of type object' - }); + }, 4); assert.throws(() => url.format(myURL, true), expectedErr); assert.throws(() => url.format(myURL, 1), expectedErr); assert.throws(() => url.format(myURL, 'test'), expectedErr); diff --git a/test/parallel/test-whatwg-url-domainto.js b/test/parallel/test-whatwg-url-domainto.js index bfd5e94d2e5dd8..9e22068a9261f8 100644 --- a/test/parallel/test-whatwg-url-domainto.js +++ b/test/parallel/test-whatwg-url-domainto.js @@ -13,7 +13,7 @@ const wptToASCIITests = require('../fixtures/url-toascii.js'); { const expectedError = common.expectsError( - { code: 'ERR_MISSING_ARGS', type: TypeError }); + { code: 'ERR_MISSING_ARGS', type: TypeError }, 2); assert.throws(() => domainToASCII(), expectedError); assert.throws(() => domainToUnicode(), expectedError); assert.strictEqual(domainToASCII(undefined), 'undefined'); diff --git a/test/parallel/test-whatwg-url-parsing.js b/test/parallel/test-whatwg-url-parsing.js index 39756449c08c6a..2c1d15d10ce351 100644 --- a/test/parallel/test-whatwg-url-parsing.js +++ b/test/parallel/test-whatwg-url-parsing.js @@ -26,7 +26,7 @@ const failureTests = tests.filter((test) => test.failure).concat([ ]); const expectedError = common.expectsError( - { code: 'ERR_INVALID_URL', type: TypeError }); + { code: 'ERR_INVALID_URL', type: TypeError }, 102); for (const test of failureTests) { assert.throws( diff --git a/test/parallel/test-whatwg-url-searchparams-constructor.js b/test/parallel/test-whatwg-url-searchparams-constructor.js index 03ea62462165df..4a549c842b1942 100644 --- a/test/parallel/test-whatwg-url-searchparams-constructor.js +++ b/test/parallel/test-whatwg-url-searchparams-constructor.js @@ -209,7 +209,7 @@ function makeIterableFunc(array) { code: 'ERR_INVALID_TUPLE', type: TypeError, message: 'Each query pair must be an iterable [name, value] tuple' - }); + }, 6); let params; params = new URLSearchParams(undefined); diff --git a/test/parallel/test-whatwg-url-searchparams.js b/test/parallel/test-whatwg-url-searchparams.js index 5bb9cf407dc1e9..db7f9a491719dc 100644 --- a/test/parallel/test-whatwg-url-searchparams.js +++ b/test/parallel/test-whatwg-url-searchparams.js @@ -76,7 +76,7 @@ sp.forEach(function() { const callbackErr = common.expectsError({ code: 'ERR_INVALID_CALLBACK', type: TypeError - }); + }, 2); assert.throws(() => sp.forEach(), callbackErr); assert.throws(() => sp.forEach(1), callbackErr); }