test: begin normalizing fixtures use

Adds a new `../common/fixtures' module to begin normalizing `test/fixtures` use. Our test code is a bit inconsistent with regards to use of the fixtures directory. Some code uses `path.join()`, some code uses string concats, some other code uses template strings, etc. In mnay cases, significant duplication of code is seen when accessing fixture files, etc. This updates many (but by no means all) of the tests in the test suite to use the new consistent API. There are still many more to update, which would make an excelent Code-n-Learn exercise. Backport-PR-URL: #16265 PR-URL: #14332 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
nodejs · Oct 26, 2017 · 1fdbaed · 1fdbaed
1 parent 88b9572
commit 1fdbaed
Show file tree

Hide file tree

Showing 113 changed files with 466 additions and 481 deletions.
diff --git a/test/common/README.md b/test/common/README.md
@@ -316,6 +316,37 @@ Decrements the `Countdown` counter.
 Specifies the remaining number of times `Countdown.prototype.dec()` must be
 called before the callback is invoked.
 
+## Fixtures Module
+
+The `common/fixtures` module provides convenience methods for working with
+files in the `test/fixtures` directory.
+
+### fixtures.fixturesDir
+
+* [&lt;String>]
+
+The absolute path to the `test/fixtures/` directory.
+
+### fixtures.path(...args)
+
+* `...args` [&lt;String>]
+
+Returns the result of `path.join(fixtures.fixturesDir, ...args)`.
+
+### fixtures.readSync(args[, enc])
+
+* `args` [&lt;String>] | [&lt;Array>]
+
+Returns the result of
+`fs.readFileSync(path.join(fixtures.fixturesDir, ...args), 'enc')`.
+
+### fixtures.readKey(arg[, enc])
+
+* `arg` [&lt;String>]
+
+Returns the result of
+`fs.readFileSync(path.join(fixtures.fixturesDir, 'keys', arg), 'enc')`.
+
 ## WPT Module
 
 The wpt.js module is a port of parts of

diff --git a/test/common/fixtures.js b/test/common/fixtures.js
@@ -0,0 +1,28 @@
+/* eslint-disable required-modules */
+'use strict';
+
+const path = require('path');
+const fs = require('fs');
+
+const fixturesDir = path.join(__dirname, '..', 'fixtures');
+
+function fixturesPath(...args) {
+  return path.join(fixturesDir, ...args);
+}
+
+function readFixtureSync(args, enc) {
+  if (Array.isArray(args))
+    return fs.readFileSync(fixturesPath(...args), enc);
+  return fs.readFileSync(fixturesPath(args), enc);
+}
+
+function readFixtureKey(name, enc) {
+  return fs.readFileSync(fixturesPath('keys', name), enc);
+}
+
+module.exports = {
+  fixturesDir,
+  path: fixturesPath,
+  readSync: readFixtureSync,
+  readKey: readFixtureKey
+};
diff --git a/test/common/index.js b/test/common/index.js
@@ -8,13 +8,15 @@ const { exec, execSync, spawn, spawnSync } = require('child_process');
 const stream = require('stream');
 const util = require('util');
 const Timer = process.binding('timer_wrap').Timer;
+const { fixturesDir } = require('./fixtures');
 
 const testRoot = process.env.NODE_TEST_DIR ?
   fs.realpathSync(process.env.NODE_TEST_DIR) : path.resolve(__dirname, '..');
 
 const noop = () => {};
 
-exports.fixturesDir = path.join(__dirname, '..', 'fixtures');
+exports.fixturesDir = fixturesDir;
+
 exports.tmpDirName = 'tmp';
 // PORT should match the definition in test/testpy/__init__.py.
 exports.PORT = +process.env.NODE_COMMON_PORT || 12346;

diff --git a/test/parallel/test-child-process-detached.js b/test/parallel/test-child-process-detached.js
@@ -1,11 +1,10 @@
 'use strict';
-const common = require('../common');
+require('../common');
 const assert = require('assert');
-const path = require('path');
+const fixtures = require('../common/fixtures');
 
 const spawn = require('child_process').spawn;
-const childPath = path.join(common.fixturesDir,
-                            'parent-process-nonpersistent.js');
+const childPath = fixtures.path('parent-process-nonpersistent.js');
 let persistentPid = -1;
 
 const child = spawn(process.execPath, [ childPath ]);

diff --git a/test/parallel/test-child-process-execfile.js b/test/parallel/test-child-process-execfile.js
@@ -2,10 +2,10 @@
 const common = require('../common');
 const assert = require('assert');
 const execFile = require('child_process').execFile;
-const path = require('path');
 const uv = process.binding('uv');
+const fixtures = require('../common/fixtures');
 
-const fixture = path.join(common.fixturesDir, 'exit.js');
+const fixture = fixtures.path('exit.js');
 
 {
   execFile(

diff --git a/test/parallel/test-child-process-exit-code.js b/test/parallel/test-child-process-exit-code.js
@@ -2,18 +2,17 @@
 const common = require('../common');
 const assert = require('assert');
 const spawn = require('child_process').spawn;
-const path = require('path');
+const fixtures = require('../common/fixtures');
 
-const exitScript = path.join(common.fixturesDir, 'exit.js');
+const exitScript = fixtures.path('exit.js');
 const exitChild = spawn(process.argv[0], [exitScript, 23]);
 exitChild.on('exit', common.mustCall(function(code, signal) {
   assert.strictEqual(code, 23);
   assert.strictEqual(signal, null);
 }));
 
 
-const errorScript = path.join(common.fixturesDir,
-                              'child_process_should_emit_error.js');
+const errorScript = fixtures.path('child_process_should_emit_error.js');
 const errorChild = spawn(process.argv[0], [errorScript]);
 errorChild.on('exit', common.mustCall(function(code, signal) {
   assert.ok(code !== 0);

diff --git a/test/parallel/test-child-process-fork-close.js b/test/parallel/test-child-process-fork-close.js
@@ -2,8 +2,9 @@
 const common = require('../common');
 const assert = require('assert');
 const fork = require('child_process').fork;
+const fixtures = require('../common/fixtures');
 
-const cp = fork(`${common.fixturesDir}/child-process-message-and-exit.js`);
+const cp = fork(fixtures.path('child-process-message-and-exit.js'));
 
 let gotMessage = false;
 let gotExit = false;

diff --git a/test/parallel/test-child-process-fork.js b/test/parallel/test-child-process-fork.js
@@ -3,8 +3,9 @@ const common = require('../common');
 const assert = require('assert');
 const fork = require('child_process').fork;
 const args = ['foo', 'bar'];
+const fixtures = require('../common/fixtures');
 
-const n = fork(`${common.fixturesDir}/child-process-spawn-node.js`, args);
+const n = fork(fixtures.path('child-process-spawn-node.js'), args);
 assert.deepStrictEqual(args, ['foo', 'bar']);
 
 n.on('message', function(m) {

diff --git a/test/parallel/test-child-process-fork3.js b/test/parallel/test-child-process-fork3.js
@@ -1,5 +1,6 @@
 'use strict';
-const common = require('../common');
+require('../common');
 const child_process = require('child_process');
+const fixtures = require('../common/fixtures');
 
-child_process.fork(`${common.fixturesDir}/empty.js`); // should not hang
+child_process.fork(fixtures.path('empty.js')); // should not hang
diff --git a/test/parallel/test-child-process-ipc.js b/test/parallel/test-child-process-ipc.js
@@ -1,13 +1,12 @@
 'use strict';
 
-const common = require('../common');
+require('../common');
 const assert = require('assert');
 
-const spawn = require('child_process').spawn;
+const { spawn } = require('child_process');
+const fixtures = require('../common/fixtures');
 
-const path = require('path');
-
-const sub = path.join(common.fixturesDir, 'echo.js');
+const sub = fixtures.path('echo.js');
 
 let gotHelloWorld = false;
 let gotEcho = false;

diff --git a/test/parallel/test-child-process-send-after-close.js b/test/parallel/test-child-process-send-after-close.js
@@ -2,8 +2,9 @@
 const common = require('../common');
 const assert = require('assert');
 const cp = require('child_process');
-const path = require('path');
-const fixture = path.join(common.fixturesDir, 'empty.js');
+const fixtures = require('../common/fixtures');
+
+const fixture = fixtures.path('empty.js');
 const child = cp.fork(fixture);
 
 child.on('close', common.mustCall((code, signal) => {

diff --git a/test/parallel/test-child-process-send-returns-boolean.js b/test/parallel/test-child-process-send-returns-boolean.js
@@ -1,11 +1,11 @@
 'use strict';
 const common = require('../common');
 const assert = require('assert');
-const path = require('path');
 const net = require('net');
 const { fork, spawn } = require('child_process');
+const fixtures = require('../common/fixtures');
 
-const emptyFile = path.join(common.fixturesDir, 'empty.js');
+const emptyFile = fixtures.path('empty.js');
 
 const n = fork(emptyFile);
 

diff --git a/test/parallel/test-child-process-spawn-typeerror.js b/test/parallel/test-child-process-spawn-typeerror.js
@@ -1,15 +1,14 @@
 'use strict';
 const common = require('../common');
 const assert = require('assert');
-const child_process = require('child_process');
-const spawn = child_process.spawn;
-const fork = child_process.fork;
-const execFile = child_process.execFile;
+const { spawn, fork, execFile } = require('child_process');
+const fixtures = require('../common/fixtures');
 const cmd = common.isWindows ? 'rundll32' : 'ls';
 const invalidcmd = 'hopefully_you_dont_have_this_on_your_machine';
 const invalidArgsMsg = /Incorrect value of args option/;
 const invalidOptionsMsg = /"options" argument must be an object/;
-const empty = `${common.fixturesDir}/empty.js`;
+
+const empty = fixtures.path('empty.js');
 
 assert.throws(function() {
   const child = spawn(invalidcmd, 'this is not an array');

diff --git a/test/parallel/test-child-process-stdout-flush.js b/test/parallel/test-child-process-stdout-flush.js
@@ -1,9 +1,10 @@
 'use strict';
 const common = require('../common');
 const assert = require('assert');
-const path = require('path');
 const spawn = require('child_process').spawn;
-const sub = path.join(common.fixturesDir, 'print-chars.js');
+const fixtures = require('../common/fixtures');
+
+const sub = fixtures.path('print-chars.js');
 
 const n = 500000;
 

diff --git a/test/parallel/test-cli-eval.js b/test/parallel/test-cli-eval.js
@@ -10,6 +10,7 @@ const common = require('../common');
 const assert = require('assert');
 const child = require('child_process');
 const path = require('path');
+const fixtures = require('../common/fixtures');
 const nodejs = `"${process.execPath}"`;
 
 if (process.argv.length > 2) {
@@ -117,7 +118,7 @@ child.exec(`${nodejs} --use-strict -p process.execArgv`,
 
 // Regression test for https://github.com/nodejs/node/issues/3574.
 {
-  const emptyFile = path.join(common.fixturesDir, 'empty.js');
+  const emptyFile = fixtures.path('empty.js');
 
   child.exec(`${nodejs} -e 'require("child_process").fork("${emptyFile}")'`,
              common.mustCall((err, stdout, stderr) => {

diff --git a/test/parallel/test-cli-syntax.js b/test/parallel/test-cli-syntax.js
@@ -1,9 +1,9 @@
 'use strict';
 
-const common = require('../common');
+require('../common');
 const assert = require('assert');
 const spawnSync = require('child_process').spawnSync;
-const path = require('path');
+const fixtures = require('../common/fixtures');
 
 const node = process.execPath;
 
@@ -24,7 +24,7 @@ const notFoundRE = /^Error: Cannot find module/m;
   'syntax/good_syntax_shebang',
   'syntax/illegal_if_not_wrapped.js'
 ].forEach(function(file) {
-  file = path.join(common.fixturesDir, file);
+  file = fixtures.path(file);
 
   // loop each possible option, `-c` or `--check`
   syntaxArgs.forEach(function(args) {
@@ -45,7 +45,7 @@ const notFoundRE = /^Error: Cannot find module/m;
   'syntax/bad_syntax_shebang.js',
   'syntax/bad_syntax_shebang'
 ].forEach(function(file) {
-  file = path.join(common.fixturesDir, file);
+  file = fixtures.path(file);
 
   // loop each possible option, `-c` or `--check`
   syntaxArgs.forEach(function(args) {
@@ -67,7 +67,7 @@ const notFoundRE = /^Error: Cannot find module/m;
   'syntax/file_not_found.js',
   'syntax/file_not_found'
 ].forEach(function(file) {
-  file = path.join(common.fixturesDir, file);
+  file = fixtures.path(file);
 
   // loop each possible option, `-c` or `--check`
   syntaxArgs.forEach(function(args) {

diff --git a/test/parallel/test-crypto-binary-default.js b/test/parallel/test-crypto-binary-default.js
@@ -11,19 +11,18 @@ if (!common.hasCrypto)
 const assert = require('assert');
 const crypto = require('crypto');
 const fs = require('fs');
-const path = require('path');
 const tls = require('tls');
+const fixtures = require('../common/fixtures');
 const DH_NOT_SUITABLE_GENERATOR = crypto.constants.DH_NOT_SUITABLE_GENERATOR;
-const fixtDir = common.fixturesDir;
 
 crypto.DEFAULT_ENCODING = 'latin1';
 
 // Test Certificates
-const certPem = fs.readFileSync(`${fixtDir}/test_cert.pem`, 'ascii');
-const certPfx = fs.readFileSync(`${fixtDir}/test_cert.pfx`);
-const keyPem = fs.readFileSync(`${fixtDir}/test_key.pem`, 'ascii');
-const rsaPubPem = fs.readFileSync(`${fixtDir}/test_rsa_pubkey.pem`, 'ascii');
-const rsaKeyPem = fs.readFileSync(`${fixtDir}/test_rsa_privkey.pem`, 'ascii');
+const certPem = fixtures.readSync('test_cert.pem', 'ascii');
+const certPfx = fixtures.readSync('test_cert.pfx');
+const keyPem = fixtures.readSync('test_key.pem', 'ascii');
+const rsaPubPem = fixtures.readSync('test_rsa_pubkey.pem', 'ascii');
+const rsaKeyPem = fixtures.readSync('test_rsa_privkey.pem', 'ascii');
 
 // PFX tests
 assert.doesNotThrow(function() {
@@ -384,7 +383,7 @@ const h2 = crypto.createHash('sha1').update('Test').update('123').digest('hex');
 assert.strictEqual(h1, h2, 'multipled updates');
 
 // Test hashing for binary files
-const fn = path.join(fixtDir, 'sample.png');
+const fn = fixtures.path('sample.png');
 const sha1Hash = crypto.createHash('sha1');
 const fileStream = fs.createReadStream(fn);
 fileStream.on('data', function(data) {
@@ -593,9 +592,8 @@ assert.strictEqual(rsaVerify.verify(rsaPubPem, rsaSignature, 'hex'), true);
 // Test RSA signing and verification
 //
 {
-  const privateKey = fs.readFileSync(`${fixtDir}/test_rsa_privkey_2.pem`);
-
-  const publicKey = fs.readFileSync(`${fixtDir}/test_rsa_pubkey_2.pem`);
+  const privateKey = fixtures.readSync('test_rsa_privkey_2.pem');
+  const publicKey = fixtures.readSync('test_rsa_pubkey_2.pem');
 
   const input = 'I AM THE WALRUS';
 
@@ -623,9 +621,8 @@ assert.strictEqual(rsaVerify.verify(rsaPubPem, rsaSignature, 'hex'), true);
 // Test DSA signing and verification
 //
 {
-  const privateKey = fs.readFileSync(`${fixtDir}/test_dsa_privkey.pem`);
-
-  const publicKey = fs.readFileSync(`${fixtDir}/test_dsa_pubkey.pem`);
+  const privateKey = fixtures.readSync('test_dsa_privkey.pem');
+  const publicKey = fixtures.readSync('test_dsa_pubkey.pem');
 
   const input = 'I AM THE WALRUS';
 

diff --git a/test/parallel/test-crypto-certificate.js b/test/parallel/test-crypto-certificate.js
@@ -5,15 +5,14 @@ if (!common.hasCrypto)
 
 const assert = require('assert');
 const crypto = require('crypto');
+const fixtures = require('../common/fixtures');
 
 crypto.DEFAULT_ENCODING = 'buffer';
 
-const fs = require('fs');
-
 // Test Certificates
-const spkacValid = fs.readFileSync(`${common.fixturesDir}/spkac.valid`);
-const spkacFail = fs.readFileSync(`${common.fixturesDir}/spkac.fail`);
-const spkacPem = fs.readFileSync(`${common.fixturesDir}/spkac.pem`);
+const spkacValid = fixtures.readSync('spkac.valid');
+const spkacFail = fixtures.readSync('spkac.fail');
+const spkacPem = fixtures.readSync('spkac.pem');
 
 const certificate = new crypto.Certificate();
 

diff --git a/test/parallel/test-crypto-fips.js b/test/parallel/test-crypto-fips.js
@@ -6,13 +6,16 @@ if (!common.hasCrypto)
 const assert = require('assert');
 const spawnSync = require('child_process').spawnSync;
 const path = require('path');
+const fixtures = require('../common/fixtures');
 
 const FIPS_ENABLED = 1;
 const FIPS_DISABLED = 0;
 const FIPS_ERROR_STRING = 'Error: Cannot set FIPS mode';
 const OPTION_ERROR_STRING = 'bad option';
-const CNF_FIPS_ON = path.join(common.fixturesDir, 'openssl_fips_enabled.cnf');
-const CNF_FIPS_OFF = path.join(common.fixturesDir, 'openssl_fips_disabled.cnf');
+
+const CNF_FIPS_ON = fixtures.path('openssl_fips_enabled.cnf');
+const CNF_FIPS_OFF = fixtures.path('openssl_fips_disabled.cnf');
+
 let num_children_ok = 0;
 
 function compiledWithFips() {