From 72c550e0fdefc01dd685ddcc0a6ca0de0f9ff5b6 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 7 Oct 2025 14:59:41 +0200 Subject: [PATCH] test: ensure assertions are reachable in `test/addons` --- test/addons/async-resource/test.js | 7 ++-- test/addons/cppgc-object/test.js | 4 +-- test/addons/make-callback-recurse/test.js | 4 +-- test/addons/no-addons/permission.js | 42 +++++------------------ test/addons/no-addons/test-worker.js | 12 +++---- test/addons/no-addons/test.js | 42 +++++------------------ test/addons/null-buffer-neuter/test.js | 4 +-- test/addons/symlinked-module/submodule.js | 6 ++-- test/eslint.config_partial.mjs | 16 ++++++++- 9 files changed, 51 insertions(+), 86 deletions(-) diff --git a/test/addons/async-resource/test.js b/test/addons/async-resource/test.js index 86cbb38e1cf2df..d7bc44a92be827 100644 --- a/test/addons/async-resource/test.js +++ b/test/addons/async-resource/test.js @@ -17,7 +17,7 @@ let after = 0; let destroy = 0; async_hooks.createHook({ - init(id, type, triggerAsyncId, resource) { + init: common.mustCall((id, type, triggerAsyncId, resource) => { assert.strictEqual(typeof id, 'number'); assert.strictEqual(typeof resource, 'object'); assert(id > 1); @@ -26,7 +26,7 @@ async_hooks.createHook({ assert.strictEqual(triggerAsyncId, expectedTriggerId); bindingUids.push(id); } - }, + }, 7), before(id) { if (bindingUids.includes(id)) before++; @@ -48,8 +48,11 @@ for (const call of [binding.callViaFunction, let uid; const object = { methöd(arg) { + // eslint-disable-next-line node-core/must-call-assert assert.strictEqual(this, object); + // eslint-disable-next-line node-core/must-call-assert assert.strictEqual(arg, 42); + // eslint-disable-next-line node-core/must-call-assert assert.strictEqual(async_hooks.executionAsyncId(), uid); return 'baz'; }, diff --git a/test/addons/cppgc-object/test.js b/test/addons/cppgc-object/test.js index e18b6e17b88df9..acdb145ae721c4 100644 --- a/test/addons/cppgc-object/test.js +++ b/test/addons/cppgc-object/test.js @@ -23,7 +23,7 @@ for (let i = 0; i < count; ++i) { globalThis.gc(); -setTimeout(async function() { +setTimeout(common.mustCall(() => (async function() { // GC should have invoked Trace() on at least some of the CppGCed objects, // but they should all be alive at this point. assert.strictEqual(states[kDestructCount], 0); @@ -48,4 +48,4 @@ setTimeout(async function() { 'All old CppGCed are destroyed', () => states[kDestructCount] === count * 2, ); -}, 1); +})().then(common.mustCall())), 1); diff --git a/test/addons/make-callback-recurse/test.js b/test/addons/make-callback-recurse/test.js index 4a540003acd8d1..6b927ffbfdba48 100644 --- a/test/addons/make-callback-recurse/test.js +++ b/test/addons/make-callback-recurse/test.js @@ -71,11 +71,11 @@ assert.throws(() => { if (arg === 1) { // The tests are first run on bootstrap during LoadEnvironment() in // src/node.cc. Now run the tests through node::MakeCallback(). - setImmediate(() => { + setImmediate(common.mustCall(() => { makeCallback({}, common.mustCall(() => { verifyExecutionOrder(2); })); - }); + })); } else if (arg === 2) { // Make sure there are no conflicts using node::MakeCallback() // within timers. diff --git a/test/addons/no-addons/permission.js b/test/addons/no-addons/permission.js index 1d1bbf6e95468e..a487c8a6c9b2af 100644 --- a/test/addons/no-addons/permission.js +++ b/test/addons/no-addons/permission.js @@ -7,37 +7,11 @@ const assert = require('assert'); const bindingPath = require.resolve(`./build/${common.buildType}/binding`); -const assertError = (error) => { - assert(error instanceof Error); - assert.strictEqual(error.code, 'ERR_DLOPEN_DISABLED'); - assert.strictEqual( - error.message, - 'Cannot load native addon because loading addons is disabled.', - ); -}; - -{ - let threw = false; - - try { - require(bindingPath); - } catch (error) { - assertError(error); - threw = true; - } - - assert(threw); -} - -{ - let threw = false; - - try { - process.dlopen({ exports: {} }, bindingPath); - } catch (error) { - assertError(error); - threw = true; - } - - assert(threw); -} +assert.throws(() => require(bindingPath), { + code: 'ERR_DLOPEN_DISABLED', + message: 'Cannot load native addon because loading addons is disabled.', +}); +assert.throws(() => process.dlopen({ exports: {} }, bindingPath), { + code: 'ERR_DLOPEN_DISABLED', + message: 'Cannot load native addon because loading addons is disabled.', +}); diff --git a/test/addons/no-addons/test-worker.js b/test/addons/no-addons/test-worker.js index 25e893ca33aea7..fd932d64f440aa 100644 --- a/test/addons/no-addons/test-worker.js +++ b/test/addons/no-addons/test-worker.js @@ -9,13 +9,13 @@ const { Worker } = require('worker_threads'); const binding = path.resolve(__dirname, `./build/${common.buildType}/binding`); -const assertError = (error) => { +const assertError = common.mustCall((error) => { assert.strictEqual(error.code, 'ERR_DLOPEN_DISABLED'); assert.strictEqual( error.message, 'Cannot load native addon because loading addons is disabled.', ); -}; +}, 4); { // Flags should be inherited @@ -23,7 +23,7 @@ const assertError = (error) => { eval: true, }); - worker.on('error', common.mustCall(assertError)); + worker.on('error', assertError); } { @@ -35,7 +35,7 @@ const assertError = (error) => { }, ); - worker.on('error', common.mustCall(assertError)); + worker.on('error', assertError); } { @@ -45,7 +45,7 @@ const assertError = (error) => { execArgv: ['--no-addons'], }); - worker.on('error', common.mustCall(assertError)); + worker.on('error', assertError); } { @@ -55,5 +55,5 @@ const assertError = (error) => { execArgv: [], }); - worker.on('error', common.mustCall(assertError)); + worker.on('error', assertError); } diff --git a/test/addons/no-addons/test.js b/test/addons/no-addons/test.js index 485fad4a8bc643..7b93806d6db046 100644 --- a/test/addons/no-addons/test.js +++ b/test/addons/no-addons/test.js @@ -7,37 +7,11 @@ const assert = require('assert'); const bindingPath = require.resolve(`./build/${common.buildType}/binding`); -const assertError = (error) => { - assert(error instanceof Error); - assert.strictEqual(error.code, 'ERR_DLOPEN_DISABLED'); - assert.strictEqual( - error.message, - 'Cannot load native addon because loading addons is disabled.', - ); -}; - -{ - let threw = false; - - try { - require(bindingPath); - } catch (error) { - assertError(error); - threw = true; - } - - assert(threw); -} - -{ - let threw = false; - - try { - process.dlopen({ exports: {} }, bindingPath); - } catch (error) { - assertError(error); - threw = true; - } - - assert(threw); -} +assert.throws(() => require(bindingPath), { + code: 'ERR_DLOPEN_DISABLED', + message: 'Cannot load native addon because loading addons is disabled.', +}); +assert.throws(() => process.dlopen({ exports: {} }, bindingPath), { + code: 'ERR_DLOPEN_DISABLED', + message: 'Cannot load native addon because loading addons is disabled.', +}); diff --git a/test/addons/null-buffer-neuter/test.js b/test/addons/null-buffer-neuter/test.js index d99690542a4d84..7a3ba1535ec9da 100644 --- a/test/addons/null-buffer-neuter/test.js +++ b/test/addons/null-buffer-neuter/test.js @@ -6,6 +6,6 @@ const binding = require(`./build/${common.buildType}/binding`); binding.run(); global.gc(); -setImmediate(() => { +setImmediate(common.mustCall(() => { assert.strictEqual(binding.isAlive(), 0); -}); +})); diff --git a/test/addons/symlinked-module/submodule.js b/test/addons/symlinked-module/submodule.js index d4b59e9efa4f5e..79400ecf4c10c8 100644 --- a/test/addons/symlinked-module/submodule.js +++ b/test/addons/symlinked-module/submodule.js @@ -1,13 +1,13 @@ 'use strict'; -require('../../common'); +const common = require('../../common'); const path = require('path'); const assert = require('assert'); // This is a subtest of symlinked-module/test.js. This is not // intended to be run directly. -module.exports.test = function test(bindingDir) { +module.exports.test = common.mustCall(function test(bindingDir) { const mod = require(path.join(bindingDir, 'binding.node')); assert.notStrictEqual(mod, null); assert.strictEqual(mod.hello(), 'world'); -}; +}, require.main === module ? 0 : 2); diff --git a/test/eslint.config_partial.mjs b/test/eslint.config_partial.mjs index 915a8b97b347ef..0980ffac7a28a1 100644 --- a/test/eslint.config_partial.mjs +++ b/test/eslint.config_partial.mjs @@ -156,7 +156,21 @@ export default [ }, { files: [ - 'test/{async-hooks,benchmark,cctest,client-proxy,message,module-hooks,node-api,pummel,pseudo-tty,v8-updates,wasi}/**/*.{js,mjs,cjs}', + `test/{${[ + 'abort', + 'addons', + 'async-hooks', + 'benchmark', + 'cctest', + 'client-proxy', + 'message', + 'module-hooks', + 'node-api', + 'pummel', + 'pseudo-tty', + 'v8-updates', + 'wasi', + ].join(',')}}/**/*.{js,mjs,cjs}`, ], rules: { 'node-core/must-call-assert': 'error',