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.

Backport-PR-URL: #14370
PR-URL: #13770
Reviewed-By: Colin Ihrig <[email protected]>
  • Loading branch information
vsemozhetbyt authored and MylesBorins committed Jul 31, 2017
1 parent 910fa50 commit 9cfec4b
Show file tree
Hide file tree
Showing 40 changed files with 195 additions and 172 deletions.
3 changes: 2 additions & 1 deletion test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,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
4 changes: 2 additions & 2 deletions test/debugger/helper-debugger-repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,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 @@ -80,9 +80,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 @@ -101,7 +103,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/test-inspector.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,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
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)));
}
9 changes: 5 additions & 4 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 @@ -53,8 +56,7 @@ const syntaxArgs = [
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 All @@ -76,8 +78,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
28 changes: 18 additions & 10 deletions test/parallel/test-crypto-authenticated.js
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,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 @@ -357,14 +364,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 @@ -383,7 +390,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 @@ -395,7 +402,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 @@ -406,7 +413,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 @@ -415,15 +422,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 @@ -434,7 +441,7 @@ for (const i in TEST_CASES) {
Buffer.from(test.key, 'hex'),
Buffer.alloc(0)
);
}, /Invalid IV length/);
}, errMessages.length);
}
}

Expand All @@ -446,6 +453,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 @@ -280,14 +280,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
3 changes: 2 additions & 1 deletion test/parallel/test-crypto.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ validateList(cryptoCiphers);
const tlsCiphers = tls.getCiphers();
assert(tls.getCiphers().includes('aes256-sha'));
// There should be no capital letters in any element.
assert(tlsCiphers.every((value) => /^[^A-Z]+$/.test(value)));
const noCapitals = /^[^A-Z]+$/;
assert(tlsCiphers.every((value) => noCapitals.test(value)));
validateList(tlsCiphers);

// Assert that we have sha and sha1 but not SHA and SHA1.
Expand Down
12 changes: 7 additions & 5 deletions test/parallel/test-error-reporting.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ function errExec(script, callback) {
});
}

const syntaxErrorMessage = /SyntaxError/;


// Simple throw error
errExec('throws_error.js', common.mustCall(function(err, stdout, stderr) {
Expand All @@ -30,30 +32,30 @@ errExec('throws_error.js', common.mustCall(function(err, stdout, stderr) {

// Trying to JSON.parse(undefined)
errExec('throws_error2.js', common.mustCall(function(err, stdout, stderr) {
assert.ok(/SyntaxError/.test(stderr));
assert.ok(syntaxErrorMessage.test(stderr));
}));


// Trying to JSON.parse(undefined) in nextTick
errExec('throws_error3.js', common.mustCall(function(err, stdout, stderr) {
assert.ok(/SyntaxError/.test(stderr));
assert.ok(syntaxErrorMessage.test(stderr));
}));


// throw ILLEGAL error
errExec('throws_error4.js', common.mustCall(function(err, stdout, stderr) {
assert.ok(/\/\*\*/.test(stderr));
assert.ok(/SyntaxError/.test(stderr));
assert.ok(syntaxErrorMessage.test(stderr));
}));

// Specific long exception line doesn't result in stack overflow
errExec('throws_error5.js', common.mustCall(function(err, stdout, stderr) {
assert.ok(/SyntaxError/.test(stderr));
assert.ok(syntaxErrorMessage.test(stderr));
}));

// Long exception line with length > errorBuffer doesn't result in assertion
errExec('throws_error6.js', common.mustCall(function(err, stdout, stderr) {
assert.ok(/SyntaxError/.test(stderr));
assert.ok(syntaxErrorMessage.test(stderr));
}));

// Object that throws in toString() doesn't print garbage
Expand Down
14 changes: 4 additions & 10 deletions test/parallel/test-event-emitter-max-listeners.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,10 @@ e.on('maxListeners', common.mustCall(function() {}));
// Should not corrupt the 'maxListeners' queue.
e.setMaxListeners(42);

assert.throws(function() {
e.setMaxListeners(NaN);
}, /^TypeError: "n" argument must be a positive number$/);
const maxError = /^TypeError: "n" argument must be a positive number$/;

assert.throws(function() {
e.setMaxListeners(-1);
}, /^TypeError: "n" argument must be a positive number$/);

assert.throws(function() {
e.setMaxListeners('and even this');
}, /^TypeError: "n" argument must be a positive number$/);
assert.throws(function() { e.setMaxListeners(NaN); }, maxError);
assert.throws(function() { e.setMaxListeners(-1); }, maxError);
assert.throws(function() { e.setMaxListeners('and even this'); }, maxError);

e.emit('maxListeners');
2 changes: 1 addition & 1 deletion test/parallel/test-fs-null-bytes.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ function check(async, sync) {
const expected = /Path must be a string without null bytes/;
const argsSync = Array.prototype.slice.call(arguments, 2);
const argsAsync = argsSync.concat((er) => {
assert(er && er.message.match(expected));
assert(er && expected.test(er.message));
assert.strictEqual(er.code, 'ENOENT');
});

Expand Down
11 changes: 6 additions & 5 deletions test/parallel/test-fs-read-stream-throw-type-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,19 @@ assert.doesNotThrow(function() {
fs.createReadStream(example, {encoding: 'utf8'});
});

const errMessage = /"options" argument must be a string or an object/;
assert.throws(function() {
fs.createReadStream(example, null);
}, /"options" argument must be a string or an object/);
}, errMessage);
assert.throws(function() {
fs.createReadStream(example, 123);
}, /"options" argument must be a string or an object/);
}, errMessage);
assert.throws(function() {
fs.createReadStream(example, 0);
}, /"options" argument must be a string or an object/);
}, errMessage);
assert.throws(function() {
fs.createReadStream(example, true);
}, /"options" argument must be a string or an object/);
}, errMessage);
assert.throws(function() {
fs.createReadStream(example, false);
}, /"options" argument must be a string or an object/);
}, errMessage);
2 changes: 1 addition & 1 deletion test/parallel/test-global-console-exists.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ process.on('warning', (warning) => {

process.stderr.write = (data) => {
if (write_calls === 0)
assert.ok(data.match(leak_warning));
assert.ok(leak_warning.test(data));
else
common.fail('stderr.write should be called only once');

Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-http-client-unescaped-path.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ const common = require('../common');
const assert = require('assert');
const http = require('http');

const errMessage = /contains unescaped characters/;
for (let i = 0; i <= 32; i += 1) {
const path = `bad${String.fromCharCode(i)}path`;
assert.throws(() => http.get({ path }, common.mustNotCall()),
/contains unescaped characters/);
assert.throws(() => http.get({ path }, common.mustNotCall()), errMessage);
}
4 changes: 2 additions & 2 deletions test/parallel/test-http-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,10 @@ process.on('exit', function() {
assert.strictEqual(4, requests_sent);

const hello = new RegExp('/hello');
assert.notStrictEqual(null, hello.exec(server_response));
assert.ok(hello.test(server_response));

const quit = new RegExp('/quit');
assert.notStrictEqual(null, quit.exec(server_response));
assert.ok(quit.test(server_response));

assert.strictEqual(true, client_got_eof);
});
Loading

0 comments on commit 9cfec4b

Please sign in to comment.