Skip to content

Commit

Permalink
fix(core): should not pass --no-color --no-parallel
Browse files Browse the repository at this point in the history
  • Loading branch information
@xiongemi
xiongemi committed Apr 23, 2024
1 parent f76b8c6 commit 118fcc9
Show file tree
Hide file tree
Showing 2 changed files with 141 additions and 26 deletions.
132 changes: 119 additions & 13 deletions packages/nx/src/executors/run-commands/run-commands.impl.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,84 @@ 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,
},
context
)
).terminalOutput.trim();
expect(result).not.toContain('--args=--key=123');
expect(result).toContain(expected);
}
);

it('should overwrite options with args', async () => {
let result = (
await runCommands(
{
command: `echo`,
__unparsed__: [],
key: 789,
},
context
)
).terminalOutput.trim();
expect(result).toContain('--key=789'); // unknown options

result = (
await runCommands(
{
command: `echo`,
__unparsed__: ['--a.b=234'],
a: { b: 123 },
},
context
)
).terminalOutput.trim();
expect(result).toContain('--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('--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 () => {
Expand Down Expand Up @@ -128,7 +196,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,
Expand All @@ -138,6 +206,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 () => {
Expand Down Expand Up @@ -238,11 +316,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(
Expand Down Expand Up @@ -354,6 +432,34 @@ 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__: ['--no-color'],
},
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(
Expand Down
35 changes: 22 additions & 13 deletions packages/nx/src/executors/run-commands/run-commands.impl.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ const propKeys = [
'command',
'commands',
'color',
'no-color',
'parallel',
'no-parallel',
'readyWhen',
'cwd',
'args',
Expand All @@ -91,7 +93,7 @@ export interface NormalizedRunCommandsOptions extends RunCommandsOptions {
[k: string]: any;
};
unparsedCommandArgs?: {
[k: string]: string;
[k: string]: string | string[];
};
args?: string;
}
Expand Down Expand Up @@ -227,6 +229,7 @@ function normalizeOptions(
'parse-numbers': false,
'parse-positional-numbers': false,
'dot-notation': false,
'camel-case-expansion': false,
},
});
options.unknownOptions = Object.keys(options)
Expand Down Expand Up @@ -494,10 +497,13 @@ export function interpolateArgsIntoCommand(
if (opts.__unparsed__?.length > 0) {
const filterdParsedOptions = filterPropKeysFromUnParsedOptions(
opts.__unparsed__,
opts.unparsedCommandArgs
opts.parsedArgs
);
if (filterdParsedOptions.length > 0) {
args += ` ${filterdParsedOptions.join(' ')}`;
if (
filterdParsedOptions.length > 0 &&
!args.includes(filterdParsedOptions)
) {
args += ' ' + filterdParsedOptions;
}
}
return `${command}${args}`;
Expand All @@ -523,21 +529,23 @@ function parseArgs(
* This function filters out the prop keys from the unparsed options
* @param __unparsed__ e.g. ['--prop1', 'value1', '--prop2=value2', '--args=test']
* @param unparsedCommandArgs e.g. { prop1: 'value1', prop2: 'value2', args: 'test'}
* @returns filtered options that are not part of the propKeys array e.g. ['--prop1', 'value1', '--prop2=value2']
* @returns filtered options that are not part of the propKeys array e.g. '--prop1 value1 --args=test'
*/
function filterPropKeysFromUnParsedOptions(
__unparsed__: string[],
unparsedCommandArgs: {
[k: string]: string;
parseArgs: {
[k: string]: string | string[];
} = {}
): string[] {
): string {
const parsedOptions = [];
for (let index = 0; index < __unparsed__.length; index++) {
const element = __unparsed__[index];
if (element.startsWith('--')) {
const key = element.replace('--', '');
if (element.includes('=')) {
if (!propKeys.includes(key.split('=')[0].split('.')[0])) {
let key = element.replace('--', '');
if (key.includes('=')) {
// key can be in the format of --key=value or --key.subkey=value (e.g. env.foo=bar)
key = key.split('=')[0].split('.')[0];
if (!propKeys.includes(key)) {
// check if the key is part of the propKeys array
parsedOptions.push(element);
}
Expand All @@ -546,7 +554,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
}
Expand All @@ -558,7 +567,7 @@ function filterPropKeysFromUnParsedOptions(
parsedOptions.push(element);
}
}
return parsedOptions;
return parsedOptions.join(' ');
}

let registered = false;
Expand Down

0 comments on commit 118fcc9

Please sign in to comment.