Skip to content

Allow Node arguments to be configured #2272

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.

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

Merged
merged 33 commits into from
Jan 18, 2020
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
103a243
feat: node-arguments for passing arguments to child processes node in…
Oct 17, 2019
0cc0abd
Remove unnecessary promise-wrapping
novemberborn Nov 10, 2019
3f6633c
Destructuring
novemberborn Nov 10, 2019
ae70f22
Tweak docs
novemberborn Nov 10, 2019
420c9b7
node-arguments: extend cli doc with example
Nov 24, 2019
8e51406
node-arguments simplify processing
Nov 24, 2019
d4acaeb
fixes
Nov 24, 2019
adfc212
fix(node-arguments): default value for config from file value
Nov 24, 2019
26c4448
fix: node-arguments test fix
Nov 24, 2019
5b99832
Clarify CLI use of --node-arguments
novemberborn Dec 1, 2019
9f00c05
Merge branch 'master' into node-arguments
novemberborn Dec 1, 2019
5cf8dd7
fix normalize arguments
Dec 7, 2019
28a2c06
fix all other minimist usages
Dec 7, 2019
1ae69c4
fix semicolon
Dec 7, 2019
c1a99d4
fix broken tests?
Dec 7, 2019
8e649b9
fix configuration for yargs parsing
Dec 7, 2019
e00e80f
memoize parse args
Dec 7, 2019
03fca0a
fix lint
Dec 7, 2019
9fcdc7b
fix quotes/double quotes
Dec 7, 2019
ae3836b
fix lint
Dec 7, 2019
be272c7
move _parseProcessArgs to top
Dec 7, 2019
b5d2796
remove node-arguments default for cli
Dec 9, 2019
18b7074
Merge branch 'master' into node-arguments
novemberborn Jan 12, 2020
037cc33
Simplify implementation
novemberborn Jan 12, 2020
518c44d
update lockfile to master
Jan 14, 2020
7aad65a
remove redundant \\
Jan 14, 2020
0e21309
fix quotes
Jan 14, 2020
c40194a
stdout/stderr logging for node-arguments test
Jan 14, 2020
f01a038
fix format
Jan 14, 2020
438148c
add stdout/stderr logs to another test
Jan 14, 2020
f670481
fix node-arguments integration test on windows
Jan 14, 2020
7ec7f34
Merge branch 'master' into node-arguments
novemberborn Jan 18, 2020
40e7716
Parse using arrgv
novemberborn Jan 18, 2020
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
1 change: 1 addition & 0 deletions docs/05-command-line.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ $ npx ava --help
--color Force color output
--no-color Disable color output
--reset-cache Reset AVA's compilation cache and exit
--node-arguments Configure Node.js arguments used to launch worker processes
--config JavaScript file for AVA to read its config from, instead of using package.json
or ava.config.js files

