test: minor refactoring

Add punctuation and comments about code that should not throw.
Also remove a obsolete test and refactor some tests.

Backport-PR-URL: #19244
PR-URL: #18669
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

test/addons/symlinked-module/test.js

@@ -30,5 +30,5 @@ const sub = require('./submodule');
   const mod = require(path.join(i, 'binding.node'));
   assert.notStrictEqual(mod, null);
   assert.strictEqual(mod.hello(), 'world');
-  sub.test(i);
+  sub.test(i); // Should not throw.
 });

test/internet/test-dns.js

@@ -579,11 +579,8 @@ process.on('exit', function() {
   assert.ok(getaddrinfoCallbackCalled);
 });
 
-
+// Should not throw.
 dns.lookup(addresses.INET6_HOST, 6, common.mustCall());
-
 dns.lookup(addresses.INET_HOST, {}, common.mustCall());
-
 dns.lookupService('0.0.0.0', '0', common.mustCall());
-
 dns.lookupService('0.0.0.0', 0, common.mustCall());

test/parallel/test-assert-deep.js

@@ -492,8 +492,8 @@ assertOnlyDeepEqual([1, , , 3], [1, , , 3, , , ]);
 
 // Handle NaN
 assert.throws(() => { assert.deepEqual(NaN, NaN); }, assert.AssertionError);
-{ assert.deepStrictEqual(NaN, NaN); }
-{ assert.deepStrictEqual({ a: NaN }, { a: NaN }); }
+assert.deepStrictEqual(NaN, NaN);
+assert.deepStrictEqual({ a: NaN }, { a: NaN });
 assert.deepStrictEqual([ 1, 2, NaN, 4 ], [ 1, 2, NaN, 4 ]);
 
 // Handle boxed primitives

test/parallel/test-buffer-alloc.js

@@ -62,7 +62,7 @@ assert.throws(() => b.write('test', 'utf8', 0),
               /is no longer supported/);
 
 
-// try to create 0-length buffers
+// Try to create 0-length buffers. Should not throw.
 Buffer.from('');
 Buffer.from('', 'ascii');
 Buffer.from('', 'latin1');
@@ -107,7 +107,7 @@ b.copy(Buffer.alloc(1), 0, 2048, 2048);
   assert.strictEqual(writeTest.toString(), 'nodejs');
 }
 
-// Offset points to the end of the buffer
+// Offset points to the end of the buffer and does not throw.
 // (see https://github.com/nodejs/node/issues/8127).
 Buffer.alloc(1).write('', 1, 0);
 
