Skip to content

Commit

Permalink
test: fix tests so they work in worker threads
Browse files Browse the repository at this point in the history
Use the `cwd` option for child_process instead of `process.chdir()` to
allow tests to work with worker threads.

PR-URL: #26453
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
  • Loading branch information
richardlau committed Mar 7, 2019
1 parent 76e67e9 commit 82a256a
Show file tree
Hide file tree
Showing 8 changed files with 24 additions and 44 deletions.
7 changes: 1 addition & 6 deletions test/parallel/test-cli-eval.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ const path = require('path');
const fixtures = require('../common/fixtures');
const nodejs = `"${process.execPath}"`;

if (!common.isMainThread)
common.skip('process.chdir is not available in Workers');

if (process.argv.length > 2) {
console.log(process.argv.slice(2).join(' '));
process.exit(0);
Expand Down Expand Up @@ -98,16 +95,14 @@ child.exec(`${nodejs} --print "os.platform()"`,
}));

// Module path resolve bug regression test.
const cwd = process.cwd();
process.chdir(path.resolve(__dirname, '../../'));
child.exec(`${nodejs} --eval "require('./test/parallel/test-cli-eval.js')"`,
{ cwd: path.resolve(__dirname, '../../') },
common.mustCall((err, stdout, stderr) => {
assert.strictEqual(err.code, 42);
assert.strictEqual(
stdout, 'Loaded as a module, exiting with status code 42.\n');
assert.strictEqual(stderr, '');
}));
process.chdir(cwd);

