From abcaf568ce36bef29fad99c009ccfc5c98412e72 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Thu, 13 Sep 2018 13:59:13 -0700 Subject: [PATCH 01/12] test: remove string literal message from assertion Remove string literal message in assert.strictEqual() call in napi test testFinalizer.js. --- test/addons-napi/test_general/testFinalizer.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/addons-napi/test_general/testFinalizer.js b/test/addons-napi/test_general/testFinalizer.js index d5df4c8c724f98..b440ed5e501ad8 100644 --- a/test/addons-napi/test_general/testFinalizer.js +++ b/test/addons-napi/test_general/testFinalizer.js @@ -29,5 +29,4 @@ test_general.wrap(finalizeAndWrap); test_general.addFinalizerOnly(finalizeAndWrap, common.mustCall()); finalizeAndWrap = null; global.gc(); -assert.strictEqual(test_general.derefItemWasCalled(), true, - 'finalize callback was called'); +assert.strictEqual(test_general.derefItemWasCalled(), true); From 486f6e86d6d191a0a67db8ac344cf188e51ad865 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Thu, 13 Sep 2018 13:56:36 -0700 Subject: [PATCH 02/12] test: remove string literal message in assertions Remove string literal message in assert.strictEqual() calls in test-async-await.js. --- test/async-hooks/test-async-await.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/async-hooks/test-async-await.js b/test/async-hooks/test-async-await.js index 7f88cd9b18176f..f5e886e9d50001 100644 --- a/test/async-hooks/test-async-await.js +++ b/test/async-hooks/test-async-await.js @@ -64,15 +64,15 @@ const timeout = common.platformTimeout(10); function checkPromisesInitState() { for (const initState of promisesInitState.values()) { - assert.strictEqual(initState, 'resolved', - 'promise initialized without being resolved'); + // Promise should not be initialized without being resolved. + assert.strictEqual(initState, 'resolved'); } } function checkPromisesExecutionState() { for (const executionState of promisesExecutionState.values()) { - assert.strictEqual(executionState, 'after', - 'mismatch between before and after hook calls'); + // Check for mismatch between before and after hook calls. + assert.strictEqual(executionState, 'after'); } } From 890c455a686e59ba3e3b938dac7f9455b86f8e03 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 1 Sep 2018 22:47:37 -0700 Subject: [PATCH 03/12] test: improve assertion in test-inspector.js Remove an unecessary string literal from assert.strictEqual() call in test-inspector.js. The string literal is printed instead of the value that causes an error. Removing the string literal allows the value that caused the error to be printed. This improves the troubleshooting experience when the test fails due to that assertion. --- test/sequential/test-inspector.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/sequential/test-inspector.js b/test/sequential/test-inspector.js index ba1fce25a04fad..d641015a3c9954 100644 --- a/test/sequential/test-inspector.js +++ b/test/sequential/test-inspector.js @@ -33,8 +33,7 @@ function checkBadPath(err) { } function checkException(message) { - assert.strictEqual(message.exceptionDetails, undefined, - 'An exception occurred during execution'); + assert.strictEqual(message.exceptionDetails, undefined); } function assertScopeValues({ result }, expected) { From d65e189c7994c65bfff4a8e59102d0e88242222b Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sun, 2 Sep 2018 10:55:33 -0700 Subject: [PATCH 04/12] test: simplify assertion in http2 tests In test-http2-timeout-large-write.js and test-http2-timeout-large-write-file.js: Use assert.ok() on a boolean that the test itself creates and sets, rather than assert.strictEqual(). This allows us to use a static message without running afoul of the upcoming "do not use string literals with assert.strictEqual()" lint rule. --- test/sequential/test-http2-timeout-large-write-file.js | 2 +- test/sequential/test-http2-timeout-large-write.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/sequential/test-http2-timeout-large-write-file.js b/test/sequential/test-http2-timeout-large-write-file.js index 910e7a0fc497bd..bb366cfff04091 100644 --- a/test/sequential/test-http2-timeout-large-write-file.js +++ b/test/sequential/test-http2-timeout-large-write-file.js @@ -48,7 +48,7 @@ server.on('stream', common.mustCall((stream) => { })); server.setTimeout(serverTimeout); server.on('timeout', () => { - assert.strictEqual(didReceiveData, false, 'Should not timeout'); + assert.ok(!didReceiveData, 'Should not timeout'); }); server.listen(0, common.mustCall(() => { diff --git a/test/sequential/test-http2-timeout-large-write.js b/test/sequential/test-http2-timeout-large-write.js index a15fb46af6d28a..73114776df0a0e 100644 --- a/test/sequential/test-http2-timeout-large-write.js +++ b/test/sequential/test-http2-timeout-large-write.js @@ -40,13 +40,13 @@ server.on('stream', common.mustCall((stream) => { stream.write(content); stream.setTimeout(serverTimeout); stream.on('timeout', () => { - assert.strictEqual(didReceiveData, false, 'Should not timeout'); + assert.ok(!didReceiveData, 'Should not timeout'); }); stream.end(); })); server.setTimeout(serverTimeout); server.on('timeout', () => { - assert.strictEqual(didReceiveData, false, 'Should not timeout'); + assert.ok(!didReceiveData, 'Should not timeout'); }); server.listen(0, common.mustCall(() => { From b23cb6c57c5439ba8784d09d6567174f152bac48 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 12 Sep 2018 20:16:23 -0700 Subject: [PATCH 05/12] test: refactor flag check Refactor test-vm-run-in-new-context so that check for `--expose-gc` flag will not run afoul of an upcoming lint rule that checks that string literals are not used for the `message` argument of `assert.strictEqual()`. --- test/parallel/test-vm-run-in-new-context.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-vm-run-in-new-context.js b/test/parallel/test-vm-run-in-new-context.js index e844cd6816bba3..51577668cfe7f8 100644 --- a/test/parallel/test-vm-run-in-new-context.js +++ b/test/parallel/test-vm-run-in-new-context.js @@ -26,8 +26,8 @@ const common = require('../common'); const assert = require('assert'); const vm = require('vm'); -assert.strictEqual(typeof global.gc, 'function', - 'Run this test with --expose-gc'); +if (typeof global.gc !== 'function') + assert.fail('Run this test with --expose-gc'); // Run a string const result = vm.runInNewContext('\'passed\';'); From 36769bec8cb797ea17e6c1746bcae803fa2b343b Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 12 Sep 2018 20:24:28 -0700 Subject: [PATCH 06/12] test: remove string literal from assertion Remove string literal from `assert.strictEqual()` call `message` parameter and make it a comment above the assertion instead. --- test/parallel/test-next-tick-domain.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-next-tick-domain.js b/test/parallel/test-next-tick-domain.js index 5c526197926df7..3e55ef3225fc40 100644 --- a/test/parallel/test-next-tick-domain.js +++ b/test/parallel/test-next-tick-domain.js @@ -27,5 +27,5 @@ const origNextTick = process.nextTick; require('domain'); -assert.strictEqual(origNextTick, process.nextTick, - 'Requiring domain should not change nextTick'); +// Requiring domain should not change nextTick. +assert.strictEqual(origNextTick, process.nextTick); From 26f2e991ada2ff76e1a18d3fd96d109b5eeed2b9 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 12 Sep 2018 21:08:53 -0700 Subject: [PATCH 07/12] test: remove string literal message from assertion Remove string literal from assert.strictEqual message to improve output of AssertionError. --- test/parallel/test-http-information-processing.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-http-information-processing.js b/test/parallel/test-http-information-processing.js index af589477f05dd1..43d1bdafdf4dd1 100644 --- a/test/parallel/test-http-information-processing.js +++ b/test/parallel/test-http-information-processing.js @@ -36,8 +36,8 @@ server.listen(0, function() { }); req.on('response', function(res) { - assert.strictEqual(countdown.remaining, 1, - 'Full response received before all 102 Processing'); + // Check that all 102 Processing received before full response received. + assert.strictEqual(countdown.remaining, 1); assert.strictEqual(200, res.statusCode, `Final status code was ${res.statusCode}, not 200.`); res.setEncoding('utf8'); From 798a6dd7e338c264d57649dbd4b04710d74a49ff Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 12 Sep 2018 21:10:45 -0700 Subject: [PATCH 08/12] test: remove string literal arg from assertion Remove unnecessary string literal from assert.deepStrictEqual() call. --- test/parallel/test-fs-readfile.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-fs-readfile.js b/test/parallel/test-fs-readfile.js index 118f2e43aab500..648bf692d1dcc8 100644 --- a/test/parallel/test-fs-readfile.js +++ b/test/parallel/test-fs-readfile.js @@ -54,6 +54,6 @@ for (const e of fileInfo) { fs.readFile(e.name, common.mustCall((err, buf) => { console.log(`Validating readFile on file ${e.name} of length ${e.len}`); assert.ifError(err); - assert.deepStrictEqual(buf, e.contents, 'Incorrect file contents'); + assert.deepStrictEqual(buf, e.contents); })); } From af1f7c762af2b30893295565d1aa203dfd65de67 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Thu, 13 Sep 2018 13:04:09 -0700 Subject: [PATCH 09/12] test: remove string literal from assertion Remove string literal as assertion message in call to assert.strictEqual() in test-dns-resolveany-bad-ancount. --- test/parallel/test-dns-resolveany-bad-ancount.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-dns-resolveany-bad-ancount.js b/test/parallel/test-dns-resolveany-bad-ancount.js index 4b13421b316aee..71fcbe03cd58f1 100644 --- a/test/parallel/test-dns-resolveany-bad-ancount.js +++ b/test/parallel/test-dns-resolveany-bad-ancount.js @@ -40,8 +40,8 @@ server.bind(0, common.mustCall(async () => { assert.strictEqual(err.syscall, 'queryAny'); assert.strictEqual(err.hostname, 'example.org'); const descriptor = Object.getOwnPropertyDescriptor(err, 'message'); - assert.strictEqual(descriptor.enumerable, - false, 'The error message should be non-enumerable'); + // The error message should be non-enumerable. + assert.strictEqual(descriptor.enumerable, false); server.close(); })); })); From 8572e6e5306b5cb4ceb60f0ccb45b6640520fb53 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Thu, 13 Sep 2018 13:40:11 -0700 Subject: [PATCH 10/12] test: remove string literal from assertion Remove string literal as assertion message in call to assert.strictEqual() in test-dns-lookup. --- test/parallel/test-dns-lookup.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-dns-lookup.js b/test/parallel/test-dns-lookup.js index 438dff0c396d5d..e67120b0d2521b 100644 --- a/test/parallel/test-dns-lookup.js +++ b/test/parallel/test-dns-lookup.js @@ -136,8 +136,8 @@ dns.lookup('example.com', common.mustCall((error, result, addressType) => { assert.strictEqual(tickValue, 1); assert.strictEqual(error.code, 'ENOENT'); const descriptor = Object.getOwnPropertyDescriptor(error, 'message'); - assert.strictEqual(descriptor.enumerable, - false, 'The error message should be non-enumerable'); + // The error message should be non-enumerable. + assert.strictEqual(descriptor.enumerable, false); })); // Make sure that the error callback is called From 4b9142cc9e36af9413010a82804113fa38075cf5 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Thu, 13 Sep 2018 13:47:57 -0700 Subject: [PATCH 11/12] test: prepare test-assert for strictEqual linting Make minor modifications to test-assert.js to prepare it for linting rule that forbids the use of string literals for the third argument of assert.strictEqual(). --- test/parallel/test-assert.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index bdbde655d783ab..0faffe4cbb047c 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -311,11 +311,11 @@ try { } try { - assert.strictEqual(1, 2, 'oh no'); + assert.strictEqual(1, 2, 'oh no'); // eslint-disable-line no-restricted-syntax } catch (e) { assert.strictEqual(e.message, 'oh no'); - assert.strictEqual(e.generatedMessage, false, - 'Message incorrectly marked as generated'); + // Message should not be marked as generated. + assert.strictEqual(e.generatedMessage, false); } { From 51cd1ea4788ef8f0d4b555ce6664971a6521b978 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 30 Jul 2018 16:33:41 -0700 Subject: [PATCH 12/12] tools: prevent string literals in some assertions String literals provided as the third argument to assert.strictEqual() or assert.deepStrictEqual() will mask the values that are causing issues. Use a lint rule to prevent such usage. --- .eslintrc.js | 5 +++++ lib/.eslintrc.yaml | 4 ++++ test/.eslintrc.yaml | 4 ++++ 3 files changed, 13 insertions(+) diff --git a/.eslintrc.js b/.eslintrc.js index 9c3a78d81cd27a..e4192c3539925c 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -156,6 +156,7 @@ module.exports = { ], /* eslint-disable max-len */ // If this list is modified, please copy the change to lib/.eslintrc.yaml + // and test/.eslintrc.yaml. 'no-restricted-syntax': [ 'error', { @@ -166,6 +167,10 @@ module.exports = { selector: "CallExpression[callee.object.name='assert'][callee.property.name='rejects'][arguments.length<2]", message: 'assert.rejects() must be invoked with at least two arguments.', }, + { + selector: "CallExpression[callee.object.name='assert'][callee.property.name='strictEqual'][arguments.2.type='Literal']", + message: 'Do not use a literal for the third argument of assert.strictEqual()' + }, { selector: "CallExpression[callee.object.name='assert'][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/lib/.eslintrc.yaml b/lib/.eslintrc.yaml index e7cab0ad931b4d..a1a0b9490a9177 100644 --- a/lib/.eslintrc.yaml +++ b/lib/.eslintrc.yaml @@ -2,10 +2,14 @@ rules: no-restricted-syntax: # Config copied from .eslintrc.js - error + - selector: "CallExpression[callee.object.name='assert'][callee.property.name='deepStrictEqual'][arguments.2.type='Literal']" + message: "Do not use a literal for the third argument of assert.deepStrictEqual()" - selector: "CallExpression[callee.object.name='assert'][callee.property.name='doesNotThrow']" message: "Please replace `assert.doesNotThrow()` and add a comment next to the code instead." - selector: "CallExpression[callee.object.name='assert'][callee.property.name='rejects'][arguments.length<2]" message: "assert.rejects() must be invoked with at least two arguments." + - selector: "CallExpression[callee.object.name='assert'][callee.property.name='strictEqual'][arguments.2.type='Literal']" + message: "Do not use a literal for the third argument of assert.strictEqual()" - selector: "CallExpression[callee.object.name='assert'][callee.property.name='throws'][arguments.1.type='Literal']:not([arguments.1.regex])" message: "Use an object as second argument of assert.throws()" - selector: "CallExpression[callee.object.name='assert'][callee.property.name='throws'][arguments.length<2]" diff --git a/test/.eslintrc.yaml b/test/.eslintrc.yaml index 25026fec5a103a..63d2127d738597 100644 --- a/test/.eslintrc.yaml +++ b/test/.eslintrc.yaml @@ -23,10 +23,14 @@ rules: no-restricted-syntax: # Config copied from .eslintrc.js - error + - selector: "CallExpression[callee.object.name='assert'][callee.property.name='deepStrictEqual'][arguments.2.type='Literal']" + message: "Do not use a literal for the third argument of assert.deepStrictEqual()" - selector: "CallExpression[callee.object.name='assert'][callee.property.name='doesNotThrow']" message: "Please replace `assert.doesNotThrow()` and add a comment next to the code instead." - selector: "CallExpression[callee.object.name='assert'][callee.property.name='rejects'][arguments.length<2]" message: "assert.rejects() must be invoked with at least two arguments." + - selector: "CallExpression[callee.object.name='assert'][callee.property.name='strictEqual'][arguments.2.type='Literal']" + message: "Do not use a literal for the third argument of assert.strictEqual()" - selector: "CallExpression[callee.object.name='assert'][callee.property.name='throws'][arguments.1.type='Literal']:not([arguments.1.regex])" message: "Use an object as second argument of assert.throws()" - selector: "CallExpression[callee.object.name='assert'][callee.property.name='throws'][arguments.length<2]"