test: common.expectsError should be a must call

Wrap expectsError in mustCall to make sure it's really called as expected. PR-URL: #14088 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
nodejs · Jul 9, 2017 · 1b2733f · 1b2733f
1 parent be20e9e
commit 1b2733f
Show file tree

Hide file tree

Showing 18 changed files with 49 additions and 60 deletions.
diff --git 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` [&lt;Function>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function)
 * `settings` [&lt;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` [&lt;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 [&lt;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
@@ -488,17 +488,15 @@ 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;
   } else if (fn === undefined) {
     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
@@ -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
@@ -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
@@ -14,17 +14,17 @@ 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');
     assert.strictEqual(result, false);
   }
 
   {
-    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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);
 }