// Missing argument should not crash.
child.exec(`${nodejs} -e`, common.mustCall((err, stdout, stderr) => {
Expand Down
5 changes: 1 addition & 4 deletions test/parallel/test-cli-node-options-disallowed.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
const common = require('../common');
if (process.config.variables.node_without_node_options)
common.skip('missing NODE_OPTIONS support');
if (!common.isMainThread)
common.skip('process.chdir is not available in Workers');

// Test options specified by env variable.

Expand All @@ -12,7 +10,6 @@ const exec = require('child_process').execFile;

const tmpdir = require('../common/tmpdir');
tmpdir.refresh();
process.chdir(tmpdir.path);

disallow('--version');
disallow('-v');
Expand All @@ -32,7 +29,7 @@ disallow('--');

function disallow(opt) {
const env = Object.assign({}, process.env, { NODE_OPTIONS: opt });
exec(process.execPath, { env }, common.mustCall(function(err) {
exec(process.execPath, { cwd: tmpdir.path, env }, common.mustCall((err) => {
const message = err.message.split(/\r?\n/)[1];
const expect = `${process.execPath}: ${opt} is not allowed in NODE_OPTIONS`;

Expand Down
4 changes: 1 addition & 3 deletions test/parallel/test-cli-node-options.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
const common = require('../common');
if (process.config.variables.node_without_node_options)
common.skip('missing NODE_OPTIONS support');
if (!common.isMainThread)
common.skip('process.chdir is not available in Workers');

// Test options specified by env variable.

Expand All @@ -12,7 +10,6 @@ const exec = require('child_process').execFile;

const tmpdir = require('../common/tmpdir');
tmpdir.refresh();
process.chdir(tmpdir.path);

const printA = require.resolve('../fixtures/printA.js');
expect(`-r ${printA}`, 'A\nB\n');
Expand Down Expand Up @@ -64,6 +61,7 @@ expect('--stack-trace-limit=100',
function expect(opt, want, command = 'console.log("B")', wantsError = false) {
const argv = ['-e', command];
const opts = {
cwd: tmpdir.path,
env: Object.assign({}, process.env, { NODE_OPTIONS: opt }),
maxBuffer: 1e6,
};
Expand Down
15 changes: 6 additions & 9 deletions test/parallel/test-preload-print-process-argv.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,28 @@
// This tests that process.argv is the same in the preloaded module
// and the user module.

const common = require('../common');
require('../common');

const tmpdir = require('../common/tmpdir');
const assert = require('assert');
const { join } = require('path');
const { spawnSync } = require('child_process');
const fs = require('fs');

if (!common.isMainThread) {
common.skip('Cannot chdir to the tmp directory in workers');
}

tmpdir.refresh();

process.chdir(tmpdir.path);
fs.writeFileSync(
'preload.js',
join(tmpdir.path, 'preload.js'),
'console.log(JSON.stringify(process.argv));',
'utf-8');

fs.writeFileSync(
'main.js',
join(tmpdir.path, 'main.js'),
'console.log(JSON.stringify(process.argv));',
'utf-8');

const child = spawnSync(process.execPath, ['-r', './preload.js', 'main.js']);
const child = spawnSync(process.execPath, ['-r', './preload.js', 'main.js'],
{ cwd: tmpdir.path });

if (child.status !== 0) {
console.log(child.stderr.toString());
Expand Down
7 changes: 3 additions & 4 deletions test/parallel/test-preload.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ const fixtures = require('../common/fixtures');
// Refs: https://github.com/nodejs/node/pull/2253
if (common.isSunOS)
common.skip('unreliable on SunOS');
if (!common.isMainThread)
common.skip('process.chdir is not available in Workers');

const assert = require('assert');
const childProcess = require('child_process');
Expand Down Expand Up @@ -133,9 +131,9 @@ childProcess.exec(
);

// Test that preloading with a relative path works
process.chdir(fixtures.fixturesDir);
childProcess.exec(
`"${nodeBinary}" ${preloadOption(['./printA.js'])} "${fixtureB}"`,
{ cwd: fixtures.fixturesDir },
common.mustCall(function(err, stdout, stderr) {
assert.ifError(err);
assert.strictEqual(stdout, 'A\nB\n');
Expand All @@ -145,6 +143,7 @@ if (common.isWindows) {
// https://github.com/nodejs/node/issues/21918
childProcess.exec(
`"${nodeBinary}" ${preloadOption(['.\\printA.js'])} "${fixtureB}"`,
{ cwd: fixtures.fixturesDir },
common.mustCall(function(err, stdout, stderr) {
assert.ifError(err);
assert.strictEqual(stdout, 'A\nB\n');
Expand All @@ -153,10 +152,10 @@ if (common.isWindows) {
}

// https://github.com/nodejs/node/issues/1691
process.chdir(fixtures.fixturesDir);
childProcess.exec(
`"${nodeBinary}" --require ` +
`"${fixtures.path('cluster-preload.js')}" cluster-preload-test.js`,
{ cwd: fixtures.fixturesDir },
function(err, stdout, stderr) {
assert.ifError(err);
assert.ok(/worker terminated with code 43/.test(stdout));
Expand Down
10 changes: 4 additions & 6 deletions test/parallel/test-tick-processor-arguments.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,18 @@ const fs = require('fs');
const assert = require('assert');
const { spawnSync } = require('child_process');

if (!common.isMainThread)
common.skip('chdir not available in workers');
if (!common.enoughTestMem)
common.skip('skipped due to memory requirements');
if (common.isAIX)
common.skip('does not work on AIX');

tmpdir.refresh();
process.chdir(tmpdir.path);

// Generate log file.
spawnSync(process.execPath, [ '--prof', '-p', '42' ]);
spawnSync(process.execPath, [ '--prof', '-p', '42' ], { cwd: tmpdir.path });

const logfile = fs.readdirSync('.').filter((name) => name.endsWith('.log'))[0];
const files = fs.readdirSync(tmpdir.path);
const logfile = files.filter((name) => /\.log$/.test(name))[0];
assert(logfile);

// Make sure that the --preprocess argument is passed through correctly,
Expand All @@ -28,7 +26,7 @@ assert(logfile);
const { stdout } = spawnSync(
process.execPath,
[ '--prof-process', '--preprocess', logfile ],
{ encoding: 'utf8' });
{ cwd: tmpdir.path, encoding: 'utf8' });

// Make sure that the result is valid JSON.
JSON.parse(stdout);
5 changes: 1 addition & 4 deletions test/parallel/test-trace-events-environment.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@ const tmpdir = require('../common/tmpdir');

// This tests the emission of node.environment trace events

if (!common.isMainThread)
common.skip('process.chdir is not available in Workers');

const names = new Set([
'Environment',
'RunAndClearNativeImmediates',
Expand All @@ -32,10 +29,10 @@ if (process.argv[2] === 'child') {
setTimeout(() => { 1 + 1; }, 1);
} else {
tmpdir.refresh();
process.chdir(tmpdir.path);

const proc = cp.fork(__filename,
[ 'child' ], {
cwd: tmpdir.path,
execArgv: [
'--trace-event-categories',
'node.environment'
Expand Down
15 changes: 7 additions & 8 deletions test/parallel/test-worker-prof.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
'use strict';
const common = require('../common');
require('../common');
const tmpdir = require('../common/tmpdir');
const fs = require('fs');
const assert = require('assert');
const { join } = require('path');
const { spawnSync } = require('child_process');
const { Worker } = require('worker_threads');

if (!common.isMainThread)
common.skip('process.chdir is not available in Workers');

// Test that --prof also tracks Worker threads.
// Refs: https://github.com/nodejs/node/issues/24016

Expand All @@ -23,13 +21,14 @@ if (process.argv[2] === 'child') {
}

tmpdir.refresh();
process.chdir(tmpdir.path);
spawnSync(process.execPath, ['--prof', __filename, 'child']);
const logfiles = fs.readdirSync('.').filter((name) => /\.log$/.test(name));
spawnSync(process.execPath, ['--prof', __filename, 'child'],
{ cwd: tmpdir.path });
const files = fs.readdirSync(tmpdir.path);
const logfiles = files.filter((name) => /\.log$/.test(name));
assert.strictEqual(logfiles.length, 2); // Parent thread + child thread.

for (const logfile of logfiles) {
const lines = fs.readFileSync(logfile, 'utf8').split('\n');
const lines = fs.readFileSync(join(tmpdir.path, logfile), 'utf8').split('\n');
const ticks = lines.filter((line) => /^tick,/.test(line)).length;

// Test that at least 15 ticks have been recorded for both parent and child
Expand Down

0 comments on commit 82a256a

Please sign in to comment.