Skip to content

Commit b41de47

Browse files
committed
fix(ci): always show execute process errors and log stdout if verbose
1 parent ebb4ed5 commit b41de47

14 files changed

+282
-76
lines changed

packages/ci/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ Optionally, you can override default options for further customization:
103103
| `nxProjectsFilter` | `string \| string[]` | `'--with-target={task}'` | Arguments passed to [`nx show projects`](https://nx.dev/nx-api/nx/documents/show#projects), only relevant for Nx in [monorepo mode](#monorepo-mode) [^2] |
104104
| `directory` | `string` | `process.cwd()` | Directory in which Code PushUp CLI should run |
105105
| `config` | `string \| null` | `null` [^1] | Path to config file (`--config` option) |
106-
| `silent` | `boolean` | `false` | Toggles if logs from CLI commands are printed |
106+
| `silent` | `boolean` | `false` | Hides logs from CLI commands (erros will be printed) |
107107
| `bin` | `string` | `'npx --no-install code-pushup'` | Command for executing Code PushUp CLI |
108108
| `detectNewIssues` | `boolean` | `true` | Toggles if new issues should be detected and returned in `newIssues` property |
109109
| `logger` | `Logger` | `console` | Logger for reporting progress and encountered problems |
Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,22 @@
11
import { DEFAULT_PERSIST_FORMAT } from '@code-pushup/models';
2-
import { executeProcess } from '@code-pushup/utils';
2+
import { executeProcess, isVerbose } from '@code-pushup/utils';
33
import type { CommandContext } from '../context.js';
44

55
export async function runCollect({
66
bin,
77
config,
88
directory,
9-
silent,
9+
observer,
1010
}: CommandContext): Promise<void> {
11-
const { stdout } = await executeProcess({
11+
await executeProcess({
1212
command: bin,
1313
args: [
14+
...(isVerbose() ? ['--verbose'] : []),
15+
'--no-progress',
1416
...(config ? [`--config=${config}`] : []),
1517
...DEFAULT_PERSIST_FORMAT.map(format => `--persist.format=${format}`),
1618
],
1719
cwd: directory,
20+
observer,
1821
});
19-
if (!silent) {
20-
console.info(stdout);
21-
}
2222
}
Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { DEFAULT_PERSIST_FORMAT } from '@code-pushup/models';
2-
import { executeProcess } from '@code-pushup/utils';
2+
import { executeProcess, isVerbose } from '@code-pushup/utils';
33
import type { CommandContext } from '../context.js';
44

55
type CompareOptions = {
@@ -10,21 +10,20 @@ type CompareOptions = {
1010

1111
export async function runCompare(
1212
{ before, after, label }: CompareOptions,
13-
{ bin, config, directory, silent }: CommandContext,
13+
{ bin, config, directory, observer }: CommandContext,
1414
): Promise<void> {
15-
const { stdout } = await executeProcess({
15+
await executeProcess({
1616
command: bin,
1717
args: [
1818
'compare',
19+
...(isVerbose() ? ['--verbose'] : []),
1920
`--before=${before}`,
2021
`--after=${after}`,
2122
...(label ? [`--label=${label}`] : []),
2223
...(config ? [`--config=${config}`] : []),
2324
...DEFAULT_PERSIST_FORMAT.map(format => `--persist.format=${format}`),
2425
],
2526
cwd: directory,
27+
observer,
2628
});
27-
if (!silent) {
28-
console.info(stdout);
29-
}
3029
}

packages/ci/src/lib/cli/commands/merge-diffs.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,30 +3,29 @@ import {
33
DEFAULT_PERSIST_FILENAME,
44
DEFAULT_PERSIST_OUTPUT_DIR,
55
} from '@code-pushup/models';
6-
import { executeProcess } from '@code-pushup/utils';
6+
import { executeProcess, isVerbose } from '@code-pushup/utils';
77
import type { CommandContext } from '../context.js';
88

99
export async function runMergeDiffs(
1010
files: string[],
11-
{ bin, config, directory, silent }: CommandContext,
11+
{ bin, config, directory, observer }: CommandContext,
1212
): Promise<string> {
1313
const outputDir = path.join(directory, DEFAULT_PERSIST_OUTPUT_DIR);
1414
const filename = `merged-${DEFAULT_PERSIST_FILENAME}`;
1515

16-
const { stdout } = await executeProcess({
16+
await executeProcess({
1717
command: bin,
1818
args: [
1919
'merge-diffs',
20+
...(isVerbose() ? ['--verbose'] : []),
2021
...files.map(file => `--files=${file}`),
2122
...(config ? [`--config=${config}`] : []),
2223
`--persist.outputDir=${outputDir}`,
2324
`--persist.filename=${filename}`,
2425
],
2526
cwd: directory,
27+
observer,
2628
});
27-
if (!silent) {
28-
console.info(stdout);
29-
}
3029

3130
return path.join(outputDir, `${filename}-diff.md`);
3231
}

packages/ci/src/lib/cli/commands/print-config.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import path from 'node:path';
33
import {
44
executeProcess,
55
generateRandomId,
6+
isVerbose,
67
readJsonFile,
78
stringifyError,
89
} from '@code-pushup/utils';
@@ -12,24 +13,23 @@ export async function runPrintConfig({
1213
bin,
1314
config,
1415
directory,
15-
silent,
16+
observer,
1617
}: CommandContext): Promise<unknown> {
1718
// random file name so command can be run in parallel
1819
const outputFile = `code-pushup.${generateRandomId()}.config.json`;
1920
const outputPath = path.join(directory, outputFile);
2021

21-
const { stdout } = await executeProcess({
22+
await executeProcess({
2223
command: bin,
2324
args: [
2425
...(config ? [`--config=${config}`] : []),
2526
'print-config',
27+
...(isVerbose() ? ['--verbose'] : []),
2628
`--output=${outputFile}`,
2729
],
2830
cwd: directory,
31+
observer,
2932
});
30-
if (!silent) {
31-
console.info(stdout);
32-
}
3333

3434
try {
3535
const content = await readJsonFile(outputPath);

packages/ci/src/lib/cli/context.ts

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,20 @@
1+
import type { ProcessObserver } from '@code-pushup/utils';
2+
import { createExecutionObserver } from '../create-execution-observer.js';
13
import type { Settings } from '../models.js';
24
import type { ProjectConfig } from '../monorepo/index.js';
35

4-
export type CommandContext = Pick<
5-
Settings,
6-
'bin' | 'config' | 'directory' | 'silent'
7-
>;
6+
export type CommandContext = Pick<Settings, 'bin' | 'config' | 'directory'> & {
7+
observer?: ProcessObserver;
8+
};
89

910
export function createCommandContext(
10-
settings: Settings,
11+
{ config, bin, directory, silent }: Settings,
1112
project: ProjectConfig | null | undefined,
1213
): CommandContext {
1314
return {
14-
bin: project?.bin ?? settings.bin,
15-
directory: project?.directory ?? settings.directory,
16-
config: settings.config,
17-
silent: settings.silent,
15+
bin: project?.bin ?? bin,
16+
directory: project?.directory ?? directory,
17+
config,
18+
observer: createExecutionObserver({ silent }),
1819
};
1920
}

packages/ci/src/lib/cli/context.unit.test.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
1+
import { expect } from 'vitest';
12
import { type CommandContext, createCommandContext } from './context.js';
23

34
describe('createCommandContext', () => {
5+
const expectedObserver = expect.objectContaining({
6+
onStderr: expect.any(Function),
7+
onStdout: expect.any(Function),
8+
});
9+
410
it('should pick CLI-related settings in standalone mode', () => {
511
expect(
612
createCommandContext(
@@ -25,7 +31,7 @@ describe('createCommandContext', () => {
2531
bin: 'npx --no-install code-pushup',
2632
directory: '/test',
2733
config: null,
28-
silent: false,
34+
observer: expectedObserver,
2935
});
3036
});
3137

@@ -57,7 +63,7 @@ describe('createCommandContext', () => {
5763
bin: 'yarn code-pushup',
5864
directory: '/test/ui',
5965
config: null,
60-
silent: false,
66+
observer: expectedObserver,
6167
});
6268
});
6369
});
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import { expect } from 'vitest';
2+
import { executeProcess } from '@code-pushup/utils';
3+
import { createExecutionObserver } from './create-execution-observer.js';
4+
5+
describe('createExecutionObserver', () => {
6+
const message = 'This is stdout';
7+
const error = 'This is stderr';
8+
9+
it('should use execute process and use observer to capture stdout message and stderr will be empty', async () => {
10+
const { stdout, stderr } = await executeProcess({
11+
command: 'node',
12+
args: ['-e', `"console.log('${message}');"`],
13+
observer: createExecutionObserver(),
14+
});
15+
16+
expect(stdout).toMatch(message);
17+
expect(stderr).toMatch('');
18+
});
19+
20+
it('should use execute process and use observer to capture stdout message and stderr will be error', async () => {
21+
const { stdout, stderr } = await executeProcess({
22+
command: 'node',
23+
args: ['-e', `"console.log('${message}'); console.error('${error}');"`],
24+
observer: createExecutionObserver(),
25+
});
26+
27+
expect(stdout).toMatch(message);
28+
expect(stderr).toMatch(error);
29+
});
30+
31+
it('should use execute process and use observer to capture stderr error and ignore stdout message', async () => {
32+
const { stdout, stderr } = await executeProcess({
33+
command: 'node',
34+
args: ['-e', `"console.log('${message}'); console.error('${error}');"`],
35+
observer: createExecutionObserver({ silent: true }),
36+
});
37+
38+
expect(stdout).toMatch('');
39+
expect(stderr).toMatch(error);
40+
});
41+
});
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import { type ProcessObserver, isVerbose } from '@code-pushup/utils';
2+
3+
export function createExecutionObserver(
4+
{
5+
silent,
6+
}: {
7+
silent?: boolean;
8+
} = { silent: false },
9+
): ProcessObserver {
10+
return {
11+
onStderr: stderr => {
12+
console.warn(stderr);
13+
},
14+
...((!silent || isVerbose()) && {
15+
onStdout: stdout => {
16+
console.info(stdout);
17+
},
18+
}),
19+
};
20+
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import { expect, vi } from 'vitest';
2+
import { createExecutionObserver } from './create-execution-observer.js';
3+
4+
describe('createExecutionObserver', () => {
5+
it('should create execution observer with default settings', () => {
6+
expect(createExecutionObserver()).toStrictEqual({
7+
onStderr: expect.any(Function),
8+
onStdout: expect.any(Function),
9+
});
10+
});
11+
12+
it('should create execution observer with silent false settings', () => {
13+
expect(createExecutionObserver({ silent: false })).toStrictEqual({
14+
onStderr: expect.any(Function),
15+
onStdout: expect.any(Function),
16+
});
17+
});
18+
19+
it('should create execution observer with default silent taking priority over CP_VERBOSE flag', () => {
20+
vi.stubEnv('CP_VERBOSE', 'false');
21+
22+
expect(createExecutionObserver()).toStrictEqual({
23+
onStderr: expect.any(Function),
24+
onStdout: expect.any(Function),
25+
});
26+
});
27+
28+
it('should create execution observer with silent setting', () => {
29+
expect(createExecutionObserver({ silent: true })).toStrictEqual({
30+
onStderr: expect.any(Function),
31+
});
32+
});
33+
});

0 commit comments

Comments
 (0)