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 committed Apr 19, 2024
1 parent 82be2ae commit 3659328
Show file tree
Hide file tree
Showing 2 changed files with 168 additions and 27 deletions.
118 changes: 105 additions & 13 deletions packages/nx/src/executors/run-commands/run-commands.impl.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,72 @@ 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 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__: ['--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: 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 interpolate all unknown args as if they were --args', async () => {
Expand Down Expand Up @@ -86,7 +140,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 +150,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 @@ -312,6 +376,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
77 changes: 63 additions & 14 deletions packages/nx/src/executors/run-commands/run-commands.impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ const propKeys = [
'command',
'commands',
'color',
'no-color',
'parallel',
'no-parallel',
'readyWhen',
'cwd',
'args',
Expand All @@ -98,7 +100,7 @@ export interface NormalizedRunCommandsOptions extends RunCommandsOptions {
[k: string]: any;
};
unparsedCommandArgs?: {
[k: string]: string;
[k: string]: string | string[];
};
args?: string;
}
Expand Down Expand Up @@ -233,6 +235,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 @@ -488,16 +491,13 @@ 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(' ');
const filteredOptions = filterPropKeysFromCommandOptions(
opts.unknownOptions,
opts.parsedArgs
);
if (filteredOptions.length > 0) {
args += ` ${filteredOptions}`;
}
}
if (opts.args) {
args += ` ${opts.args}`;
Expand Down Expand Up @@ -530,6 +530,53 @@ function parseArgs(
});
}

/**
* This function expands the command options into a string.
* It filters out the prop keys from the command options.
* Depends on the type, it will return the command options as a string.
* @param commandOptions
* @param parsedArgs
* @returns a string of the command options
*/
function filterPropKeysFromCommandOptions(
commandOptions: {
[k: string]: any;
} = {},
parsedArgs: {
[k: string]: any;
} = {}
): string {
return Object.keys(commandOptions)
.filter(
(commandOptionsKey) =>
propKeys.includes(commandOptionsKey) === false &&
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(' ');
}
return `--${commandOptionsKey}=${commandOptions[commandOptionsKey]}`;
})
.join(' ');
}

/**
* This function filters out the prop keys from the unparsed options
* @param __unparsed__ e.g. ['--prop1', 'value1', '--prop2=value2', '--args=test']
Expand All @@ -539,16 +586,18 @@ function parseArgs(
function filterPropKeysFromUnParsedOptions(
__unparsed__: string[],
unparsedCommandArgs: {
[k: string]: string;
[k: 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('--', '');
let key = element.replace('--', '');
if (element.includes('=')) {
if (!propKeys.includes(key.split('=')[0].split('.')[0])) {
// 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 Down

0 comments on commit 3659328

Please sign in to comment.