Skip to content

Commit

Permalink
test: fix RegExp nits
Browse files Browse the repository at this point in the history
* Remove needless RegExp flag

  In fixed case, `/g` flag is needless in the boolean context.

* Remove needless RegExp capturing

  Use non-capturing grouping or remove capturing completely when:

  * capturing is useless per se, e.g. in test() check;
  * captured groups are not used afterward at all;
  * some of the later captured groups are not used afterward.

* Use test, not match/exec in boolean context

  match() and exec() return a complicated object,
  unneeded in a boolean context.

* Do not needlessly repeat RegExp creation

  This commit takes RegExp creation out of cycles and other repetitions.

  As long as the RegExp does not use /g flag and match indices,
  we are safe here.

  In tests, this fix hardly gives a significant performance gain,
  but it increases clarity and maintainability,
  reassuring some RegExps to be identical.

  RegExp in functions are not taken out of their functions:
  while these functions are called many times
  and their RegExps are recreated with each call,
  the performance gain in test cases
  does not seem to be worth decreasing function self-dependency.

PR-URL: #13770
Reviewed-By: Colin Ihrig <[email protected]>
  • Loading branch information
vsemozhetbyt authored and addaleax committed Jun 21, 2017
1 parent 1fe455f commit 1f32d9e
Show file tree
Hide file tree
Showing 56 changed files with 301 additions and 244 deletions.
5 changes: 3 additions & 2 deletions test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -267,8 +267,9 @@ if (exports.isWindows) {
}

