Skip to content

Commit

Permalink
test: minor refactoring
Browse files Browse the repository at this point in the history
Add punctuation and comments about code that should not throw.
Also remove a obsolete test and refactor some tests.

PR-URL: nodejs#18669
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
  • Loading branch information
BridgeAR committed Mar 21, 2018
1 parent 9a063f9 commit 9e9f331
Show file tree
Hide file tree
Showing 49 changed files with 163 additions and 204 deletions.
2 changes: 1 addition & 1 deletion test/addons/symlinked-module/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
});
5 changes: 1 addition & 4 deletions test/internet/test-dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -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());
4 changes: 2 additions & 2 deletions test/parallel/test-assert-deep.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions test/parallel/test-buffer-alloc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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 });
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-buffer-bad-overload.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -14,4 +14,4 @@ assert.throws(function() {
Buffer.from(10, 'hex');
}, err);

Buffer.from('deadbeaf', 'hex');
Buffer.from('deadbeaf', 'hex'); // Should not throw.
2 changes: 1 addition & 1 deletion test/parallel/test-buffer-constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-buffer-copy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions test/parallel/test-buffer-sharedarraybuffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
5 changes: 2 additions & 3 deletions test/parallel/test-buffer-slice.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

{
Expand Down
19 changes: 9 additions & 10 deletions test/parallel/test-child-process-spawn-typeerror.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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() {}
Expand All @@ -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);

Expand All @@ -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:
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions test/parallel/test-console-async-write-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
}
16 changes: 8 additions & 8 deletions test/parallel/test-console-instance.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(); },
{
Expand All @@ -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 = () => {};
Expand Down Expand Up @@ -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++;
Expand All @@ -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.
Expand Down
9 changes: 3 additions & 6 deletions test/parallel/test-console-sync-write-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
}

{
Expand All @@ -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.
}

{
Expand All @@ -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.
}
}
36 changes: 9 additions & 27 deletions test/parallel/test-crypto-padding.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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 \
Expand All @@ -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);
Expand All @@ -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
);
4 changes: 1 addition & 3 deletions test/parallel/test-crypto-pbkdf2.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down
1 change: 0 additions & 1 deletion test/parallel/test-crypto-rsa-dsa.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,4 @@ common.expectsError(
);

process.setUncaughtExceptionCaptureCallback(null);

require('domain');
require('domain'); // Should not throw.
2 changes: 1 addition & 1 deletion test/parallel/test-fs-access.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Loading

0 comments on commit 9e9f331

Please sign in to comment.