test: add common.mustSucceed

PR-URL: #35086
Reviewed-By: Ruy Adorno <ruyadorno@github.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

doc/guides/writing-tests.md

@@ -209,6 +209,11 @@ const server = http.createServer(common.mustCall((req, res) => {
 
 ```
 
+**Note:** Many functions invoke their callback with an `err` value as the first
+argument. It is not a good idea to simply pass `common.mustCall()` to those
+because `common.mustCall()` will ignore the error. Use `common.mustSucceed()`
+instead.
+
 #### Countdown Module
 
 The common [Countdown module](https://github.com/nodejs/node/tree/master/test/common#countdown-module)

test/.eslintrc.yaml

@@ -50,6 +50,7 @@ rules:
   node-core/prefer-assert-iferror: error
   node-core/prefer-assert-methods: error
   node-core/prefer-common-mustnotcall: error
+  node-core/prefer-common-mustsucceed: error
   node-core/crypto-check: error
   node-core/eslint-check: error
   node-core/async-iife-no-unused-result: error

test/common/README.md

@@ -314,6 +314,15 @@ If `fn` is not provided, an empty function will be used.
 Returns a function that triggers an `AssertionError` if it is invoked. `msg` is
 used as the error message for the `AssertionError`.
 
+### `mustSucceed([fn])`
+
+* `fn` [<Function>][] default = () => {}
+* return [<Function>][]
+
+Returns a function that accepts arguments `(err, ...args)`. If `err` is not
+`undefined` or `null`, it triggers an `AssertionError`. Otherwise, it calls
+`fn(...args)`.
+
 ### `nodeProcessAborted(exitCode, signal)`
 
 * `exitCode` [<number>][]

test/common/index.js

@@ -335,6 +335,14 @@ function mustCall(fn, exact) {
   return _mustCallInner(fn, exact, 'exact');
 }
 
+function mustSucceed(fn, exact) {
+  return mustCall(function(err, ...args) {
+    assert.ifError(err);
+    if (typeof fn === 'function')
+      return fn.apply(this, args);
+  }, exact);
+}
+
 function mustCallAtLeast(fn, minimum) {
   return _mustCallInner(fn, minimum, 'minimum');
 }
@@ -722,6 +730,7 @@ const common = {
   mustCall,
   mustCallAtLeast,
   mustNotCall,
+  mustSucceed,
   nodeProcessAborted,
   PIPE,
   platformTimeout,

test/doctool/test-doctool-json.js

@@ -229,10 +229,8 @@ const testData = [
 ];
 
 testData.forEach((item) => {
-  fs.readFile(item.file, 'utf8', common.mustCall((err, input) => {
-    assert.ifError(err);
-    toJSON(input, 'foo', common.mustCall((err, output) => {
-      assert.ifError(err);
+  fs.readFile(item.file, 'utf8', common.mustSucceed((input) => {
+    toJSON(input, 'foo', common.mustSucceed((output) => {
       assert.deepStrictEqual(output.json, item.json);
     }));
   }));

test/internet/test-dns-any.js

@@ -127,8 +127,7 @@ TEST(async function test_sip2sip_for_naptr(done) {
   const req = dns.resolve(
     'sip2sip.info',
     'ANY',
-    common.mustCall(function(err, ret) {
-      assert.ifError(err);
+    common.mustSucceed((ret) => {
       validateResult(ret);
       done();
     }));
@@ -147,8 +146,7 @@ TEST(async function test_google_for_cname_and_srv(done) {
   const req = dns.resolve(
     '_jabber._tcp.google.com',
     'ANY',
-    common.mustCall(function(err, ret) {
-      assert.ifError(err);
+    common.mustSucceed((ret) => {
       validateResult(ret);
       done();
     }));
@@ -167,8 +165,7 @@ TEST(async function test_ptr(done) {
   const req = dns.resolve(
     '8.8.8.8.in-addr.arpa',
     'ANY',
-    common.mustCall(function(err, ret) {
-      assert.ifError(err);
+    common.mustSucceed((ret) => {
       validateResult(ret);
       done();
     }));

test/internet/test-dns-ipv4.js

@@ -50,8 +50,7 @@ TEST(async function test_resolve4(done) {
 
   const req = dns.resolve4(
     addresses.INET4_HOST,
-    common.mustCall((err, ips) => {
-      assert.ifError(err);
+    common.mustSucceed((ips) => {
       validateResult(ips);
       done();
     }));
@@ -73,8 +72,7 @@ TEST(async function test_reverse_ipv4(done) {
 
   const req = dns.reverse(
     addresses.INET4_IP,
-    common.mustCall((err, domains) => {
-      assert.ifError(err);
+    common.mustSucceed((domains) => {
       validateResult(domains);
       done();
     }));
@@ -92,8 +90,7 @@ TEST(async function test_lookup_ipv4_explicit(done) {
 
   const req = dns.lookup(
     addresses.INET4_HOST, 4,
-    common.mustCall((err, ip, family) => {
-      assert.ifError(err);
+    common.mustSucceed((ip, family) => {
       validateResult({ address: ip, family });
       done();
     }));
@@ -111,8 +108,7 @@ TEST(async function test_lookup_ipv4_implicit(done) {
 
   const req = dns.lookup(
     addresses.INET4_HOST,
-    common.mustCall((err, ip, family) => {
-      assert.ifError(err);
+    common.mustSucceed((ip, family) => {
       validateResult({ address: ip, family });
       done();
     }));
@@ -130,8 +126,7 @@ TEST(async function test_lookup_ipv4_explicit_object(done) {
 
   const req = dns.lookup(addresses.INET4_HOST, {
     family: 4
-  }, common.mustCall((err, ip, family) => {
-    assert.ifError(err);
+  }, common.mustSucceed((ip, family) => {
     validateResult({ address: ip, family });
     done();
   }));
@@ -151,8 +146,7 @@ TEST(async function test_lookup_ipv4_hint_addrconfig(done) {
 
   const req = dns.lookup(addresses.INET4_HOST, {
     hints: dns.ADDRCONFIG
-  }, common.mustCall((err, ip, family) => {
-    assert.ifError(err);
+  }, common.mustSucceed((ip, family) => {
     validateResult({ address: ip, family });
     done();
   }));
@@ -169,8 +163,7 @@ TEST(async function test_lookup_ip_ipv4(done) {
   validateResult(await dnsPromises.lookup('127.0.0.1'));
 
   const req = dns.lookup('127.0.0.1',
-                         common.mustCall((err, ip, family) => {
-                           assert.ifError(err);
+                         common.mustSucceed((ip, family) => {
                            validateResult({ address: ip, family });
                            done();
                          }));
@@ -187,8 +180,7 @@ TEST(async function test_lookup_localhost_ipv4(done) {
   validateResult(await dnsPromises.lookup('localhost', 4));
 
   const req = dns.lookup('localhost', 4,
-                         common.mustCall((err, ip, family) => {
-                           assert.ifError(err);
+                         common.mustSucceed((ip, family) => {
                            validateResult({ address: ip, family });
                            done();
                          }));
@@ -215,8 +207,7 @@ TEST(async function test_lookup_all_ipv4(done) {
   const req = dns.lookup(
     addresses.INET4_HOST,
     { all: true, family: 4 },
-    common.mustCall((err, ips) => {
-      assert.ifError(err);
+    common.mustSucceed((ips) => {
       validateResult(ips);
       done();
     })
@@ -236,8 +227,7 @@ TEST(async function test_lookupservice_ip_ipv4(done) {
 
   const req = dns.lookupService(
     '127.0.0.1', 80,
-    common.mustCall((err, hostname, service) => {
-      assert.ifError(err);
+    common.mustSucceed((hostname, service) => {
       validateResult({ hostname, service });
       done();
     })

test/internet/test-dns-ipv6.js

@@ -52,8 +52,7 @@ TEST(async function test_resolve6(done) {
 
   const req = dns.resolve6(
     addresses.INET6_HOST,
-    common.mustCall((err, ips) => {
-      assert.ifError(err);
+    common.mustSucceed((ips) => {
       validateResult(ips);
       done();
     }));
@@ -74,8 +73,7 @@ TEST(async function test_reverse_ipv6(done) {
 
   const req = dns.reverse(
     addresses.INET6_IP,
-    common.mustCall((err, domains) => {
-      assert.ifError(err);
+    common.mustSucceed((domains) => {
       validateResult(domains);
       done();
     }));
@@ -94,8 +92,7 @@ TEST(async function test_lookup_ipv6_explicit(done) {
   const req = dns.lookup(
     addresses.INET6_HOST,
     6,
-    common.mustCall((err, ip, family) => {
-      assert.ifError(err);
+    common.mustSucceed((ip, family) => {
       validateResult({ address: ip, family });
       done();
     }));
@@ -126,8 +123,7 @@ TEST(async function test_lookup_ipv6_explicit_object(done) {
 
   const req = dns.lookup(addresses.INET6_HOST, {
     family: 6
-  }, common.mustCall((err, ip, family) => {
-    assert.ifError(err);
+  }, common.mustSucceed((ip, family) => {
     validateResult({ address: ip, family });
     done();
   }));
@@ -173,8 +169,7 @@ TEST(async function test_lookup_ip_ipv6(done) {
 
   const req = dns.lookup(
     '::1',
-    common.mustCall((err, ip, family) => {
-      assert.ifError(err);
+    common.mustSucceed((ip, family) => {
       validateResult({ address: ip, family });
       done();
     }));
@@ -202,8 +197,7 @@ TEST(async function test_lookup_all_ipv6(done) {
   const req = dns.lookup(
     addresses.INET6_HOST,
     { all: true, family: 6 },
-    common.mustCall((err, ips) => {
-      assert.ifError(err);
+    common.mustSucceed((ips) => {
       validateResult(ips);
       done();
     })

test/internet/test-http2-issue-32922.js

@@ -1,6 +1,5 @@
 'use strict';
 const common = require('../common');
-const assert = require('assert');
 
 if (!common.hasCrypto)
   common.skip('missing crypto');
@@ -29,9 +28,7 @@ function normalSession(cb) {
     });
   });
 }
-normalSession(common.mustCall(function(err) {
-  assert.ifError(err);
-}));
+normalSession(common.mustSucceed());
 
 // Create a session using a socket that has not yet finished connecting
 function socketNotFinished(done) {
@@ -52,9 +49,7 @@ function socketNotFinished(done) {
     });
   });
 }
-socketNotFinished(common.mustCall(function(err) {
-  assert.ifError(err);
-}));
+socketNotFinished(common.mustSucceed());
 
 // Create a session using a socket that has finished connecting
 function socketFinished(done) {
@@ -75,6 +70,4 @@ function socketFinished(done) {
     });
   });
 }
-socketFinished(common.mustCall(function(err) {
-  assert.ifError(err);
-}));
+socketFinished(common.mustSucceed());

test/parallel/test-c-ares.js

@@ -43,20 +43,17 @@ const dnsPromises = dns.promises;
 })().then(common.mustCall());
 
 // Try resolution without hostname.
-dns.lookup(null, common.mustCall((error, result, addressType) => {
-  assert.ifError(error);
+dns.lookup(null, common.mustSucceed((result, addressType) => {
   assert.strictEqual(result, null);
   assert.strictEqual(addressType, 4);
 }));
 
-dns.lookup('127.0.0.1', common.mustCall((error, result, addressType) => {
-  assert.ifError(error);
+dns.lookup('127.0.0.1', common.mustSucceed((result, addressType) => {
   assert.strictEqual(result, '127.0.0.1');
   assert.strictEqual(addressType, 4);
 }));
 
-dns.lookup('::1', common.mustCall((error, result, addressType) => {
-  assert.ifError(error);
+dns.lookup('::1', common.mustSucceed((result, addressType) => {
   assert.strictEqual(result, '::1');
   assert.strictEqual(addressType, 6);
 }));
@@ -86,8 +83,7 @@ dns.lookup('::1', common.mustCall((error, result, addressType) => {
 // so we disable this test on Windows.
 // IBMi reports `ENOTFOUND` when get hostname by address 127.0.0.1
 if (!common.isWindows && !common.isIBMi) {
-  dns.reverse('127.0.0.1', common.mustCall(function(error, domains) {
-    assert.ifError(error);
+  dns.reverse('127.0.0.1', common.mustSucceed((domains) => {
     assert.ok(Array.isArray(domains));
   }));
 

test/parallel/test-child-process-exec-any-shells-windows.js

@@ -17,8 +17,8 @@ fs.mkdirSync(tmpPath);
 
 const test = (shell) => {
   cp.exec('echo foo bar', { shell: shell },
-          common.mustCall((error, stdout, stderror) => {
-            assert.ok(!error && !stderror);
+          common.mustSucceed((stdout, stderror) => {
+            assert.ok(!stderror);
             assert.ok(stdout.includes('foo') && stdout.includes('bar'));
           }));
 };
@@ -43,12 +43,11 @@ test('CMD');
 test('powershell');
 testCopy('powershell.exe',
          `${system32}\\WindowsPowerShell\\v1.0\\powershell.exe`);
-fs.writeFile(`${tmpPath}\\test file`, 'Test', common.mustCall((err) => {
-  assert.ifError(err);
+fs.writeFile(`${tmpPath}\\test file`, 'Test', common.mustSucceed(() => {
   cp.exec(`Get-ChildItem "${tmpPath}" | Select-Object -Property Name`,
           { shell: 'PowerShell' },
-          common.mustCall((error, stdout, stderror) => {
-            assert.ok(!error && !stderror);
+          common.mustSucceed((stdout, stderror) => {
+            assert.ok(!stderror);
             assert.ok(stdout.includes(
               'test file'));
           }));

test/parallel/test-child-process-exec-cwd.js

@@ -34,7 +34,6 @@ if (common.isWindows) {
   dir = '/dev';
 }
 
-exec(pwdcommand, { cwd: dir }, common.mustCall(function(err, stdout, stderr) {
-  assert.ifError(err);
+exec(pwdcommand, { cwd: dir }, common.mustSucceed((stdout, stderr) => {
   assert(stdout.startsWith(dir));
 }));

test/parallel/test-child-process-exec-encoding.js

@@ -15,8 +15,7 @@ if (process.argv[2] === 'child') {
   function run(options, callback) {
     const cmd = `"${process.execPath}" "${__filename}" child`;
 
-    cp.exec(cmd, options, common.mustCall((err, stdout, stderr) => {
-      assert.ifError(err);
+    cp.exec(cmd, options, common.mustSucceed((stdout, stderr) => {
       callback(stdout, stderr);
     }));
   }