From 75d9dedc4514e7a988a1fc5ac88bcc243b8faaf0 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 1 Sep 2018 22:47:37 -0700 Subject: [PATCH 1/6] 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. PR-URL: https://github.com/nodejs/node/pull/22849 Reviewed-By: Teddy Katz Reviewed-By: Matteo Collina Reviewed-By: Anna Henningsen Reviewed-By: Trivikram Kamat Reviewed-By: Luigi Pinca Reviewed-By: Colin Ihrig --- 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 50fcff2d425655..e7fec6f256399c 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 assertNoUrlsWhileConnected(response) { From c729488ca9fa4ee672d8c7cef3911de5ee89c287 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sun, 2 Sep 2018 10:55:33 -0700 Subject: [PATCH 2/6] 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. PR-URL: https://github.com/nodejs/node/pull/22849 Reviewed-By: Teddy Katz Reviewed-By: Matteo Collina Reviewed-By: Anna Henningsen Reviewed-By: Trivikram Kamat Reviewed-By: Luigi Pinca Reviewed-By: Colin Ihrig --- 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 1bd44dd25a5c9c3a3f95abdaf238d382ca3984b1 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 12 Sep 2018 20:16:23 -0700 Subject: [PATCH 3/6] 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()`. PR-URL: https://github.com/nodejs/node/pull/22849 Reviewed-By: Teddy Katz Reviewed-By: Matteo Collina Reviewed-By: Anna Henningsen Reviewed-By: Trivikram Kamat Reviewed-By: Luigi Pinca Reviewed-By: Colin Ihrig --- 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 1edb061ea6a871..e506905c554c2c 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'); common.globalCheck = false; From f6f6cd577ac84dc594e53cf58ac78ff921aecdd7 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 12 Sep 2018 20:24:28 -0700 Subject: [PATCH 4/6] test: remove string literal from assertion Remove string literal from `assert.strictEqual()` call `message` parameter and make it a comment above the assertion instead. PR-URL: https://github.com/nodejs/node/pull/22849 Reviewed-By: Teddy Katz Reviewed-By: Matteo Collina Reviewed-By: Anna Henningsen Reviewed-By: Trivikram Kamat Reviewed-By: Luigi Pinca Reviewed-By: Colin Ihrig --- 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 1cd2d5d21acc7200a384563b18bd52c0c0f0d433 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Thu, 13 Sep 2018 13:04:09 -0700 Subject: [PATCH 5/6] test: remove string literal from assertion Remove string literal as assertion message in call to assert.strictEqual() in test-dns-resolveany-bad-ancount. PR-URL: https://github.com/nodejs/node/pull/22849 Reviewed-By: Teddy Katz Reviewed-By: Matteo Collina Reviewed-By: Anna Henningsen Reviewed-By: Trivikram Kamat Reviewed-By: Luigi Pinca Reviewed-By: Colin Ihrig --- 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 63ed1774b11933..82378b31bc3552 100644 --- a/test/parallel/test-dns-resolveany-bad-ancount.js +++ b/test/parallel/test-dns-resolveany-bad-ancount.js @@ -31,8 +31,8 @@ server.bind(0, common.mustCall(() => { 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 be38652731d5ef8a0d2c8e1f288082a0b864e142 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Thu, 13 Sep 2018 13:47:57 -0700 Subject: [PATCH 6/6] 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(). PR-URL: https://github.com/nodejs/node/pull/22849 Reviewed-By: Teddy Katz Reviewed-By: Matteo Collina Reviewed-By: Anna Henningsen Reviewed-By: Trivikram Kamat Reviewed-By: Luigi Pinca Reviewed-By: Colin Ihrig --- 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 cdb824c3cffa8f..c73b49deb328cb 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -610,11 +610,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.split('\n')[0], 'oh no'); - assert.strictEqual(e.generatedMessage, false, - 'Message incorrectly marked as generated'); + // Message should not be marked as generated. + assert.strictEqual(e.generatedMessage, false); } {