const ifaces = os.networkInterfaces();
const re = /lo/;
exports.hasIPv6 = Object.keys(ifaces).some(function(name) {
return /lo/.test(name) && ifaces[name].some(function(info) {
return re.test(name) && ifaces[name].some(function(info) {
return info.family === 'IPv6';
});
});
Expand Down Expand Up @@ -433,7 +434,7 @@ function leakedGlobals() {
leaked.push(val);

if (global.__coverage__) {
return leaked.filter((varname) => !/^(cov_|__cov)/.test(varname));
return leaked.filter((varname) => !/^(?:cov_|__cov)/.test(varname));
} else {
return leaked;
}
Expand Down
4 changes: 2 additions & 2 deletions test/debugger/helper-debugger-repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ function startDebugger(scriptToDebug) {
child.stderr.pipe(process.stderr);

child.on('line', function(line) {
line = line.replace(/^(debug> *)+/, '');
line = line.replace(/^(?:debug> *)+/, '');
console.log(line);
assert.ok(expected.length > 0, `Got unexpected line: ${line}`);

const expectedLine = expected[0].lines.shift();
assert.ok(line.match(expectedLine) !== null, `${line} != ${expectedLine}`);
assert.ok(expectedLine.test(line), `${line} != ${expectedLine}`);

if (expected[0].lines.length === 0) {
const callback = expected[0].callback;
Expand Down
6 changes: 4 additions & 2 deletions test/doctool/test-doctool-html.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,11 @@ const testData = [
},
];

const spaces = /\s/g;

testData.forEach((item) => {
// Normalize expected data by stripping whitespace
const expected = item.html.replace(/\s/g, '');
const expected = item.html.replace(spaces, '');
const includeAnalytics = typeof item.analyticsId !== 'undefined';

fs.readFile(item.file, 'utf8', common.mustCall((err, input) => {
Expand All @@ -112,7 +114,7 @@ testData.forEach((item) => {
common.mustCall((err, output) => {
assert.ifError(err);

const actual = output.replace(/\s/g, '');
const actual = output.replace(spaces, '');
// Assert that the input stripped of all whitespace contains the
// expected list
assert.notStrictEqual(actual.indexOf(expected), -1);
Expand Down
4 changes: 2 additions & 2 deletions test/inspector/inspector-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -521,11 +521,11 @@ exports.startNodeForInspectorTest = function(callback,
clearTimeout(timeoutId);
console.log('[err]', text);
if (found) return;
const match = text.match(/Debugger listening on ws:\/\/(.+):(\d+)\/(.+)/);
const match = text.match(/Debugger listening on ws:\/\/.+:(\d+)\/.+/);
found = true;
child.stderr.removeListener('data', dataCallback);
assert.ok(match, text);
callback(new Harness(match[2], child));
callback(new Harness(match[1], child));
});

child.stderr.on('data', dataCallback);
Expand Down
4 changes: 2 additions & 2 deletions test/inspector/test-inspector.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ function checkListResponse(err, response) {
assert.strictEqual(1, response.length);
assert.ok(response[0]['devtoolsFrontendUrl']);
assert.ok(
response[0]['webSocketDebuggerUrl']
.match(/ws:\/\/127\.0\.0\.1:\d+\/[0-9A-Fa-f]{8}-/));
/ws:\/\/127\.0\.0\.1:\d+\/[0-9A-Fa-f]{8}-/
.test(response[0]['webSocketDebuggerUrl']));
}

function checkVersion(err, response) {
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-assert-checktag.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ const util = require('util');
// for assert.throws()
function re(literals, ...values) {
let result = literals[0];
const escapeRE = /[\\^$.*+?()[\]{}|=!<>:-]/g;
for (const [i, value] of values.entries()) {
const str = util.inspect(value);
// Need to escape special characters.
result += str.replace(/[\\^$.*+?()[\]{}|=!<>:-]/g, '\\$&');
result += str.replace(escapeRE, '\\$&');
result += literals[i + 1];
}
return common.expectsError({
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-assert-deep.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ const util = require('util');
// for assert.throws()
function re(literals, ...values) {
let result = literals[0];
const escapeRE = /[\\^$.*+?()[\]{}|=!<>:-]/g;
for (const [i, value] of values.entries()) {
const str = util.inspect(value);
// Need to escape special characters.
result += str.replace(/[\\^$.*+?()[\]{}|=!<>:-]/g, '\\$&');
result += str.replace(escapeRE, '\\$&');
result += literals[i + 1];
}
return common.expectsError({
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -722,13 +722,14 @@ assert.throws(() => {
{
// bad args to AssertionError constructor should throw TypeError
const args = [1, true, false, '', null, Infinity, Symbol('test'), undefined];
const re = /^The "options" argument must be of type object$/;
args.forEach((input) => {
assert.throws(
() => new assert.AssertionError(input),
common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: /^The "options" argument must be of type object$/
message: re
}));
});
}
13 changes: 5 additions & 8 deletions test/parallel/test-buffer-bytelength.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,11 @@ const SlowBuffer = require('buffer').SlowBuffer;
const vm = require('vm');

// coerce values to string
assert.throws(() => { Buffer.byteLength(32, 'latin1'); },
/"string" must be a string, Buffer, or ArrayBuffer/);
assert.throws(() => { Buffer.byteLength(NaN, 'utf8'); },
/"string" must be a string, Buffer, or ArrayBuffer/);
assert.throws(() => { Buffer.byteLength({}, 'latin1'); },
/"string" must be a string, Buffer, or ArrayBuffer/);
assert.throws(() => { Buffer.byteLength(); },
/"string" must be a string, Buffer, or ArrayBuffer/);
const re = /"string" must be a string, Buffer, or ArrayBuffer/;
assert.throws(() => { Buffer.byteLength(32, 'latin1'); }, re);
assert.throws(() => { Buffer.byteLength(NaN, 'utf8'); }, re);
assert.throws(() => { Buffer.byteLength({}, 'latin1'); }, re);
assert.throws(() => { Buffer.byteLength(); }, re);

assert.strictEqual(Buffer.byteLength('', undefined, true), -1);

Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-buffer-prototype-inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,5 @@ const util = require('util');

{
const buf = Buffer.from('x'.repeat(51));
assert.ok(/^<Buffer (78 ){50}\.\.\. >$/.test(util.inspect(buf)));
assert.ok(/^<Buffer (?:78 ){50}\.\.\. >$/.test(util.inspect(buf)));
}
12 changes: 8 additions & 4 deletions test/parallel/test-child-process-constructor.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,44 +8,48 @@ assert.strictEqual(typeof ChildProcess, 'function');
{
// Verify that invalid options to spawn() throw.
const child = new ChildProcess();
const re = /^TypeError: "options" must be an object$/;

[undefined, null, 'foo', 0, 1, NaN, true, false].forEach((options) => {
assert.throws(() => {
child.spawn(options);
}, /^TypeError: "options" must be an object$/);
}, re);
});
}

{
// Verify that spawn throws if file is not a string.
const child = new ChildProcess();
const re = /^TypeError: "file" must be a string$/;

[undefined, null, 0, 1, NaN, true, false, {}].forEach((file) => {
assert.throws(() => {
child.spawn({ file });
}, /^TypeError: "file" must be a string$/);
}, re);
});
}

{
// Verify that spawn throws if envPairs is not an array or undefined.
const child = new ChildProcess();
const re = /^TypeError: "envPairs" must be an array$/;

[null, 0, 1, NaN, true, false, {}, 'foo'].forEach((envPairs) => {
assert.throws(() => {
child.spawn({ envPairs, stdio: ['ignore', 'ignore', 'ignore', 'ipc'] });
}, /^TypeError: "envPairs" must be an array$/);
}, re);
});
}

{
// Verify that spawn throws if args is not an array or undefined.
const child = new ChildProcess();
const re = /^TypeError: "args" must be an array$/;

[null, 0, 1, NaN, true, false, {}, 'foo'].forEach((args) => {
assert.throws(() => {
child.spawn({ file: 'foo', args });
}, /^TypeError: "args" must be an array$/);
}, re);
});
}

Expand Down
12 changes: 6 additions & 6 deletions test/parallel/test-cli-syntax.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ const syntaxArgs = [
['--check']
];

const syntaxErrorRE = /^SyntaxError: Unexpected identifier$/m;
const notFoundRE = /^Error: Cannot find module/m;

// test good syntax with and without shebang
[
'syntax/good_syntax.js',
Expand Down Expand Up @@ -56,8 +59,7 @@ const syntaxArgs = [
assert(c.stderr.startsWith(file), "stderr doesn't start with the filename");

// stderr should have a syntax error message
const match = c.stderr.match(/^SyntaxError: Unexpected identifier$/m);
assert(match, 'stderr incorrect');
assert(syntaxErrorRE.test(c.stderr), 'stderr incorrect');

assert.strictEqual(c.status, 1, `code === ${c.status}`);
});
Expand All @@ -79,8 +81,7 @@ const syntaxArgs = [
assert.strictEqual(c.stdout, '', 'stdout produced');

// stderr should have a module not found error message
const match = c.stderr.match(/^Error: Cannot find module/m);
assert(match, 'stderr incorrect');
assert(notFoundRE.test(c.stderr), 'stderr incorrect');

assert.strictEqual(c.status, 1, `code === ${c.status}`);
});
Expand Down Expand Up @@ -112,8 +113,7 @@ syntaxArgs.forEach(function(args) {
assert.strictEqual(c.stdout, '', 'stdout produced');

// stderr should have a syntax error message
const match = c.stderr.match(/^SyntaxError: Unexpected identifier$/m);
assert(match, 'stderr incorrect');
assert(syntaxErrorRE.test(c.stderr), 'stderr incorrect');

assert.strictEqual(c.status, 1, `code === ${c.status}`);
});
Expand Down
28 changes: 18 additions & 10 deletions test/parallel/test-crypto-authenticated.js
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,13 @@ const TEST_CASES = [
tag: 'a44a8266ee1c8eb0c8b5d4cf5ae9f19a', tampered: false },
];

const errMessages = {
auth: / auth/,
state: / state/,
FIPS: /not supported in FIPS mode/,
length: /Invalid IV length/,
};

const ciphers = crypto.getCiphers();

for (const i in TEST_CASES) {
Expand Down Expand Up @@ -378,14 +385,14 @@ for (const i in TEST_CASES) {
assert.strictEqual(msg, test.plain);
} else {
// assert that final throws if input data could not be verified!
assert.throws(function() { decrypt.final('ascii'); }, / auth/);
assert.throws(function() { decrypt.final('ascii'); }, errMessages.auth);
}
}

if (test.password) {
if (common.hasFipsCrypto) {
assert.throws(() => { crypto.createCipher(test.algo, test.password); },
/not supported in FIPS mode/);
errMessages.FIPS);
} else {
const encrypt = crypto.createCipher(test.algo, test.password);
if (test.aad)
Expand All @@ -404,7 +411,7 @@ for (const i in TEST_CASES) {
if (test.password) {
if (common.hasFipsCrypto) {
assert.throws(() => { crypto.createDecipher(test.algo, test.password); },
/not supported in FIPS mode/);
errMessages.FIPS);
} else {
const decrypt = crypto.createDecipher(test.algo, test.password);
decrypt.setAuthTag(Buffer.from(test.tag, 'hex'));
Expand All @@ -416,7 +423,7 @@ for (const i in TEST_CASES) {
assert.strictEqual(msg, test.plain);
} else {
// assert that final throws if input data could not be verified!
assert.throws(function() { decrypt.final('ascii'); }, / auth/);
assert.throws(function() { decrypt.final('ascii'); }, errMessages.auth);
}
}
}
Expand All @@ -427,7 +434,7 @@ for (const i in TEST_CASES) {
Buffer.from(test.key, 'hex'),
Buffer.from(test.iv, 'hex'));
encrypt.update('blah', 'ascii');
assert.throws(function() { encrypt.getAuthTag(); }, / state/);
assert.throws(function() { encrypt.getAuthTag(); }, errMessages.state);
}

{
Expand All @@ -436,15 +443,15 @@ for (const i in TEST_CASES) {
Buffer.from(test.key, 'hex'),
Buffer.from(test.iv, 'hex'));
assert.throws(() => { encrypt.setAuthTag(Buffer.from(test.tag, 'hex')); },
/ state/);
errMessages.state);
}

{
// trying to read tag from decryption object:
const decrypt = crypto.createDecipheriv(test.algo,
Buffer.from(test.key, 'hex'),
Buffer.from(test.iv, 'hex'));
assert.throws(function() { decrypt.getAuthTag(); }, / state/);
assert.throws(function() { decrypt.getAuthTag(); }, errMessages.state);
}

{
Expand All @@ -455,7 +462,7 @@ for (const i in TEST_CASES) {
Buffer.from(test.key, 'hex'),
Buffer.alloc(0)
);
}, /Invalid IV length/);
}, errMessages.length);
}
}

Expand All @@ -467,6 +474,7 @@ for (const i in TEST_CASES) {
'6fKjEjR3Vl30EUYC');
encrypt.update('blah', 'ascii');
encrypt.final();
assert.throws(() => encrypt.getAuthTag(), / state/);
assert.throws(() => encrypt.setAAD(Buffer.from('123', 'ascii')), / state/);
assert.throws(() => encrypt.getAuthTag(), errMessages.state);
assert.throws(() => encrypt.setAAD(Buffer.from('123', 'ascii')),
errMessages.state);
}
8 changes: 5 additions & 3 deletions test/parallel/test-crypto-cipheriv-decipheriv.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,14 @@ testCipher2(Buffer.from('0123456789abcd0123456789'), Buffer.from('12345678'));
// Zero-sized IV should be accepted in ECB mode.
crypto.createCipheriv('aes-128-ecb', Buffer.alloc(16), Buffer.alloc(0));

const errMessage = /Invalid IV length/;

// But non-empty IVs should be rejected.
for (let n = 1; n < 256; n += 1) {
assert.throws(
() => crypto.createCipheriv('aes-128-ecb', Buffer.alloc(16),
Buffer.alloc(n)),
/Invalid IV length/);
errMessage);
}

// Correctly sized IV should be accepted in CBC mode.
Expand All @@ -83,14 +85,14 @@ for (let n = 0; n < 256; n += 1) {
assert.throws(
() => crypto.createCipheriv('aes-128-cbc', Buffer.alloc(16),
Buffer.alloc(n)),
/Invalid IV length/);
errMessage);
}

// Zero-sized IV should be rejected in GCM mode.
assert.throws(
() => crypto.createCipheriv('aes-128-gcm', Buffer.alloc(16),
Buffer.alloc(0)),
/Invalid IV length/);
errMessage);

// But all other IV lengths should be accepted.
for (let n = 1; n < 256; n += 1) {
Expand Down
11 changes: 6 additions & 5 deletions test/parallel/test-crypto-dh.js
Original file line number Diff line number Diff line change
Expand Up @@ -295,14 +295,15 @@ if (availableCurves.has('prime256v1') && availableCurves.has('secp256k1')) {
// rejected.
ecdh5.setPrivateKey(cafebabeKey, 'hex');

[ // Some invalid private keys for the secp256k1 curve.
'0000000000000000000000000000000000000000000000000000000000000000',
'FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141',
'FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF',
// Some invalid private keys for the secp256k1 curve.
const errMessage = /^Error: Private key is not valid for specified curve\.$/;
['0000000000000000000000000000000000000000000000000000000000000000',
'FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141',
'FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF',
].forEach((element) => {
assert.throws(() => {
ecdh5.setPrivateKey(element, 'hex');
}, /^Error: Private key is not valid for specified curve\.$/);
}, errMessage);
// Verify object state did not change.
assert.strictEqual(ecdh5.getPrivateKey('hex'), cafebabeKey);
});
Expand Down
Loading

0 comments on commit 1f32d9e

Please sign in to comment.