From 4b289d047d2b7ee1e6c056e5abc1c3e43d34f9be Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Wed, 23 Jul 2025 18:28:06 +0200 Subject: [PATCH] test: expand linting rules around `assert` w literal messages Expands the existing restrictions around not using literal messages to the `not` variants of `strictEqual` and `deepStrictEqual`, and forbids `assert()` where the second argument is a non-string literal, which almost always is a mis-typed `assert.strictEquals()`. PR-URL: https://github.com/nodejs/node/pull/59147 Reviewed-By: James M Snell Reviewed-By: Luigi Pinca Reviewed-By: Chemi Atlow --- test/addons/report-fatalerror/test.js | 4 ++-- test/eslint.config_partial.mjs | 12 ++++++++++++ test/js-native-api/test_object/test.js | 6 +++--- test/parallel/test-buffer-write-fast.js | 2 +- .../test-net-listen-exclusive-random-ports.js | 2 +- test/parallel/test-stream-toArray.js | 6 +++--- .../test-report-fatalerror-oomerror-compact.js | 2 +- .../test-report-fatalerror-oomerror-directory.js | 2 +- .../test-report-fatalerror-oomerror-filename.js | 2 +- .../test-report-fatalerror-oomerror-not-set.js | 2 +- test/report/test-report-fatalerror-oomerror-set.js | 2 +- 11 files changed, 27 insertions(+), 15 deletions(-) diff --git a/test/addons/report-fatalerror/test.js b/test/addons/report-fatalerror/test.js index 96ee7223c0f..b5018565ea2 100644 --- a/test/addons/report-fatalerror/test.js +++ b/test/addons/report-fatalerror/test.js @@ -27,7 +27,7 @@ const ARGS = [ tmpdir.refresh(); const args = ['--report-on-fatalerror', ...ARGS]; const child = spawnSync(process.execPath, args, { cwd: tmpdir.path }); - assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly'); + assert.notStrictEqual(child.status, 0); const reports = helper.findReports(child.pid, tmpdir.path); assert.strictEqual(reports.length, 1); @@ -46,7 +46,7 @@ const ARGS = [ // Verify that --report-on-fatalerror is respected when not set. const args = ARGS; const child = spawnSync(process.execPath, args, { cwd: tmpdir.path }); - assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly'); + assert.notStrictEqual(child.status, 0); const reports = helper.findReports(child.pid, tmpdir.path); assert.strictEqual(reports.length, 0); } diff --git a/test/eslint.config_partial.mjs b/test/eslint.config_partial.mjs index 889d2f5cddd..9568dc9d1c9 100644 --- a/test/eslint.config_partial.mjs +++ b/test/eslint.config_partial.mjs @@ -36,6 +36,10 @@ export default [ selector: "CallExpression:matches([callee.name='deepStrictEqual'], [callee.property.name='deepStrictEqual'])[arguments.2.type='Literal']", message: 'Do not use a literal for the third argument of assert.deepStrictEqual()', }, + { + selector: "CallExpression:matches([callee.name='notDeepStrictEqual'], [callee.property.name='deepStrictEqual'])[arguments.2.type='Literal']", + message: 'Do not use a literal for the third argument of assert.notDeepStrictEqual()', + }, { selector: "CallExpression:matches([callee.name='doesNotThrow'], [callee.property.name='doesNotThrow'])", message: 'Do not use `assert.doesNotThrow()`. Write the code without the wrapper and add a comment instead.', @@ -48,10 +52,18 @@ export default [ selector: "CallExpression:matches([callee.name='rejects'], [callee.property.name='rejects'])[arguments.length<2]", message: '`assert.rejects()` must be invoked with at least two arguments.', }, + { + selector: "CallExpression[callee.property.name='notStrictEqual'][arguments.2.type='Literal']", + message: 'Do not use a literal for the third argument of assert.notStrictEqual()', + }, { selector: "CallExpression[callee.property.name='strictEqual'][arguments.2.type='Literal']", message: 'Do not use a literal for the third argument of assert.strictEqual()', }, + { + selector: "CallExpression[callee.name='assert'][arguments.1.type='Literal']:not([arguments.1.raw=/['\"`].*/])", + message: 'Do not use a non-string literal for the second argument of assert()', + }, { selector: "CallExpression:matches([callee.name='throws'], [callee.property.name='throws'])[arguments.1.type='Literal']:not([arguments.1.regex])", message: 'Use an object as second argument of `assert.throws()`.', diff --git a/test/js-native-api/test_object/test.js b/test/js-native-api/test_object/test.js index 670ae200a35..a745d45e6f7 100644 --- a/test/js-native-api/test_object/test.js +++ b/test/js-native-api/test_object/test.js @@ -140,7 +140,7 @@ assert.strictEqual(newObject.test_string, 'test string'); test_object.Wrap(wrapper); assert(test_object.Unwrap(wrapper)); - assert(wrapper.protoA); + assert.strictEqual(wrapper.protoA, true); } { @@ -155,8 +155,8 @@ assert.strictEqual(newObject.test_string, 'test string'); Object.setPrototypeOf(wrapper, protoB); assert(test_object.Unwrap(wrapper)); - assert(wrapper.protoA, true); - assert(wrapper.protoB, true); + assert.strictEqual(wrapper.protoA, true); + assert.strictEqual(wrapper.protoB, true); } { diff --git a/test/parallel/test-buffer-write-fast.js b/test/parallel/test-buffer-write-fast.js index 4594934f758..4f385992ed6 100644 --- a/test/parallel/test-buffer-write-fast.js +++ b/test/parallel/test-buffer-write-fast.js @@ -41,5 +41,5 @@ testFastUtf8Write(); if (common.isDebug) { const { getV8FastApiCallCount } = internalBinding('debug'); - assert(getV8FastApiCallCount('buffer.writeString'), 4); + assert.strictEqual(getV8FastApiCallCount('buffer.writeString'), 4); } diff --git a/test/parallel/test-net-listen-exclusive-random-ports.js b/test/parallel/test-net-listen-exclusive-random-ports.js index 66dfb598204..b9e92a750db 100644 --- a/test/parallel/test-net-listen-exclusive-random-ports.js +++ b/test/parallel/test-net-listen-exclusive-random-ports.js @@ -16,7 +16,7 @@ if (cluster.isPrimary) { worker2.on('message', function(port2) { assert.strictEqual(port2, port2 | 0, `second worker could not listen on port ${port2}`); - assert.notStrictEqual(port1, port2, 'ports should not be equal'); + assert.notStrictEqual(port1, port2); worker1.kill(); worker2.kill(); }); diff --git a/test/parallel/test-stream-toArray.js b/test/parallel/test-stream-toArray.js index 5c86410ed74..438e21724e4 100644 --- a/test/parallel/test-stream-toArray.js +++ b/test/parallel/test-stream-toArray.js @@ -59,7 +59,7 @@ const assert = require('assert'); const ac = new AbortController(); let stream; assert.rejects(async () => { - stream = Readable.from([1, 2, 3]).map(async (x) => { + stream = Readable.from([1, 2, 3, 4]).map(async (x) => { if (x === 3) { await new Promise(() => {}); // Explicitly do not pass signal here } @@ -69,8 +69,8 @@ const assert = require('assert'); }, { name: 'AbortError', }).then(common.mustCall(() => { - // Only stops toArray, does not destroy the stream - assert(stream.destroyed, false); + // Stops toArray *and* destroys the stream + assert.strictEqual(stream.destroyed, true); })); ac.abort(); } diff --git a/test/report/test-report-fatalerror-oomerror-compact.js b/test/report/test-report-fatalerror-oomerror-compact.js index 66bc1fa88e6..efe91f0bb4b 100644 --- a/test/report/test-report-fatalerror-oomerror-compact.js +++ b/test/report/test-report-fatalerror-oomerror-compact.js @@ -25,7 +25,7 @@ const REPORT_FIELDS = [ tmpdir.refresh(); const args = ['--report-on-fatalerror', '--report-compact', ...ARGS]; const child = spawnSync(process.execPath, args, { cwd: tmpdir.path }); - assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly'); + assert.notStrictEqual(child.status, 0); const reports = helper.findReports(child.pid, tmpdir.path); assert.strictEqual(reports.length, 1); diff --git a/test/report/test-report-fatalerror-oomerror-directory.js b/test/report/test-report-fatalerror-oomerror-directory.js index 39ba7a1ca4b..a0ba3ee9f81 100644 --- a/test/report/test-report-fatalerror-oomerror-directory.js +++ b/test/report/test-report-fatalerror-oomerror-directory.js @@ -27,7 +27,7 @@ const REPORT_FIELDS = [ const dir = '--report-directory=' + tmpdir.path; const args = ['--report-on-fatalerror', dir, ...ARGS]; const child = spawnSync(process.execPath, args, { }); - assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly'); + assert.notStrictEqual(child.status, 0); const reports = helper.findReports(child.pid, tmpdir.path); assert.strictEqual(reports.length, 1); diff --git a/test/report/test-report-fatalerror-oomerror-filename.js b/test/report/test-report-fatalerror-oomerror-filename.js index 9c3bb7e4d1a..fd0a6a5f378 100644 --- a/test/report/test-report-fatalerror-oomerror-filename.js +++ b/test/report/test-report-fatalerror-oomerror-filename.js @@ -30,7 +30,7 @@ const REPORT_FIELDS = [ ...ARGS, ]; const child = spawnSync(process.execPath, args, { encoding: 'utf8' }); - assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly'); + assert.notStrictEqual(child.status, 0); const reports = helper.findReports(child.pid, tmpdir.path); assert.strictEqual(reports.length, 0); diff --git a/test/report/test-report-fatalerror-oomerror-not-set.js b/test/report/test-report-fatalerror-oomerror-not-set.js index a54003ac719..79fb81771b2 100644 --- a/test/report/test-report-fatalerror-oomerror-not-set.js +++ b/test/report/test-report-fatalerror-oomerror-not-set.js @@ -20,7 +20,7 @@ const ARGS = [ // Verify that --report-on-fatalerror is respected when not set. const args = ARGS; const child = spawnSync(process.execPath, args, { cwd: tmpdir.path }); - assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly'); + assert.notStrictEqual(child.status, 0); const reports = helper.findReports(child.pid, tmpdir.path); assert.strictEqual(reports.length, 0); } diff --git a/test/report/test-report-fatalerror-oomerror-set.js b/test/report/test-report-fatalerror-oomerror-set.js index 1a05f83d4e3..3363ada155f 100644 --- a/test/report/test-report-fatalerror-oomerror-set.js +++ b/test/report/test-report-fatalerror-oomerror-set.js @@ -24,7 +24,7 @@ const REPORT_FIELDS = [ tmpdir.refresh(); const args = ['--report-on-fatalerror', ...ARGS]; const child = spawnSync(process.execPath, args, { cwd: tmpdir.path }); - assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly'); + assert.notStrictEqual(child.status, 0); const reports = helper.findReports(child.pid, tmpdir.path); assert.strictEqual(reports.length, 1);