Skip to content

Commit

Permalink
fix(core): prefer NX_TASKS_RUNNER over NX_RUNNER (#16618)
Browse files Browse the repository at this point in the history
(cherry picked from commit 22c0047)
  • Loading branch information
AgentEnder authored and FrozenPandaz committed Apr 28, 2023
1 parent 4554f65 commit 58d089c
Show file tree
Hide file tree
Showing 3 changed files with 241 additions and 94 deletions.
3 changes: 2 additions & 1 deletion docs/shared/reference/environment-variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ The following environment variables are ones that you can set to change the beha
| NX_PROFILE | string | Prepend `NX_PROFILE=profile.json` before running targets with Nx to generate a file that be [loaded in Chrome dev tools](/recipes/other/performance-profiling) to visualize the performance of Nx across multiple processes. |
| NX_PROJECT_GRAPH_CACHE_DIRECTORY | string | The project graph cache is stored in `node_modules/.cache/nx` by default. Set this variable to use a different directory. |
| NX_PROJECT_GRAPH_MAX_WORKERS | number | The number of workers to use when calculating the project graph. |
| NX_RUNNER | string | The name of task runner from the config to use. Can be overridden on the command line with `--runner`. |
| NX_RUNNER | string | The name of task runner from the config to use. Can be overridden on the command line with `--runner`. Not read if NX_TASKS_RUNNER is set. |
| NX_SKIP_NX_CACHE | boolean | Rerun the tasks even when the results are available in the cache |
| NX_TASKS_RUNNER | string | The name of task runner from the config to use. Can be overridden on the command line with `--runner`. Preferred over NX_RUNNER. |
| NX_TASKS_RUNNER_DYNAMIC_OUTPUT | boolean | If set to `false`, will use non-dynamic terminal output strategy (what you see in CI), even when you terminal can support the dynamic version |
| NX_VERBOSE_LOGGING | boolean | If set to `true`, will print debug information useful for troubleshooting |
| NX_DRY_RUN | boolean | If set to `true`, will perform a dry run of the generator. No files will be created and no packages will be installed. |
Expand Down
269 changes: 186 additions & 83 deletions packages/nx/src/utils/command-line-utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,100 +185,191 @@ describe('splitArgs', () => {
});

it('should set base and head based on environment variables in affected mode, if they are not provided directly on the command', () => {
const originalNxBase = process.env.NX_BASE;
process.env.NX_BASE = 'envVarSha1';
const originalNxHead = process.env.NX_HEAD;
process.env.NX_HEAD = 'envVarSha2';
withEnvironment(
{
NX_BASE: 'envVarSha1',
NX_HEAD: 'envVarSha2',
},
() => {
expect(
splitArgsIntoNxArgsAndOverrides(
{
__overrides_unparsed__: ['--notNxArg', 'true', '--override'],
$0: '',
},
'affected',
{} as any,
{} as any
).nxArgs
).toEqual({
base: 'envVarSha1',
head: 'envVarSha2',
skipNxCache: false,
parallel: 3,
});

expect(
splitArgsIntoNxArgsAndOverrides(
{
__overrides_unparsed__: ['--notNxArg', 'true', '--override'],
$0: '',
head: 'directlyOnCommandSha1', // higher priority than $NX_HEAD
},
'affected',
{} as any,
{} as any
).nxArgs
).toEqual({
base: 'envVarSha1',
head: 'directlyOnCommandSha1',
skipNxCache: false,
parallel: 3,
});

expect(
splitArgsIntoNxArgsAndOverrides(
{
__overrides_unparsed__: ['--notNxArg', 'true', '--override'],
$0: '',
base: 'directlyOnCommandSha2', // higher priority than $NX_BASE
},
'affected',
{} as any,
{} as any
).nxArgs
).toEqual({
base: 'directlyOnCommandSha2',
head: 'envVarSha2',
skipNxCache: false,
parallel: 3,
});
}
);
});

expect(
splitArgsIntoNxArgsAndOverrides(
{
__overrides_unparsed__: ['--notNxArg', 'true', '--override'],
$0: '',
},
'affected',
{} as any,
{} as any
).nxArgs
).toEqual({
base: 'envVarSha1',
head: 'envVarSha2',
skipNxCache: false,
parallel: 3,
describe('--runner environment handling', () => {
it('should set runner based on environment NX_RUNNER, if it is not provided directly on the command', () => {
withEnvironment({ NX_RUNNER: 'some-env-runner-name' }, () => {
expect(
splitArgsIntoNxArgsAndOverrides(
{
__overrides_unparsed__: ['--notNxArg', 'true', '--override'],
$0: '',
},
'run-one',
{} as any,
{
tasksRunnerOptions: {
'some-env-runner-name': { runner: '' },
},
}
).nxArgs.runner
).toEqual('some-env-runner-name');

expect(
splitArgsIntoNxArgsAndOverrides(
{
__overrides_unparsed__: ['--notNxArg', 'true', '--override'],
$0: '',
runner: 'directlyOnCommand', // higher priority than $NX_RUNNER
},
'run-one',
{} as any,
{
tasksRunnerOptions: {
'some-env-runner-name': { runner: '' },
},
}
).nxArgs.runner
).toEqual('directlyOnCommand');
});
});

expect(
splitArgsIntoNxArgsAndOverrides(
{
__overrides_unparsed__: ['--notNxArg', 'true', '--override'],
$0: '',
head: 'directlyOnCommandSha1', // higher priority than $NX_HEAD
},
'affected',
{} as any,
{} as any
).nxArgs
).toEqual({
base: 'envVarSha1',
head: 'directlyOnCommandSha1',
skipNxCache: false,
parallel: 3,
it('should set runner based on environment NX_TASKS_RUNNER, if it is not provided directly on the command', () => {
withEnvironment({ NX_TASKS_RUNNER: 'some-env-runner-name' }, () => {
expect(
splitArgsIntoNxArgsAndOverrides(
{
__overrides_unparsed__: ['--notNxArg', 'true', '--override'],
$0: '',
},
'run-one',
{} as any,
{
tasksRunnerOptions: {
'some-env-runner-name': { runner: '' },
},
}
).nxArgs.runner
).toEqual('some-env-runner-name');

expect(
splitArgsIntoNxArgsAndOverrides(
{
__overrides_unparsed__: ['--notNxArg', 'true', '--override'],
$0: '',
runner: 'directlyOnCommand', // higher priority than $NX_RUNNER
},
'run-one',
{} as any,
{
tasksRunnerOptions: {
'some-env-runner-name': { runner: '' },
},
}
).nxArgs.runner
).toEqual('directlyOnCommand');
});
});

expect(
splitArgsIntoNxArgsAndOverrides(
it('should prefer NX_TASKS_RUNNER', () => {
withEnvironment(
{
__overrides_unparsed__: ['--notNxArg', 'true', '--override'],
$0: '',
base: 'directlyOnCommandSha2', // higher priority than $NX_BASE
NX_TASKS_RUNNER: 'some-env-runner-name',
NX_RUNNER: 'some-other-runner',
},
'affected',
{} as any,
{} as any
).nxArgs
).toEqual({
base: 'directlyOnCommandSha2',
head: 'envVarSha2',
skipNxCache: false,
parallel: 3,
() => {
expect(
splitArgsIntoNxArgsAndOverrides(
{
__overrides_unparsed__: ['--notNxArg', 'true', '--override'],
$0: '',
},
'run-one',
{} as any,
{
tasksRunnerOptions: {
'some-env-runner-name': { runner: '' },
'some-other-runner': { runner: '' },
},
}
).nxArgs.runner
).toEqual('some-env-runner-name');
}
);
});

// Reset process data
process.env.NX_BASE = originalNxBase;
process.env.NX_HEAD = originalNxHead;
});

it('should set runner based on environment variables, if it is not provided directly on the command', () => {
const originalRunner = process.env.NX_RUNNER;
process.env.NX_RUNNER = 'some-env-runner-name';

expect(
splitArgsIntoNxArgsAndOverrides(
{
__overrides_unparsed__: ['--notNxArg', 'true', '--override'],
$0: '',
},
'run-one',
{} as any,
{} as any
).nxArgs.runner
).toEqual('some-env-runner-name');

expect(
splitArgsIntoNxArgsAndOverrides(
it('should ignore runners based on environment, if it is valid', () => {
withEnvironment(
{
__overrides_unparsed__: ['--notNxArg', 'true', '--override'],
$0: '',
runner: 'directlyOnCommand', // higher priority than $NX_RUNNER
NX_TASKS_RUNNER: 'some-env-runner-name',
NX_RUNNER: 'some-other-runner',
},
'run-one',
{} as any,
{} as any
).nxArgs.runner
).toEqual('directlyOnCommand');

// Reset process data
process.env.NX_RUNNER = originalRunner;
() => {
expect(
splitArgsIntoNxArgsAndOverrides(
{
__overrides_unparsed__: ['--notNxArg', 'true', '--override'],
$0: '',
},
'run-one',
{} as any,
{} as any
).nxArgs.runner
).not.toBeDefined();
}
);
});
});

describe('--parallel', () => {
Expand Down Expand Up @@ -389,3 +480,15 @@ describe('splitArgs', () => {
});
});
});

function withEnvironment(env: Record<string, string>, callback: () => void) {
const originalValues: Record<string, string> = {};
for (const key in env) {
originalValues[key] = process.env[key];
process.env[key] = env[key];
}
callback();
for (const key in env) {
process.env[key] = originalValues[key];
}
}
63 changes: 53 additions & 10 deletions packages/nx/src/utils/command-line-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,16 +181,7 @@ export function splitArgsIntoNxArgsAndOverrides(
nxArgs.skipNxCache = process.env.NX_SKIP_NX_CACHE === 'true';
}

if (!nxArgs.runner && process.env.NX_RUNNER) {
nxArgs.runner = process.env.NX_RUNNER;
if (options.printWarnings) {
output.note({
title: `No explicit --runner argument provided, but found environment variable NX_RUNNER so using its value: ${output.bold(
`${nxArgs.runner}`
)}`,
});
}
}
normalizeNxArgsRunner(nxArgs, nxJson, options);

if (args['parallel'] === 'false' || args['parallel'] === false) {
nxArgs['parallel'] = 1;
Expand All @@ -210,6 +201,58 @@ export function splitArgsIntoNxArgsAndOverrides(
return { nxArgs, overrides } as any;
}

function normalizeNxArgsRunner(
nxArgs: RawNxArgs,
nxJson: NxJsonConfiguration<string[] | '*'>,
options: { printWarnings: boolean }
) {
if (!nxArgs.runner) {
// TODO: Remove NX_RUNNER environment variable support in Nx v17
for (const envKey of ['NX_TASKS_RUNNER', 'NX_RUNNER']) {
const runner = process.env[envKey];
if (runner) {
const runnerExists = nxJson.tasksRunnerOptions?.[runner];
if (options.printWarnings) {
if (runnerExists) {
output.note({
title: `No explicit --runner argument provided, but found environment variable ${envKey} so using its value: ${output.bold(
`${runner}`
)}`,
});
} else if (
nxArgs.verbose ||
process.env.NX_VERBOSE_LOGGING === 'true'
) {
output.warn({
title: `Could not find ${output.bold(
`${runner}`
)} within \`nx.json\` tasksRunnerOptions.`,
bodyLines: [
`${output.bold(`${runner}`)} was set by ${envKey}`,
``,
`To suppress this message, either:`,
` - provide a valid task runner with --runner`,
` - ensure NX_TASKS_RUNNER matches a task runner defined in nx.json`,
],
});
}
}
if (runnerExists) {
// TODO: Remove in v17
if (envKey === 'NX_RUNNER' && options.printWarnings) {
output.warn({
title:
'NX_RUNNER is deprecated, please use NX_TASKS_RUNNER instead.',
});
}
nxArgs.runner = runner;
}
break;
}
}
}
}

export function parseFiles(options: NxArgs): { files: string[] } {
const { files, uncommitted, untracked, base, head } = options;

Expand Down

0 comments on commit 58d089c

Please sign in to comment.