tools: add lint rule to ensure assertions are reached

test/eslint.config_partial.mjs

@@ -154,6 +154,14 @@ export default [
       ],
     },
   },
+  {
+    files: [
+      'test/{message,module-hooks,node-api,pummel,pseudo-tty,v8-updates,wasi}/**/*.{js,mjs,cjs}',
+    ],
+    rules: {
+      'node-core/must-call-assert': 'error',
+    },
+  },
   {
     files: [
       'test/{common,fixtures,wpt}/**/*.{js,mjs,cjs}',

test/message/assert_throws_stack.js

@@ -1,6 +1,6 @@
 'use strict';
 
 require('../common');
-const assert = require('assert').strict;
+const assert = require('assert/strict');
 
 assert.throws(() => { throw new Error('foo'); }, { bar: true });

test/message/node_run_non_existent.js

@@ -1,7 +1,7 @@
 'use strict';
 
 require('../common');
-const assert = require('node:assert').strict;
+const assert = require('node:assert/strict');
 const childProcess = require('node:child_process');
 const fixtures = require('../common/fixtures');
 

test/module-hooks/test-module-hooks-import-wasm.mjs

@@ -1,13 +1,13 @@
 // This tests that module.registerHooks() can be used to support unknown formats, like
 // import(wasm)
-import '../common/index.mjs';
+import { mustCall } from '../common/index.mjs';
 
 import assert from 'node:assert';
 import { registerHooks, createRequire } from 'node:module';
 import { readFileSync } from 'node:fs';
 
 registerHooks({
-  load(url, context, nextLoad) {
+  load: mustCall((url, context, nextLoad) => {
     assert.match(url, /simple\.wasm$/);
     const source =
       `const buf = Buffer.from([${Array.from(readFileSync(new URL(url))).join(',')}]);
@@ -21,7 +21,7 @@ registerHooks({
       source,
       format: 'module',
     };
-  },
+  }),
 });
 
 // Checks that it works with require.

test/module-hooks/test-module-hooks-load-buffers.js

@@ -1,6 +1,6 @@
 'use strict';
 
-require('../common');
+const common = require('../common');
 const assert = require('assert');
 const { registerHooks } = require('module');
 
@@ -15,15 +15,7 @@ const hook1 = registerHooks({
   resolve(specifier, context, nextResolve) {
     return { shortCircuit: true, url: `test://${specifier}` };
   },
-  load(url, context, nextLoad) {
-    const result = nextLoad(url, context);
-    if (url === 'test://array_buffer') {
-      assert.deepStrictEqual(result.source, encoder.encode(arrayBufferSource).buffer);
-    } else if (url === 'test://array_buffer_view') {
-      assert.deepStrictEqual(result.source, encoder.encode(arrayBufferViewSource));
-    }
-    return result;
-  },
+  load: common.mustNotCall(),
 });
 
 const hook2 = registerHooks({

test/module-hooks/test-module-hooks-load-chained.js

@@ -1,30 +1,30 @@
 'use strict';
 
-require('../common');
+const common = require('../common');
 const assert = require('assert');
 const { registerHooks } = require('module');
 
 // Test that multiple loaders works together.
 const hook1 = registerHooks({
-  load(url, context, nextLoad) {
+  load: common.mustCall((url, context, nextLoad) => {
     const result = nextLoad(url, context);
     assert.strictEqual(result.source, '');
     return {
       source: 'exports.hello = "world"',
       format: 'commonjs',
     };
-  },
+  }),
 });
 
 const hook2 = registerHooks({
-  load(url, context, nextLoad) {
+  load: common.mustCall((url, context, nextLoad) => {
     const result = nextLoad(url, context);
     assert.strictEqual(result.source, 'exports.hello = "world"');
     return {
       source: 'export const hello = "world"',
       format: 'module',
     };
-  },
+  }),
 });
 
 const mod = require('../fixtures/empty.js');

test/module-hooks/test-module-hooks-load-detection.js

@@ -1,18 +1,18 @@
 'use strict';
 
-require('../common');
+const common = require('../common');
 const assert = require('assert');
 const { registerHooks } = require('module');
 
 // Test that module syntax detection works.
 const hook = registerHooks({
-  load(url, context, nextLoad) {
+  load: common.mustCall((url, context, nextLoad) => {
     const result = nextLoad(url, context);
     assert.strictEqual(result.source, '');
     return {
       source: 'export const hello = "world"',
     };
-  },
+  }),
 });
 
 const mod = require('../fixtures/empty.js');

test/module-hooks/test-module-hooks-load-url-change-import.mjs

@@ -7,13 +7,13 @@ import { fileURL } from '../common/fixtures.mjs';
 // It changes `foo` package name into `redirected-fs` and then loads `redirected-fs`
 
 const hook = registerHooks({
-  resolve(specifier, context, nextResolve) {
+  resolve: mustCall((specifier, context, nextResolve) => {
     assert.strictEqual(specifier, 'foo');
     return {
       url: 'foo://bar',
       shortCircuit: true,
     };
-  },
+  }),
   load: mustCall(function load(url, context, nextLoad) {
     assert.strictEqual(url, 'foo://bar');
     return nextLoad(fileURL('module-hooks', 'redirected-fs.js').href, context);

test/module-hooks/test-module-hooks-load-url-change-require.js

@@ -9,13 +9,13 @@ const fixtures = require('../common/fixtures');
 // It changes `foo` package name into `redirected-fs` and then loads `redirected-fs`
 
 const hook = registerHooks({
-  resolve(specifier, context, nextResolve) {
+  resolve: common.mustCall((specifier, context, nextResolve) => {
     assert.strictEqual(specifier, 'foo');
     return {
       url: 'foo://bar',
       shortCircuit: true,
     };
-  },
+  }),
   load: common.mustCall(function load(url, context, nextLoad) {
     assert.strictEqual(url, 'foo://bar');
     return nextLoad(

test/module-hooks/test-module-hooks-require-wasm.js

@@ -9,7 +9,7 @@ const { registerHooks } = require('module');
 const { readFileSync } = require('fs');
 
 registerHooks({
-  load(url, context, nextLoad) {
+  load: common.mustCall((url, context, nextLoad) => {
     assert.match(url, /simple\.wasm$/);
     const source =
       `const buf = Buffer.from([${Array.from(readFileSync(new URL(url))).join(',')}]);
@@ -20,7 +20,7 @@ registerHooks({
       source,
       format: 'commonjs',
     };
-  },
+  }, 2),
 });
 
 // Checks that it works with require.

test/module-hooks/test-module-hooks-resolve-builtin-builtin-require.js

@@ -21,6 +21,7 @@ const hook = registerHooks({
 
 // Check assert, which is already loaded.
 // zlib.createGzip is a function.
+// eslint-disable-next-line node-core/must-call-assert
 assert.strictEqual(typeof require('assert').createGzip, 'function');
 
 hook.deregister();

test/module-hooks/test-module-hooks-resolve-builtin-on-disk-require.js

@@ -20,6 +20,7 @@ const hook = registerHooks({
 });
 
 // Check assert, which is already loaded.
+// eslint-disable-next-line node-core/must-call-assert
 assert.strictEqual(require('assert').exports_for_test, 'redirected assert');
 // Check zlib, which is not yet loaded.
 assert.strictEqual(require('zlib').exports_for_test, 'redirected zlib');

test/node-api/test_async_context/test-gcable-callback.js

@@ -46,17 +46,17 @@ for (let i = 0; i < arr.length; i++)
   arr[i] = {};
 
 assert.strictEqual(hook_result.destroy_called, false);
-setImmediate(() => {
+setImmediate(common.mustCall(() => {
   assert.strictEqual(hook_result.destroy_called, false);
-  makeCallback(asyncResource, process, () => {
+  makeCallback(asyncResource, process, common.mustCall(() => {
     const executionAsyncResource = async_hooks.executionAsyncResource();
     // Previous versions of Node-API would have gargbage-collected
     // the `asyncResource` object, now we can just assert that it is intact.
     assert.strictEqual(typeof executionAsyncResource, 'object');
     assert.strictEqual(executionAsyncResource.foo, 'bar');
     destroyAsyncResource(asyncResource);
-    setImmediate(() => {
+    setImmediate(common.mustCall(() => {
       assert.strictEqual(hook_result.destroy_called, true);
-    });
-  });
-});
+    }));
+  }));
+}));

test/node-api/test_async_context/test.js

@@ -40,24 +40,24 @@ const resourceWrap = createAsyncResource(
 
 assert.strictEqual(hook_result.destroy_called, false);
 const recv = {};
-makeCallback(resourceWrap, recv, function callback() {
+makeCallback(resourceWrap, recv, common.mustCall(function callback() {
   assert.strictEqual(hook_result.destroy_called, false);
   assert.strictEqual(
     hook_result.resource,
     async_hooks.executionAsyncResource(),
   );
   assert.strictEqual(this, recv);
 
-  setImmediate(() => {
+  setImmediate(common.mustCall(() => {
     assert.strictEqual(hook_result.destroy_called, false);
     assert.notStrictEqual(
       hook_result.resource,
       async_hooks.executionAsyncResource(),
     );
 
     destroyAsyncResource(resourceWrap);
-    setImmediate(() => {
+    setImmediate(common.mustCall(() => {
       assert.strictEqual(hook_result.destroy_called, true);
-    });
-  });
-});
+    }));
+  }));
+}));

test/node-api/test_exception/test.js

@@ -11,6 +11,7 @@ function testFinalize(binding) {
   x = null;
   global.gc();
   process.on('uncaughtException', (err) => {
+    // eslint-disable-next-line node-core/must-call-assert
     assert.strictEqual(err.message, 'Error during Finalize');
   });
 

test/node-api/test_make_callback/test-async-hooks.js

@@ -39,15 +39,15 @@ test_hook.enable();
  * slots. Testing with plain object here.
  */
 const resource = {};
-makeCallback(resource, process, function cb() {
+makeCallback(resource, process, common.mustCall(function cb() {
   assert.strictEqual(this, process);
   assert.strictEqual(async_hooks.executionAsyncResource(), resource);
-});
+}));
 
 assert.strictEqual(hook_result.init_called, true);
 assert.strictEqual(hook_result.before_called, true);
 assert.strictEqual(hook_result.after_called, true);
-setImmediate(() => {
+setImmediate(common.mustCall(() => {
   assert.strictEqual(hook_result.destroy_called, true);
   test_hook.disable();
-});
+}));

test/node-api/test_make_callback_recurse/test.js

@@ -72,11 +72,11 @@ assert.throws(function() {
     if (arg === 1) {
       // The tests are first run on bootstrap during LoadEnvironment() in
       // src/node.cc. Now run the tests through node::MakeCallback().
-      setImmediate(function() {
+      setImmediate(common.mustCall(function() {
         makeCallback({}, common.mustCall(function() {
           verifyExecutionOrder(2);
         }));
-      });
+      }));
     } else if (arg === 2) {
       // Make sure there are no conflicts using node::MakeCallback()
       // within timers.

test/node-api/test_reference_by_node_api_version/test.js

@@ -7,7 +7,7 @@
 // and symbol types, while in newer versions they can be created for
 // any value type.
 //
-const { buildType } = require('../../common');
+const { mustCall, buildType } = require('../../common');
 const { gcUntil } = require('../../common/gc');
 const assert = require('assert');
 const addon_v8 = require(`./build/${buildType}/test_reference_obj_only`);
@@ -122,4 +122,4 @@ async function runAllTests() {
   await runTests(addon_new, /* isVersion8 */ false, /* isLocalSymbol */ false);
 }
 
-runAllTests();
+runAllTests().then(mustCall());

test/node-api/test_threadsafe_function/test.js

@@ -44,9 +44,9 @@ function testWithJSMarshaller({
       array.push(value);
       if (array.length === quitAfter) {
         setImmediate(() => {
-          binding.StopThread(common.mustCall(() => {
+          binding.StopThread(() => {
             resolve(array);
-          }), !!abort);
+          }, !!abort);
         });
       }
     }, !!abort, !!launchSecondary, maxQueueSize);
@@ -79,19 +79,19 @@ function testUnref(queueSize) {
 
 new Promise(function testWithoutJSMarshaller(resolve) {
   let callCount = 0;
-  binding.StartThreadNoNative(function testCallback() {
+  binding.StartThreadNoNative(common.mustCallAtLeast(function testCallback() {
     callCount++;
 
     // The default call-into-JS implementation passes no arguments.
     assert.strictEqual(arguments.length, 0);
     if (callCount === binding.ARRAY_LENGTH) {
       setImmediate(() => {
-        binding.StopThread(common.mustCall(() => {
+        binding.StopThread(() => {
           resolve();
-        }), false);
+        }, false);
       });
     }
-  }, false /* abort */, false /* launchSecondary */, binding.MAX_QUEUE_SIZE);
+  }), false /* abort */, false /* launchSecondary */, binding.MAX_QUEUE_SIZE);
 })
 
 // Start the thread in blocking mode, and assert that all values are passed.
@@ -224,4 +224,6 @@ new Promise(function testWithoutJSMarshaller(resolve) {
 .then(() => testUnref(binding.MAX_QUEUE_SIZE))
 
 // Start a child process with an infinite queue to test rapid teardown
-.then(() => testUnref(0));
+.then(() => testUnref(0))
+
+.then(common.mustCall());