From 58d089ced02ab81031df6bdc9be0eb39b2374b90 Mon Sep 17 00:00:00 2001 From: Craigory Coppola Date: Fri, 28 Apr 2023 16:50:48 -0400 Subject: [PATCH] fix(core): prefer NX_TASKS_RUNNER over NX_RUNNER (#16618) (cherry picked from commit 22c0047b63813f813706e9501babf38ad4017a9a) --- .../shared/reference/environment-variables.md | 3 +- .../nx/src/utils/command-line-utils.spec.ts | 269 ++++++++++++------ packages/nx/src/utils/command-line-utils.ts | 63 +++- 3 files changed, 241 insertions(+), 94 deletions(-) diff --git a/docs/shared/reference/environment-variables.md b/docs/shared/reference/environment-variables.md index c0db0e8bec46a..7321ac424153a 100644 --- a/docs/shared/reference/environment-variables.md +++ b/docs/shared/reference/environment-variables.md @@ -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. | diff --git a/packages/nx/src/utils/command-line-utils.spec.ts b/packages/nx/src/utils/command-line-utils.spec.ts index f234e71033a3b..0f72ce5e0cfb4 100644 --- a/packages/nx/src/utils/command-line-utils.spec.ts +++ b/packages/nx/src/utils/command-line-utils.spec.ts @@ -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', () => { @@ -389,3 +480,15 @@ describe('splitArgs', () => { }); }); }); + +function withEnvironment(env: Record, callback: () => void) { + const originalValues: Record = {}; + 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]; + } +} diff --git a/packages/nx/src/utils/command-line-utils.ts b/packages/nx/src/utils/command-line-utils.ts index b7dce0df0374f..7728cf7970a65 100644 --- a/packages/nx/src/utils/command-line-utils.ts +++ b/packages/nx/src/utils/command-line-utils.ts @@ -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; @@ -210,6 +201,58 @@ export function splitArgsIntoNxArgsAndOverrides( return { nxArgs, overrides } as any; } +function normalizeNxArgsRunner( + nxArgs: RawNxArgs, + nxJson: NxJsonConfiguration, + 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;