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 10, 2024
1 parent caf663f commit 44b7d98
Show file tree
Hide file tree
Showing 2 changed files with 138 additions and 88 deletions.
106 changes: 75 additions & 31 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,19 +36,32 @@ describe('Run Commands', () => {
expect(readFile(f)).toEqual('123');
});

it('should not pass --args into underlying command', async () => {
const f = fileSync().name;
const result = await runCommands(
{
command: `echo`,
__unparsed__: ['--args=--key=123'],
args: '--key=123',
unparsedCommandArgs: { args: '--key=123' },
},
context
);
expect(result.terminalOutput.trim()).not.toContain('--args=--key=123');
});
it.each([
{
unparsed: ['test', '--args=--key=123', '--test=1', '--test=2'],
expected: 'test --test=1 --test=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 interpolate all unknown args as if they were --args', async () => {
const f = fileSync().name;
Expand Down Expand Up @@ -86,7 +99,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 @@ -96,6 +109,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 @@ -184,19 +207,21 @@ describe('Run Commands', () => {
expect(
interpolateArgsIntoCommand(
'echo',
{ __unparsed__: ['one', '-a=b'] } as any,
{
unparsedCommandArgs: { _: ['one'], a: 'b' },
parsedArgs: { a: 'b' },
} as any,
true
)
).toEqual('echo one -a=b');
).toEqual('echo one --a=b');
});

it('should not forward all unparsed args when the options is a prop to run command', () => {
expect(
interpolateArgsIntoCommand(
'echo',
{
__unparsed__: ['--args', 'test', 'hello'],
unparsedCommandArgs: { args: 'test' },
unparsedCommandArgs: { _: ['hello'], args: 'test' },
} as any,
true
)
Expand All @@ -206,26 +231,18 @@ describe('Run Commands', () => {
interpolateArgsIntoCommand(
'echo',
{
__unparsed__: ['--args=test', 'hello'],
unparsedCommandArgs: { _: ['hello'], parallel: true, args: 'test' },
} as any,
true
)
).toEqual('echo hello');

expect(
interpolateArgsIntoCommand(
'echo',
{ __unparsed__: ['--parallel=true', 'hello'] } as any,
true
)
).toEqual('echo hello');
});

it('should add all args when forwardAllArgs is true', () => {
expect(
interpolateArgsIntoCommand(
'echo',
{ args: '--additional-arg', __unparsed__: [] } as any,
{ args: '--additional-arg' } as any,
true
)
).toEqual('echo --additional-arg');
Expand All @@ -247,7 +264,8 @@ describe('Run Commands', () => {
'echo',
{
args: '--additional-arg',
__unparsed__: ['--additional-unparsed-arg'],
unparsedCommandArgs: { 'additional-unparsed-arg': true },
parsedArgs: { 'additional-unparsed-arg': true },
} as any,
true
)
Expand All @@ -260,7 +278,6 @@ describe('Run Commands', () => {
'echo {args.someValue}',
{
parsedArgs: {},
__unparsed__: [],
},
false
)
Expand All @@ -275,7 +292,6 @@ describe('Run Commands', () => {
parsedArgs: {
someValue: '"hello world"',
},
__unparsed__: [],
},
false
)
Expand Down Expand Up @@ -312,6 +328,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
120 changes: 63 additions & 57 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 @@ -86,7 +86,7 @@ export interface NormalizedRunCommandsOptions extends RunCommandsOptions {
[k: string]: any;
};
unparsedCommandArgs?: {
[k: string]: string;
[k: string]: string | string[];
};
args?: string;
}
Expand Down Expand Up @@ -462,11 +462,7 @@ export function interpolateArgsIntoCommand(
command: string,
opts: Pick<
NormalizedRunCommandsOptions,
| 'args'
| 'parsedArgs'
| '__unparsed__'
| 'unknownOptions'
| 'unparsedCommandArgs'
'args' | 'parsedArgs' | 'unknownOptions' | 'unparsedCommandArgs'
>,
forwardAllArgs: boolean
): string {
Expand All @@ -477,30 +473,37 @@ 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]}`)
.join(' ');
if (opts.unknownOptions && Object.keys(opts.unknownOptions).length > 0) {
const filteredOptions = filterPropKeysFromUnParsedOptions(
opts.unknownOptions,
opts.parsedArgs
);
if (filteredOptions.length > 0) {
args += ` ${filteredOptions}`;
}
}
if (opts.args) {
args += ` ${opts.args}`;
}
if (opts.__unparsed__?.length > 0) {
const filterdParsedOptions = filterPropKeysFromUnParsedOptions(
opts.__unparsed__,
opts.unparsedCommandArgs
if (
opts.unparsedCommandArgs &&
Object.keys(opts.unparsedCommandArgs).length > 0
) {
if (
Array.isArray(opts.unparsedCommandArgs?.['_']) &&
opts.unparsedCommandArgs['_'].length > 0
) {
args += ` ${opts.unparsedCommandArgs['_'].join(' ')}`;
}
const filteredOptions = filterPropKeysFromUnParsedOptions(
opts.unparsedCommandArgs,
opts.parsedArgs
);
if (filterdParsedOptions.length > 0) {
args += ` ${filterdParsedOptions.join(' ')}`;
if (filteredOptions.length > 0) {
args += ` ${filteredOptions}`;
}
}

return `${command}${args}`;
} else {
return command;
Expand All @@ -522,42 +525,45 @@ 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']
* @param commandOptions e.g. { prop1: 'value1', prop2: 'value2', args: 'test', 'env': {a: 'b'}, 'test': ['a', 'b'] }
* @returns filtered options that are not part of the propKeys array e.g. '--prop1=value1 --prop2=value2 --test=a --test=b'
*/
function filterPropKeysFromUnParsedOptions(
__unparsed__: string[],
unparsedCommandArgs: {
[k: string]: string;
commandOptions: {
[k: string]: any;
} = {},
parsedArgs: {
[k: string]: any;
} = {}
): 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])) {
// check if the key is part of the propKeys array
parsedOptions.push(element);
}
} else {
// check if the next element is a value for the key
if (propKeys.includes(key)) {
if (
index + 1 < __unparsed__.length &&
__unparsed__[index + 1] === unparsedCommandArgs[key]
) {
index++; // skip the next element
}
} else {
parsedOptions.push(element);
}
): string {
return Object.keys(commandOptions)
.filter(
(commandOptionsKey) =>
propKeys.includes(commandOptionsKey) === false &&
commandOptionsKey !== '_' &&
parsedArgs[commandOptionsKey] === commandOptions[commandOptionsKey]
)
.map((commandOptionsKey) => {
if (Array.isArray(commandOptions[commandOptionsKey])) {
return commandOptions[commandOptionsKey]
.map((v) => `--${commandOptionsKey}=${v}`)
.join(' ');
} else if (typeof commandOptions[commandOptionsKey] === 'boolean') {
return commandOptions[commandOptionsKey]
? `--${commandOptionsKey}`
: '';
} else if (
typeof commandOptions[commandOptionsKey] === 'object' &&
Object.keys(commandOptions[commandOptionsKey]).length > 0
) {
return Object.keys(commandOptions[commandOptionsKey])
.map(
(key) =>
`--${commandOptionsKey}.${key}=${commandOptions[commandOptionsKey][key]}`
)
.join(' ');
}
} else {
parsedOptions.push(element);
}
}
return parsedOptions;
return `--${commandOptionsKey}=${commandOptions[commandOptionsKey]}`;
})
.join(' ');
}

0 comments on commit 44b7d98

Please sign in to comment.