Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: begin normalizing fixtures use #14332

Closed
wants to merge 1 commit into from
+496 −507
Closed

test: begin normalizing fixtures use #14332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions test/common/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,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

* [<String>]

The absolute path to the `test/fixtures/` directory.

### fixtures.path(...args)

* `...args` [<String>]

Returns the result of `path.join(fixtures.fixturesDir, ...args)`.

### fixtures.readSync(args[, enc])

* `args` [<String>] | [<Array>]

Returns the result of
`fs.readFileSync(path.join(fixtures.fixturesDir, ...args), 'enc')`.

### fixtures.readKey(arg[, enc])

* `arg` [<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
Expand Down
28 changes: 28 additions & 0 deletions test/common/fixtures.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless I'm misreading this (quite possible), readFixtureKey('agent1-key.pem') == readFixtureSync('keys', 'agent1-key.pem'),. If so I'd rather we just have the latter, having an extra method adds unnecessary complexity for no particular gain IMO.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Take another look at the signatures. readFixtureSync(...args) takes an arbitrary number of args that are concatenated using path.join(...args), whereas readFixtureKey(name, enc) takes a single path segment and an optional encoding. Yes, readFixtureKey() uses readFixtureSync() but they are not equivalent to one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not saying they're identical, I'm saying that fixtures.readKey() is a specialised version of fixtures.readSync() (the one where the first arg is keys and you can omit the encoding).

You're right, I should have said:

fixtures.readKey('agent1-key.pem') == fixtures.readSync(['keys', 'agent1-key.pem'], 'utf8')

For me the latter is clearer (which is why I don't understand the need for readFixtureKey())

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally because I find the necessity of using the [...] syntax a bit ugly and unfriendly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough, I have a mild preference the other way, but not strongly enough to block this.

}

module.exports = {
fixturesDir,
path: fixturesPath,
readSync: readFixtureSync,
readKey: readFixtureKey
};
4 changes: 3 additions & 1 deletion test/common/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,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;
Expand Down
8 changes: 4 additions & 4 deletions test/parallel/test-async-wrap-GH13045.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ if (!common.hasCrypto)

const assert = require('assert');
const https = require('https');
const fs = require('fs');
const fixtures = require('../common/fixtures');

const serverOptions = {
key: fs.readFileSync(`${common.fixturesDir}/keys/agent1-key.pem`),
cert: fs.readFileSync(`${common.fixturesDir}/keys/agent1-cert.pem`),
ca: fs.readFileSync(`${common.fixturesDir}/keys/ca1-cert.pem`)
key: fixtures.readKey('agent1-key.pem'),
cert: fixtures.readKey('agent1-cert.pem'),
ca: fixtures.readKey('ca1-cert.pem')
};

const server = https.createServer(serverOptions, common.mustCall((req, res) => {
Expand Down
8 changes: 5 additions & 3 deletions test/parallel/test-async-wrap-getasyncid.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const assert = require('assert');
const fs = require('fs');
const net = require('net');
const providers = Object.assign({}, process.binding('async_wrap').Providers);
const fixtures = require('../common/fixtures');

// Make sure that all Providers are tested.
{
Expand Down Expand Up @@ -218,9 +219,10 @@ if (common.hasCrypto) {
const TCP = process.binding('tcp_wrap').TCP;
const tcp = new TCP();

const ca = fs.readFileSync(common.fixturesDir + '/test_ca.pem', 'ascii');
const cert = fs.readFileSync(common.fixturesDir + '/test_cert.pem', 'ascii');
const key = fs.readFileSync(common.fixturesDir + '/test_key.pem', 'ascii');
const ca = fixtures.readSync('test_ca.pem', 'ascii');
const cert = fixtures.readSync('test_cert.pem', 'ascii');
const key = fixtures.readSync('test_key.pem', 'ascii');

const credentials = require('tls').createSecureContext({ ca, cert, key });

// TLSWrap is exposed, but needs to be instantiated via tls_wrap.wrap().
Expand Down
7 changes: 3 additions & 4 deletions test/parallel/test-child-process-detached.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,12 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'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 ]);
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-child-process-execfile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
7 changes: 3 additions & 4 deletions test/parallel/test-child-process-exit-code.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,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);
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-child-process-fork-close.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,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;
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-child-process-fork-stdio-string-variant.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ const common = require('../common');

const assert = require('assert');
const fork = require('child_process').fork;
const fixtures = require('../common/fixtures');

const childScript = `${common.fixturesDir}/child-process-spawn-node`;
const childScript = fixtures.path('child-process-spawn-node');
const errorRegexp = /^TypeError: Incorrect value of stdio option:/;
const malFormedOpts = { stdio: '33' };
const payload = { hello: 'world' };
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-child-process-fork.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,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.strictEqual(n.channel, n._channel);
assert.deepStrictEqual(args, ['foo', 'bar']);
Expand Down
5 changes: 3 additions & 2 deletions test/parallel/test-child-process-fork3.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'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
9 changes: 4 additions & 5 deletions test/parallel/test-child-process-ipc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,13 @@

'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;
Expand Down
5 changes: 3 additions & 2 deletions test/parallel/test-child-process-send-after-close.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-child-process-send-returns-boolean.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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);

Expand Down
9 changes: 4 additions & 5 deletions test/parallel/test-child-process-spawn-typeerror.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,16 @@
'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 invalidFileMsg =
/^TypeError: "file" argument must be a non-empty string$/;
const empty = `${common.fixturesDir}/empty.js`;

const empty = fixtures.path('empty.js');

assert.throws(function() {
const child = spawn(invalidcmd, 'this is not an array');
Expand Down
5 changes: 3 additions & 2 deletions test/parallel/test-child-process-stdout-flush.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,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;

Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-cli-eval.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,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) {
Expand Down Expand Up @@ -138,7 +139,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) => {
Expand Down
8 changes: 4 additions & 4 deletions test/parallel/test-cli-syntax.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
const common = require('../common');
const assert = require('assert');
const { exec, spawnSync } = require('child_process');
const path = require('path');
const fixtures = require('../common/fixtures');

const node = process.execPath;

Expand All @@ -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) {
Expand All @@ -46,7 +46,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) {
Expand All @@ -73,7 +73,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) {
Expand Down
Loading