Expand Down
7 changes: 6 additions & 1 deletion docs/06-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@ To ignore files, prefix the pattern with an `!` (exclamation mark).
"testOptions": {
"babelrc": false
}
}
},
"nodeArguments": [
"--trace-deprecation",
"--napi-modules"
]
}
}
```
Expand All @@ -70,6 +74,7 @@ Arguments passed to the CLI will always take precedence over the CLI options con
- `babel`: test file specific Babel options. See our [Babel recipe](./recipes/babel.md#configuring-babel) for more details
- `babel.extensions`: extensions of test files that will be precompiled using AVA's Babel presets. Setting this overrides the default `"js"` value, so make sure to include that extension in the list
- `timeout`: Timeouts in AVA behave differently than in other test frameworks. AVA resets a timer after each test, forcing tests to quit if no new test results were received within the specified timeout. This can be used to handle stalled tests. See our [timeout documentation](./07-test-timeouts.md) for more options.
- `nodeArguments`: Configure Node.js arguments used to launch worker processes.

Note that providing files on the CLI overrides the `files` option.

Expand Down
39 changes: 21 additions & 18 deletions lib/api.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
'use strict';

const path = require('path');
const fs = require('fs');
const os = require('os');
Expand All @@ -15,6 +16,8 @@ const makeDir = require('make-dir');
const ms = require('ms');
const chunkd = require('chunkd');
const Emittery = require('emittery');
const minimist = require('minimist');
Copy link
Member

Choose a reason for hiding this comment

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

Could we use https://www.npmjs.com/package/yargs-parser instead? It's already in our dependency tree as part of meow.

(Though it looks like so is minimist, because Node.js modules, but for less apparent reasons.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I choose minimist, because it has reverse package, maybe there is another package for it?

Copy link
Member

Choose a reason for hiding this comment

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

I reckon dargs may work with yargs-parser too, assuming it also outputs straight-forward objects.

const dargs = require('dargs');
const globs = require('./globs');
const RunStatus = require('./run-status');
const fork = require('./fork');
Expand Down Expand Up @@ -320,28 +323,28 @@ class Api extends Emittery {
}

async _computeForkExecArgv() {
const execArgv = this.options.testOnlyExecArgv || process.execArgv;
if (execArgv.length === 0) {
return Promise.resolve(execArgv);
}
const {nodeArguments: configExecArgs} = this.options;

// --inspect-brk is used in addition to --inspect to break on first line and wait
const inspectArgIndex = execArgv.findIndex(arg => /^--inspect(-brk)?($|=)/.test(arg));
if (inspectArgIndex === -1) {
return Promise.resolve(execArgv);
}
const mainProcessArgs = minimist(process.execArgv);
Copy link
Member

Choose a reason for hiding this comment

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

Could we cache this? This function runs for every test file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure, what I correctly understand you, check please


const port = await getPort();
const forkExecArgv = execArgv.slice();
let flagName = '--inspect';
const oldValue = forkExecArgv[inspectArgIndex];
if (oldValue.includes('brk')) {
flagName += '-brk';
}
const args = {
...mainProcessArgs,
...configExecArgs
};

forkExecArgv[inspectArgIndex] = `${flagName}=${port}`;
const hasInspect = args.inspect || args['inspect-brk'];
if (hasInspect) {
if (args.inspect && args['inspect-brk']) {
throw new Error('Flags inspect and inspect-brk provided simultaneously');
Copy link
Member

Choose a reason for hiding this comment

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

Note to self: check how this ends up being reported.

}

const port = await getPort();

const flagName = args.inspect ? 'inspect' : 'inspect-brk';
args[flagName] = port;
}

return forkExecArgv;
return dargs(args);
}
}

Expand Down
14 changes: 14 additions & 0 deletions lib/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ exports.run = async () => { // eslint-disable-line complexity
--color Force color output
--no-color Disable color output
--reset-cache Reset AVA's compilation cache and exit
--node-arguments Configure Node.js arguments used to launch worker processes (For example --node-arguments="--frozen-intrinsics --experimental-modules")
--config JavaScript file for AVA to read its config from, instead of using package.json
or ava.config.js files

Expand Down Expand Up @@ -115,6 +116,10 @@ exports.run = async () => { // eslint-disable-line complexity
type: 'boolean',
default: false
},
'node-arguments': {
type: 'string',
default: ''
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove this default? See my comment for normalizeNodeArguments().

},
'--': {
type: 'string'
}
Expand Down Expand Up @@ -180,6 +185,7 @@ exports.run = async () => { // eslint-disable-line complexity
const Watcher = require('./watcher');
const babelManager = require('./babel-manager');
const normalizeExtensions = require('./extensions');
const normalizeNodeArguments = require('./node-arguments');
const {normalizeGlobs} = require('./globs');
const validateEnvironmentVariables = require('./environment-variables');

Expand Down Expand Up @@ -214,6 +220,13 @@ exports.run = async () => { // eslint-disable-line complexity
exit(error.message);
}

let nodeArguments;
try {
nodeArguments = normalizeNodeArguments(conf.nodeArguments, cli.flags.nodeArguments);
} catch (error) {
exit(error.message);
}

// Copy resultant cli.flags into conf for use with Api and elsewhere
Object.assign(conf, cli.flags);

Expand Down Expand Up @@ -265,6 +278,7 @@ exports.run = async () => { // eslint-disable-line complexity
snapshotDir: conf.snapshotDir ? path.resolve(projectDir, conf.snapshotDir) : null,
timeout: conf.timeout,
updateSnapshots: conf.updateSnapshots,
nodeArguments,
workerArgv: cli.flags['--']
});

Expand Down
18 changes: 18 additions & 0 deletions lib/node-arguments.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
'use strict';

const minimist = require('minimist');
const omit = require('lodash/omit');

/**
* @param {string[]} confParams
* @param {string} cliParams
* @return {object}
*/
function normalizeNodeArguments(confParams, cliParams) {
Copy link
Member

Choose a reason for hiding this comment

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

If we can assume the arguments may be undefined, we can use default parameter values:

Suggested change
function normalizeNodeArguments(confParams, cliParams) {
function normalizeNodeArguments(confParams = [], cliParams = '') {

And we can get rid of the JSDoc comment too.

We do need to enforce confParams is an array.

return omit({
...minimist(confParams || []),
...minimist(cliParams.split(' '))
Copy link
Member

Choose a reason for hiding this comment

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

Are there Node.js arguments that take strings? Cause then splitting on the empty string won't work.

}, '_');
Copy link
Member

Choose a reason for hiding this comment

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

Is the _ returned by minimist? I'd prefer rest-spread for this:

const { _, ...normalized } = { ...minimist(), ...minimist() };
return normalized;

}

module.exports = normalizeNodeArguments;
26 changes: 23 additions & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
"concordance": "^4.0.0",
"convert-source-map": "^1.6.0",
"currently-unhandled": "^0.4.1",
"dargs": "^7.0.0",
"debug": "^4.1.1",
"del": "^5.1.0",
"dot-prop": "^5.2.0",
Expand All @@ -106,6 +107,7 @@
"md5-hex": "^3.0.1",
"meow": "^5.0.0",
"micromatch": "^4.0.2",
"minimist": "^1.2.0",
"ms": "^2.1.2",
"observable-to-promise": "^1.0.0",
"ora": "^4.0.2",
Expand Down
6 changes: 4 additions & 2 deletions test/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const assert = require('assert');
const path = require('path');
const fs = require('fs');
const del = require('del');
const minimist = require('minimist');
const {test} = require('tap');
const Api = require('../lib/api');
const babelManager = require('../lib/babel-manager');
Expand Down Expand Up @@ -37,6 +38,7 @@ function apiCreator(options = {}) {
options.experiments = {};
options.globs = normalizeGlobs(options.files, options.helpers, options.sources, options.extensions.all);
options.resolveTestsFrom = options.resolveTestsFrom || options.projectDir;
options.nodeArguments = options.nodeArguments || {};
const instance = new Api(options);
if (!options.precompileHelpers) {
instance._precompileHelpers = () => Promise.resolve();
Expand Down Expand Up @@ -1179,7 +1181,7 @@ test('using --match with matching tests will only report those passing tests', t

function generatePassDebugTests(execArgv) {
test(`pass ${execArgv.join(' ')} to fork`, t => {
const api = apiCreator({testOnlyExecArgv: execArgv});
const api = apiCreator({nodeArguments: minimist(execArgv)});
return api._computeForkExecArgv()
.then(result => {
t.true(result.length === execArgv.length);
Expand All @@ -1190,7 +1192,7 @@ function generatePassDebugTests(execArgv) {

function generatePassInspectIntegrationTests(execArgv) {
test(`pass ${execArgv.join(' ')} to fork`, t => {
const api = apiCreator({testOnlyExecArgv: execArgv});
const api = apiCreator({nodeArguments: minimist(execArgv)});
return api.run([path.join(__dirname, 'fixture/inspect-arg.js')])
.then(runStatus => {
t.is(runStatus.stats.passedTests, 1);
Expand Down
7 changes: 7 additions & 0 deletions test/fixture/node-arguments.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import test from '../..';

test('exec arguments includes --throw-deprecation and --zero-fill-buffers', t => {
t.plan(2);
t.truthy(process.execArgv.includes('--throw-deprecation'));
t.truthy(process.execArgv.includes('--zero-fill-buffers'));
});
8 changes: 8 additions & 0 deletions test/fixture/node-arguments/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"ava": {
"nodeArguments": [
"--require",
"./setup.js"
]
}
}
1 change: 1 addition & 0 deletions test/fixture/node-arguments/setup.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
global.SETUP_CALLED = true;
7 changes: 7 additions & 0 deletions test/fixture/node-arguments/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import test from '../../..';

test('works', t => {
t.plan(2);
t.truthy(global.SETUP_CALLED);
t.truthy(process.execArgv.includes('--require'));
});
3 changes: 2 additions & 1 deletion test/helper/report.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ const run = (type, reporter, match = []) => {
concurrency: 1,
updateSnapshots: false,
snapshotDir: false,
color: true
color: true,
nodeArguments: {}
};
let pattern = '*.js';

Expand Down
15 changes: 15 additions & 0 deletions test/integration/node-arguments.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
'use strict';
const {test} = require('tap');
const {execCli} = require('../helper/cli');

test('passes node arguments to workers', t => {
t.plan(1);
execCli(['--node-arguments="--throw-deprecation --zero-fill-buffers"', 'node-arguments.js'], err => t.ifError(err));
});

test('reads node arguments from config', t => {
t.plan(1);
execCli(['test.js'], {
dirname: 'fixture/node-arguments'
}, err => t.ifError(err));
});
27 changes: 27 additions & 0 deletions test/node-arguments.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
'use strict';

const {test} = require('tap');
const normalizeNodeArguments = require('../lib/node-arguments');

test('normalizes multiple node arguments from cli', t => {
t.deepEqual(normalizeNodeArguments([], '--a --b --c'), {a: true, b: true, c: true});
t.end();
});

test('normalizes multiple node arguments from config', t => {
t.deepEqual(normalizeNodeArguments(['--arg1', '--b'], ''), {arg1: true, b: true});
t.end();
});

test('normalizes node arguments from config and cli', t => {
t.deepEqual(
normalizeNodeArguments(['--arg1', '--b=2'], '--arg2 --b=12'),
{arg1: true, arg2: true, b: 12}
);
t.end();
});

test('normalizes single node arguments from cli', t => {
t.deepEqual(normalizeNodeArguments([], '--test-flag'), {'test-flag': true});
t.end();
});