From 9221e4a9b9e4c887693c81ed5181fdda02b78cab Mon Sep 17 00:00:00 2001 From: Emily Xiong Date: Wed, 10 Apr 2024 03:18:56 -0400 Subject: [PATCH] fix(core): should not pass --no-color --no-parallel --- e2e/nx/src/run.test.ts | 17 +++ .../run-commands/run-commands.impl.spec.ts | 134 ++++++++++++++++-- .../run-commands/run-commands.impl.ts | 38 ++--- 3 files changed, 160 insertions(+), 29 deletions(-) diff --git a/e2e/nx/src/run.test.ts b/e2e/nx/src/run.test.ts index 0518850a1b2be..7d07f8c393a97 100644 --- a/e2e/nx/src/run.test.ts +++ b/e2e/nx/src/run.test.ts @@ -86,6 +86,23 @@ describe('Nx Running Tests', () => { const output = runCLI(`echo ${proj} ${args}`); expect(output).toContain(`ECHO: ${result}`); }); + + it.each([ + { + args: '-- a b c --a --a.b=1 --no-color --no-parallel', + result: 'ECHO: a b c --a --a.b=1', + }, + { + args: '-- a b c --a --a.b=1 --color --parallel', + result: 'ECHO: a b c --a --a.b=1', + }, + ])( + 'should not forward --color --parallel for $args', + ({ args, result }) => { + const output = runCLI(`echo ${proj} ${args}`); + expect(output).toContain(result); + } + ); }); it('should execute long running tasks', () => { diff --git a/packages/nx/src/executors/run-commands/run-commands.impl.spec.ts b/packages/nx/src/executors/run-commands/run-commands.impl.spec.ts index aa2192f087380..547c56067854a 100644 --- a/packages/nx/src/executors/run-commands/run-commands.impl.spec.ts +++ b/packages/nx/src/executors/run-commands/run-commands.impl.spec.ts @@ -36,16 +36,85 @@ describe('Run Commands', () => { expect(readFile(f)).toEqual('123'); }); - it('should not pass --args into underlying command', async () => { - const result = await runCommands( - { - command: `echo`, - __unparsed__: ['--args=--key=123'], - args: '--key=123', - }, - context - ); - expect(result.terminalOutput.trim()).not.toContain('--args=--key=123'); + it.each([ + { + unparsed: ['test1', '--args=--key=123', '--test2=1', '--test2=2'], + expected: 'test1 --test2=1 --test2=2', + }, + { + unparsed: ['test', '--args=--key=123', '--test.a=1', '--test.b=2'], + expected: 'test --test.a=1 --test.b=2', + }, + { unparsed: ['one', '-a=b', `--args=--key=123`], expected: 'one -a=b' }, + ])( + 'should pass command line args $unparsed to the command and ignore --args', + async ({ unparsed: unparsedOptions, expected }) => { + let result = ( + await runCommands( + { + command: `echo`, + __unparsed__: unparsedOptions, + args: '--key=123', + }, + context + ) + ).terminalOutput.trim(); + expect(result).not.toContain('--args=--key=123'); + expect(result).toContain(`echo --key=123 ${expected}`); + } + ); + + it('should overwrite options with args', async () => { + let result = ( + await runCommands( + { + command: `echo`, + __unparsed__: [], + key: 789, + }, + context + ) + ).terminalOutput.trim(); + expect(result).toContain('echo --key=789'); // unknown options + + result = ( + await runCommands( + { + command: `echo`, + __unparsed__: ['--a.b=234'], + a: { b: 123 }, + }, + context + ) + ).terminalOutput.trim(); + expect(result).toContain('echo --a.b=234'); + + result = ( + await runCommands( + { + command: `echo`, + __unparsed__: ['--key=456'], + key: 123, + }, + context + ) + ).terminalOutput.trim(); + expect(result).not.toContain('--key=123'); + expect(result).toContain('echo --key=456'); // should take unparsed over unknown options + + result = ( + await runCommands( + { + command: `echo`, + __unparsed__: ['--key=456'], + key: 123, + args: '--key=789', + }, + context + ) + ).terminalOutput.trim(); + expect(result).not.toContain('--key=123'); + expect(result).toContain('--key=789'); // should take args over unknown options }); it('should not foward any args to underlying command if forwardAllArgs is false', async () => { @@ -128,7 +197,7 @@ describe('Run Commands', () => { it('should run commands serially', async () => { const f = fileSync().name; - const result = await runCommands( + let result = await runCommands( { commands: [`sleep 0.2 && echo 1 >> ${f}`, `echo 2 >> ${f}`], parallel: false, @@ -138,6 +207,16 @@ describe('Run Commands', () => { ); expect(result).toEqual(expect.objectContaining({ success: true })); expect(readFile(f)).toEqual('12'); + + result = await runCommands( + { + commands: [`sleep 0.2 && echo 1 >> ${f}`, `echo 2 >> ${f}`], + __unparsed__: ['--no-parallel'], + }, + context + ); + expect(result).toEqual(expect.objectContaining({ success: true })); + expect(readFile(f)).toEqual('1212'); }); it('should run commands in parallel', async () => { @@ -238,11 +317,11 @@ describe('Run Commands', () => { 'echo', { __unparsed__: ['--args', 'test', 'hello'], - unparsedCommandArgs: { args: 'test' }, + parsedArgs: { args: 'test' }, } as any, true ) - ).toEqual('echo hello'); + ).toEqual('echo hello'); // should not pass --args test to underlying command expect( interpolateArgsIntoCommand( @@ -404,6 +483,35 @@ describe('Run Commands', () => { }); }); + it('should not set FORCE_COLOR=true when --no-color is passed', async () => { + const exec = jest.spyOn(require('child_process'), 'exec'); + await runCommands( + { + commands: [`echo 'Hello World'`, `echo 'Hello Universe'`], + parallel: true, + __unparsed__: [], + color: false, + }, + context + ); + + expect(exec).toHaveBeenCalledTimes(2); + expect(exec).toHaveBeenNthCalledWith(1, `echo 'Hello World'`, { + maxBuffer: LARGE_BUFFER, + env: { + ...process.env, + ...env(), + }, + }); + expect(exec).toHaveBeenNthCalledWith(2, `echo 'Hello Universe'`, { + maxBuffer: LARGE_BUFFER, + env: { + ...process.env, + ...env(), + }, + }); + }); + it('should set FORCE_COLOR=true when running with --color', async () => { const exec = jest.spyOn(require('child_process'), 'exec'); await runCommands( diff --git a/packages/nx/src/executors/run-commands/run-commands.impl.ts b/packages/nx/src/executors/run-commands/run-commands.impl.ts index af8c4c8abc609..37827c469c97e 100644 --- a/packages/nx/src/executors/run-commands/run-commands.impl.ts +++ b/packages/nx/src/executors/run-commands/run-commands.impl.ts @@ -67,7 +67,9 @@ const propKeys = [ 'command', 'commands', 'color', + 'no-color', 'parallel', + 'no-parallel', 'readyWhen', 'cwd', 'args', @@ -93,7 +95,7 @@ export interface NormalizedRunCommandsOptions extends RunCommandsOptions { [k: string]: any; }; unparsedCommandArgs?: { - [k: string]: string; + [k: string]: string | string[]; }; args?: string; } @@ -232,6 +234,7 @@ function normalizeOptions( 'parse-numbers': false, 'parse-positional-numbers': false, 'dot-notation': false, + 'camel-case-expansion': false, }, }); options.unknownOptions = Object.keys(options) @@ -485,17 +488,18 @@ export function interpolateArgsIntoCommand( } else if (forwardAllArgs) { let args = ''; if (Object.keys(opts.unknownOptions ?? {}).length > 0) { - args += - ' ' + - Object.keys(opts.unknownOptions) - .filter( - (k) => - typeof opts.unknownOptions[k] !== 'object' && - opts.parsedArgs[k] === opts.unknownOptions[k] - ) - .map((k) => `--${k}=${opts.unknownOptions[k]}`) - .map(wrapArgIntoQuotesIfNeeded) - .join(' '); + const unknownOptionsArgs = Object.keys(opts.unknownOptions) + .filter( + (k) => + typeof opts.unknownOptions[k] !== 'object' && + opts.parsedArgs[k] === opts.unknownOptions[k] + ) + .map((k) => `--${k}=${opts.unknownOptions[k]}`) + .map(wrapArgIntoQuotesIfNeeded) + .join(' '); + if (unknownOptionsArgs) { + args += ` ${unknownOptionsArgs}`; + } } if (opts.args) { args += ` ${opts.args}`; @@ -503,7 +507,7 @@ export function interpolateArgsIntoCommand( if (opts.__unparsed__?.length > 0) { const filterdParsedOptions = filterPropKeysFromUnParsedOptions( opts.__unparsed__, - opts.unparsedCommandArgs + opts.parsedArgs ); if (filterdParsedOptions.length > 0) { args += ` ${filterdParsedOptions @@ -538,8 +542,8 @@ function parseArgs( */ function filterPropKeysFromUnParsedOptions( __unparsed__: string[], - unparsedCommandArgs: { - [k: string]: string; + parseArgs: { + [k: string]: string | string[]; } = {} ): string[] { const parsedOptions = []; @@ -548,6 +552,7 @@ function filterPropKeysFromUnParsedOptions( if (element.startsWith('--')) { const key = element.replace('--', ''); if (element.includes('=')) { + // key can be in the format of --key=value or --key.subkey=value (e.g. env.foo=bar) if (!propKeys.includes(key.split('=')[0].split('.')[0])) { // check if the key is part of the propKeys array parsedOptions.push(element); @@ -557,7 +562,8 @@ function filterPropKeysFromUnParsedOptions( if (propKeys.includes(key)) { if ( index + 1 < __unparsed__.length && - __unparsed__[index + 1] === unparsedCommandArgs[key] + parseArgs[key] && + __unparsed__[index + 1].toString() === parseArgs[key].toString() ) { index++; // skip the next element }