@@ -992,10 +992,10 @@ common.expectsError(() => {
   assert.strictEqual(ubuf.buffer.byteLength, 10);
 }
 
-// Regression test
+// Regression test to verify that an empty ArrayBuffer does not throw.
 Buffer.from(new ArrayBuffer());
 
-// Test that ArrayBuffer from a different context is detected correctly
+// Test that ArrayBuffer from a different context is detected correctly.
 const arrayBuf = vm.runInNewContext('new ArrayBuffer()');
 Buffer.from(arrayBuf);
 Buffer.from({ buffer: arrayBuf });

test/parallel/test-buffer-bad-overload.js

@@ -2,7 +2,7 @@
 const common = require('../common');
 const assert = require('assert');
 
-Buffer.allocUnsafe(10);
+Buffer.allocUnsafe(10); // Should not throw.
 
 const err = common.expectsError({
   code: 'ERR_INVALID_ARG_TYPE',
@@ -14,4 +14,4 @@ assert.throws(function() {
   Buffer.from(10, 'hex');
 }, err);
 
-Buffer.from('deadbeaf', 'hex');
+Buffer.from('deadbeaf', 'hex'); // Should not throw.

test/parallel/test-buffer-constants.js

@@ -11,7 +11,7 @@ assert(MAX_STRING_LENGTH <= MAX_LENGTH);
 assert.throws(() => ' '.repeat(MAX_STRING_LENGTH + 1),
               /^RangeError: Invalid string length$/);
 
-' '.repeat(MAX_STRING_LENGTH);
+' '.repeat(MAX_STRING_LENGTH); // Should not throw.
 
 // Legacy values match:
 assert.strictEqual(kMaxLength, MAX_LENGTH);

test/parallel/test-buffer-copy.js

@@ -92,7 +92,7 @@ const bb = Buffer.allocUnsafe(10);
 bb.fill('hello crazy world');
 
 
-// try to copy from before the beginning of b
+// Try to copy from before the beginning of b. Should not throw.
 b.copy(c, 0, 100, 10);
 
 // copy throws at negative sourceStart

test/parallel/test-buffer-sharedarraybuffer.js

@@ -22,8 +22,7 @@ arr2[1] = 6000;
 
 assert.deepStrictEqual(arr_buf, ar_buf);
 
-// Checks for calling Buffer.byteLength on a SharedArrayBuffer
-
+// Checks for calling Buffer.byteLength on a SharedArrayBuffer.
 assert.strictEqual(Buffer.byteLength(sab), sab.byteLength);
 
-Buffer.from({ buffer: sab });
+Buffer.from({ buffer: sab }); // Should not throw.

test/parallel/test-buffer-slice.js

@@ -77,9 +77,8 @@ expectedSameBufs.forEach(([buf1, buf2]) => {
 
 const utf16Buf = Buffer.from('0123456789', 'utf16le');
 assert.deepStrictEqual(utf16Buf.slice(0, 6), Buffer.from('012', 'utf16le'));
-// try to slice a zero length Buffer
-// see https://github.com/joyent/node/issues/5881
-Buffer.alloc(0).slice(0, 1);
+// Try to slice a zero length Buffer.
+// See https://github.com/joyent/node/issues/5881
 assert.strictEqual(Buffer.alloc(0).slice(0, 1).length, 0);
 
 {

test/parallel/test-child-process-spawn-typeerror.js

@@ -38,13 +38,13 @@ assert.throws(function() {
   child.on('error', common.mustNotCall());
 }, TypeError);
 
-// verify that valid argument combinations do not throw
+// Verify that valid argument combinations do not throw.
 spawn(cmd);
 spawn(cmd, []);
 spawn(cmd, {});
 spawn(cmd, [], {});
 
-// verify that invalid argument combinations throw
+// Verify that invalid argument combinations throw.
 assert.throws(function() {
   spawn();
 }, invalidFileMsg);
@@ -74,7 +74,7 @@ assert.throws(function() {
   spawn(cmd, [], 1);
 }, invalidOptionsMsg);
 
-// Argument types for combinatorics
+// Argument types for combinatorics.
 const a = [];
 const o = {};
 function c() {}
@@ -92,7 +92,7 @@ spawn(cmd, a);
 spawn(cmd, a, o);
 spawn(cmd, o);
 
-// Variants of undefined as explicit 'no argument' at a position
+// Variants of undefined as explicit 'no argument' at a position.
 spawn(cmd, u, o);
 spawn(cmd, a, u);
 
@@ -103,7 +103,7 @@ assert.throws(function() { spawn(cmd, s); }, TypeError);
 assert.throws(function() { spawn(cmd, a, s); }, TypeError);
 
 
-// verify that execFile has same argument parsing behavior as spawn
+// Verify that execFile has same argument parsing behavior as spawn.
 //
 // function execFile(file=f [,args=a] [, options=o] [, callback=c]) has valid
 // combinations:
@@ -124,7 +124,7 @@ execFile(cmd, o);
 execFile(cmd, o, c);
 execFile(cmd, c);
 
-// Variants of undefined as explicit 'no argument' at a position
+// Variants of undefined as explicit 'no argument' at a position.
 execFile(cmd, u, o, c);
 execFile(cmd, a, u, c);
 execFile(cmd, a, o, u);
@@ -146,7 +146,7 @@ execFile(cmd, o, n);
 execFile(cmd, c, u);
 execFile(cmd, c, n);
 
-// string is invalid in arg position (this may seem strange, but is
+// String is invalid in arg position (this may seem strange, but is
 // consistent across node API, cf. `net.createServer('not options', 'not
 // callback')`
 assert.throws(function() { execFile(cmd, s, o, c); }, TypeError);
@@ -160,10 +160,9 @@ assert.throws(function() { execFile(cmd, a, u, s); }, TypeError);
 assert.throws(function() { execFile(cmd, a, n, s); }, TypeError);
 assert.throws(function() { execFile(cmd, u, o, s); }, TypeError);
 assert.throws(function() { execFile(cmd, n, o, s); }, TypeError);
-execFile(cmd, c, s);
+execFile(cmd, c, s); // Should not throw.
 
-
-// verify that fork has same argument parsing behavior as spawn
+// Verify that fork has same argument parsing behavior as spawn.
 //
 // function fork(file=f [,args=a] [, options=o]) has valid combinations:
 //   (f)

test/parallel/test-console-async-write-error.js

@@ -11,6 +11,5 @@ for (const method of ['dir', 'log', 'warn']) {
   });
 
   const c = new Console(out, out, true);
-
-  c[method]('abc');
+  c[method]('abc'); // Should not throw.
 }

test/parallel/test-console-instance.js

@@ -28,15 +28,15 @@ const Console = require('console').Console;
 const out = new Stream();
 const err = new Stream();
 
-// ensure the Console instance doesn't write to the
-// process' "stdout" or "stderr" streams
+// Ensure the Console instance doesn't write to the
+// process' "stdout" or "stderr" streams.
 process.stdout.write = process.stderr.write = common.mustNotCall();
 
-// make sure that the "Console" function exists
+// Make sure that the "Console" function exists.
 assert.strictEqual('function', typeof Console);
 
-// make sure that the Console constructor throws
-// when not given a writable stream instance
+// Make sure that the Console constructor throws
+// when not given a writable stream instance.
 common.expectsError(
   () => { new Console(); },
   {
@@ -46,7 +46,7 @@ common.expectsError(
   }
 );
 
-// Console constructor should throw if stderr exists but is not writable
+// Console constructor should throw if stderr exists but is not writable.
 common.expectsError(
   () => {
     out.write = () => {};
@@ -77,7 +77,7 @@ out.write = common.mustCall((d) => {
 
 c.dir({ foo: 1 });
 
-// ensure that the console functions are bound to the console instance
+// Ensure that the console functions are bound to the console instance.
 let called = 0;
 out.write = common.mustCall((d) => {
   called++;
@@ -86,7 +86,7 @@ out.write = common.mustCall((d) => {
 
 [1, 2, 3].forEach(c.log);
 
-// Console() detects if it is called without `new` keyword
+// Console() detects if it is called without `new` keyword.
 Console(out, err);
 
 // Instance that does not ignore the stream errors.

test/parallel/test-console-sync-write-error.js

@@ -12,8 +12,7 @@ for (const method of ['dir', 'log', 'warn']) {
     });
 
     const c = new Console(out, out, true);
-
-    c[method]('abc');
+    c[method]('abc'); // Should not throw.
   }
 
   {
@@ -24,8 +23,7 @@ for (const method of ['dir', 'log', 'warn']) {
     });
 
     const c = new Console(out, out, true);
-
-    c[method]('abc');
+    c[method]('abc'); // Should not throw.
   }
 
   {
@@ -36,7 +34,6 @@ for (const method of ['dir', 'log', 'warn']) {
     });
 
     const c = new Console(out, out, true);
-
-    c[method]('abc');
+    c[method]('abc'); // Should not throw.
   }
 }

test/parallel/test-crypto-padding.js

@@ -29,11 +29,7 @@ const crypto = require('crypto');
 
 crypto.DEFAULT_ENCODING = 'buffer';
 
-
-/*
- * Input data
- */
-
+// Input data.
 const ODD_LENGTH_PLAIN = 'Hello node world!';
 const EVEN_LENGTH_PLAIN = 'Hello node world!AbC09876dDeFgHi';
 
@@ -42,10 +38,7 @@ const IV_PLAIN = 'blahFizz2011Buzz';
 
 const CIPHER_NAME = 'aes-128-cbc';
 
-
-/*
- * Expected result data
- */
+// Expected result data.
 
 // echo -n 'Hello node world!' | \
 // openssl enc -aes-128-cbc -e -K 5333632e722e652e742e4b2e652e5921 \
@@ -67,10 +60,7 @@ const EVEN_LENGTH_ENCRYPTED_NOPAD =
     '7f57859550d4d2fdb9806da2a750461ab46e71b3d78ebe2d9684dfc87f7575b9';
 
 
-/*
- * Helper wrappers
- */
-
+// Helper wrappers.
 function enc(plain, pad) {
   const encrypt = crypto.createCipheriv(CIPHER_NAME, KEY_PLAIN, IV_PLAIN);
   encrypt.setAutoPadding(pad);
@@ -87,41 +77,33 @@ function dec(encd, pad) {
   return plain;
 }
 
-
-/*
- * Test encryption
- */
-
+// Test encryption
 assert.strictEqual(enc(ODD_LENGTH_PLAIN, true), ODD_LENGTH_ENCRYPTED);
 assert.strictEqual(enc(EVEN_LENGTH_PLAIN, true), EVEN_LENGTH_ENCRYPTED);
 
 assert.throws(function() {
-  // input must have block length %
+  // Input must have block length %.
   enc(ODD_LENGTH_PLAIN, false);
 }, /data not multiple of block length/);
 
 assert.strictEqual(
   enc(EVEN_LENGTH_PLAIN, false), EVEN_LENGTH_ENCRYPTED_NOPAD
 );
 
-
-/*
- * Test decryption
- */
-
+// Test decryption.
 assert.strictEqual(dec(ODD_LENGTH_ENCRYPTED, true), ODD_LENGTH_PLAIN);
 assert.strictEqual(dec(EVEN_LENGTH_ENCRYPTED, true), EVEN_LENGTH_PLAIN);
 
-// returns including original padding
+// Returns including original padding.
 assert.strictEqual(dec(ODD_LENGTH_ENCRYPTED, false).length, 32);
 assert.strictEqual(dec(EVEN_LENGTH_ENCRYPTED, false).length, 48);
 
 assert.throws(function() {
-  // must have at least 1 byte of padding (PKCS):
+  // Must have at least 1 byte of padding (PKCS):
   assert.strictEqual(dec(EVEN_LENGTH_ENCRYPTED_NOPAD, true), EVEN_LENGTH_PLAIN);
 }, /bad decrypt/);
 
-// no-pad encrypted string should return the same:
+// No-pad encrypted string should return the same:
 assert.strictEqual(
   dec(EVEN_LENGTH_ENCRYPTED_NOPAD, false), EVEN_LENGTH_PLAIN
 );

test/parallel/test-crypto-pbkdf2.js

@@ -99,9 +99,7 @@ common.expectsError(
 
 // Should not get FATAL ERROR with empty password and salt
 // https://github.com/nodejs/node/issues/8571
-crypto.pbkdf2('', '', 1, 32, 'sha256', common.mustCall((e) => {
-  assert.ifError(e);
-}));
+crypto.pbkdf2('', '', 1, 32, 'sha256', common.mustCall(assert.ifError));
 
 common.expectsError(
   () => crypto.pbkdf2('password', 'salt', 8, 8, common.mustNotCall()),

test/parallel/test-crypto-rsa-dsa.js

@@ -256,7 +256,6 @@ const input = 'I AM THE WALRUS';
   // against
   const sign = crypto.createSign('SHA1');
   sign.update(input);
-
   const signOptions = { key: dsaKeyPemEncrypted, passphrase: 'password' };
   const signature = sign.sign(signOptions, 'hex');
 

test/parallel/test-domain-load-after-set-uncaught-exception-capture.js

@@ -13,5 +13,4 @@ common.expectsError(
 );
 
 process.setUncaughtExceptionCaptureCallback(null);
-
-require('domain');
+require('domain'); // Should not throw.

test/parallel/test-fs-access.js

@@ -105,8 +105,8 @@ common.expectsError(
     type: TypeError
   });
 
+// Regular access should not throw.
 fs.accessSync(__filename);
-
 const mode = fs.F_OK | fs.R_OK | fs.W_OK;
 fs.accessSync(readWriteFile, mode);