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.

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

Add filterNodeArgumentsForWorkerThreads option #3336

Merged
merged 14 commits into from Aug 20, 2024
16 changes: 16 additions & 0 deletions docs/06-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -337,3 +337,19 @@ These may also export a function which is then invoked, and can receive argument
The `nodeArguments` configuration may be used to specify additional arguments for launching worker processes. These are combined with `--node-arguments` passed on the CLI and any arguments passed to the `node` binary when starting AVA.

[CLI]: ./05-command-line.md

## Thread arguments filter

In a config file only, `threadArgumentsFilter` may provide a function used for filtering `nodeArguments` sent to worker threads. This enables excluding arguments that throw if sent to a thread. The filter is ignored by worker processes.

`ava.config.js`:
```js
This conversation was marked as resolved.
Show resolved Hide resolved
const processOnly = new Set([
'--allow-natives-syntax',
'--expose-gc'
]);

export default {
threadArgumentsFilter: argument => !processOnly.has(argument)
This conversation was marked as resolved.
Show resolved Hide resolved
}
```
7 changes: 7 additions & 0 deletions lib/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -388,9 +388,16 @@ export default async function loadCli() { // eslint-disable-line complexity
exit(error.message);
}

const workerThreads = combined.workerThreads !== false;

let nodeArguments;
try {
nodeArguments = normalizeNodeArguments(conf.nodeArguments, argv['node-arguments']);
nodeArguments ??= process.execArgv;
This conversation was marked as resolved.
Show resolved Hide resolved
if (workerThreads && 'threadArgumentsFilter' in conf) {
const {threadArgumentsFilter: filter} = conf;
nodeArguments = nodeArguments.filter(argument => filter(argument));
}
} catch (error) {
exit(error.message);
}
Expand Down
2 changes: 1 addition & 1 deletion lib/fork.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ const createWorker = (options, execArgv) => {
};
};

export default function loadFork(file, options, execArgv = process.execArgv) {
export default function loadFork(file, options, execArgv) {
Copy link
Member

Choose a reason for hiding this comment

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

This default assignment is still required.

let finished = false;

const emitter = new Emittery();
Expand Down
4 changes: 4 additions & 0 deletions lib/load-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@ export async function loadConfig({configFile, resolveFrom = process.cwd(), defau
...defaults, nonSemVerExperiments: {}, ...fileConf, ...packageConf, projectDir, configFile,
};

if ('threadArgumentsFilter' in config && typeof config.threadArgumentsFilter !== 'function') {
throw new Error(`threadArgumentsFilter from ${fileForErrorMessage} must be a function`);
}

const {nonSemVerExperiments: experiments} = config;
if (!isPlainObject(experiments)) {
throw new Error(`nonSemVerExperiments from ${fileForErrorMessage} must be an object`);
Expand Down
2 changes: 2 additions & 0 deletions test-tap/api.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import fs from 'node:fs';
import path from 'node:path';
import process from 'node:process';
import {fileURLToPath} from 'node:url';

import ciInfo from 'ci-info';
Expand All @@ -20,6 +21,7 @@ async function apiCreator(options = {}) {
options.globs = normalizeGlobs({
files: options.files, ignoredByWatcher: options.watchMode?.ignoreChanges, extensions: options.extensions, providers: [],
});
options.nodeArguments = process.execArgv;
const instance = new Api(options);

return instance;
Expand Down
3 changes: 3 additions & 0 deletions test-tap/fixture/load-config/non-function/ava.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default {
threadArgumentsFilter: [],
};
3 changes: 3 additions & 0 deletions test-tap/fixture/load-config/non-function/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"type": "module"
}
2 changes: 2 additions & 0 deletions test/config/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ test.serial('throws an error if a .js config file has no default export', notOk(

test.serial('throws an error if a config file contains `ava` property', notOk('contains-ava-property'));

test.serial('throws an error if a config file contains a non-function `threadArgumentsFilter` property', notOk('non-function'));

test.serial('throws an error if a config file contains a non-object `nonSemVerExperiments` property', notOk('non-object-experiments'));

test.serial('throws an error if a config file enables an unsupported experiment', notOk('unsupported-experiments'));
Expand Down
6 changes: 6 additions & 0 deletions test/config/snapshots/loader.js.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ Generated by [AVA](https://avajs.dev).

'Encountered ’ava’ property in ava.config.js; avoid wrapping the configuration'

## throws an error if a config file contains a non-function `threadArgumentsFilter` property

> error message

'threadArgumentsFilter from ava.config.js must be a function'

## throws an error if a config file contains a non-object `nonSemVerExperiments` property

> error message
Expand Down
Binary file modified test/config/snapshots/loader.js.snap
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"type": "module"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import test from 'ava';

test('exec arguments unfiltered', t => {
t.plan(2);
t.truthy(process.execArgv.includes('--throw-deprecation'));
t.truthy(process.execArgv.includes('--allow-natives-syntax'));
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
const processOnly = new Set(['--allow-natives-syntax']);

export default {
nodeArguments: [
'--throw-deprecation',
'--allow-natives-syntax',
],
threadArgumentsFilter: argument => !processOnly.has(argument),
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import test from 'ava';

test('exec arguments filtered', t => {
t.plan(2);
t.truthy(process.execArgv.includes('--throw-deprecation'));
t.falsy(process.execArgv.includes('--allow-natives-syntax'));
});
22 changes: 22 additions & 0 deletions test/node-arguments/snapshots/test.js.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,28 @@ Generated by [AVA](https://avajs.dev).
},
]

## `threadArgumentsFilter` configuration filters arguments for worker thread

> tests pass

[
{
file: 'thread.js',
title: 'exec arguments filtered',
},
]

## `threadArgumentsFilter` configuration ignored for worker process

> tests pass

[
{
file: 'process.js',
title: 'exec arguments unfiltered',
},
]

## detects incomplete --node-arguments

> fails with message
Expand Down
Binary file modified test/node-arguments/snapshots/test.js.snap
Binary file not shown.
20 changes: 20 additions & 0 deletions test/node-arguments/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,26 @@ test('passed node arguments to workers', async t => {
t.snapshot(result.stats.passed, 'tests pass');
});

test('`threadArgumentsFilter` configuration filters arguments for worker thread', async t => {
const options = {
cwd: cwd('thread-arguments-filter'),
};

const result = await fixture(['--config=thread-arguments-filter.config.mjs', 'thread.js'], options);

t.snapshot(result.stats.passed, 'tests pass');
});

test('`threadArgumentsFilter` configuration ignored for worker process', async t => {
const options = {
cwd: cwd('thread-arguments-filter'),
};

const result = await fixture(['--config=thread-arguments-filter.config.mjs', '--no-worker-threads', 'process.js'], options);

t.snapshot(result.stats.passed, 'tests pass');
});

test('detects incomplete --node-arguments', async t => {
const options = {
cwd: cwd('node-arguments'),
Expand Down
Loading