fix(nx-plugin): print process output for CLI command (#1095)

This PR includes:
- use `executeProcess` in `cli` executor to log process output

This changes are needed to be able to use the plugin in combination with
the CI actions on all platforms

---------

Co-authored-by: Matěj Chalk <34691111+matejchalk@users.noreply.github.com>

Author: BioPhoton

e2e/nx-plugin-e2e/tests/executor-cli.e2e.test.ts

@@ -172,6 +172,7 @@ describe('executor command', () => {
     expect(cleanStdout).toContain(
       'nx run my-lib:code-pushup collect --persist.filename=terminal-report',
     );
+    expect(cleanStdout).toContain('Code PushUp CLI');
 
     await expect(
       readJsonFile(path.join(cwd, '.reports', 'terminal-report.json')),

packages/nx-plugin/src/executors/cli/executor.int.test.ts

@@ -1,25 +1,29 @@
-import { execSync } from 'node:child_process';
 import { afterEach, expect, vi } from 'vitest';
 import { executorContext } from '@code-pushup/test-nx-utils';
+import * as executeProcessModule from '../../internal/execute-process.js';
 import runAutorunExecutor from './executor.js';
 import * as utils from './utils.js';
 
-vi.mock('node:child_process', async () => {
-  const actual = await vi.importActual('node:child_process');
-  return {
-    ...actual,
-    execSync: vi.fn(),
-  };
-});
-
 describe('runAutorunExecutor', () => {
   const parseAutorunExecutorOptionsSpy = vi.spyOn(
     utils,
     'parseAutorunExecutorOptions',
   );
+  const executeProcessSpy = vi.spyOn(executeProcessModule, 'executeProcess');
+
+  beforeEach(() => {
+    executeProcessSpy.mockResolvedValue({
+      code: 0,
+      stdout: '',
+      stderr: '',
+      date: new Date().toISOString(),
+      duration: 100,
+    });
+  });
 
   afterEach(() => {
     parseAutorunExecutorOptionsSpy.mockReset();
+    executeProcessSpy.mockReset();
   });
 
   it('should normalize context, parse CLI options and execute command', async () => {
@@ -38,11 +42,15 @@ describe('runAutorunExecutor', () => {
         projectConfig: expect.objectContaining({ name: 'utils' }),
       }),
     );
-    // eslint-disable-next-line n/no-sync
-    expect(execSync).toHaveBeenCalledTimes(1);
-    // eslint-disable-next-line n/no-sync
-    expect(execSync).toHaveBeenCalledWith(expect.stringContaining('utils'), {
+    expect(executeProcessSpy).toHaveBeenCalledTimes(1);
+    expect(executeProcessSpy).toHaveBeenCalledWith({
+      command: 'npx',
+      args: expect.arrayContaining(['@code-pushup/cli']),
       cwd: process.cwd(),
+      observer: {
+        onError: expect.any(Function),
+        onStdout: expect.any(Function),
+      },
     });
   });
 });

packages/nx-plugin/src/executors/cli/executor.ts

@@ -1,6 +1,9 @@
 import { type ExecutorContext, logger } from '@nx/devkit';
-import { execSync } from 'node:child_process';
-import { createCliCommand } from '../internal/cli.js';
+import { executeProcess } from '../../internal/execute-process.js';
+import {
+  createCliCommandObject,
+  createCliCommandString,
+} from '../internal/cli.js';
 import { normalizeContext } from '../internal/context.js';
 import type { AutorunCommandExecutorOptions } from './schema.js';
 import { mergeExecutorOptions, parseAutorunExecutorOptions } from './utils.js';
@@ -11,7 +14,7 @@ export type ExecutorOutput = {
   error?: Error;
 };
 
-export default function runAutorunExecutor(
+export default async function runAutorunExecutor(
   terminalAndExecutorOptions: AutorunCommandExecutorOptions,
   context: ExecutorContext,
 ): Promise<ExecutorOutput> {
@@ -25,9 +28,10 @@ export default function runAutorunExecutor(
     normalizedContext,
   );
   const { dryRun, verbose, command } = mergedOptions;
-
-  const commandString = createCliCommand({ command, args: cliArgumentObject });
-  const commandStringOptions = context.cwd ? { cwd: context.cwd } : {};
+  const commandString = createCliCommandString({
+    command,
+    args: cliArgumentObject,
+  });
   if (verbose) {
     logger.info(`Run CLI executor ${command ?? ''}`);
     logger.info(`Command: ${commandString}`);
@@ -36,21 +40,21 @@ export default function runAutorunExecutor(
     logger.warn(`DryRun execution of: ${commandString}`);
   } else {
     try {
-      // @TODO use executeProcess instead of execSync -> non blocking, logs #761
-      // eslint-disable-next-line n/no-sync
-      execSync(commandString, commandStringOptions);
+      await executeProcess({
+        ...createCliCommandObject({ command, args: cliArgumentObject }),
+        ...(context.cwd ? { cwd: context.cwd } : {}),
+      });
     } catch (error) {
       logger.error(error);
-      return Promise.resolve({
+      return {
         success: false,
         command: commandString,
         error: error as Error,
-      });
+      };
     }
   }
-
-  return Promise.resolve({
+  return {
     success: true,
     command: commandString,
-  });
+  };
 }

packages/nx-plugin/src/executors/cli/executor.unit.test.ts

@@ -1,29 +1,17 @@
 import { logger } from '@nx/devkit';
-import { execSync } from 'node:child_process';
 import { afterAll, afterEach, beforeEach, expect, vi } from 'vitest';
 import { executorContext } from '@code-pushup/test-nx-utils';
 import { MEMFS_VOLUME } from '@code-pushup/test-utils';
+import * as executeProcessModule from '../../internal/execute-process.js';
 import runAutorunExecutor from './executor.js';
 
-vi.mock('node:child_process', async () => {
-  const actual = await vi.importActual('node:child_process');
-
-  return {
-    ...actual,
-    execSync: vi.fn((command: string) => {
-      if (command.includes('THROW_ERROR')) {
-        throw new Error(command);
-      }
-    }),
-  };
-});
-
 describe('runAutorunExecutor', () => {
   const processEnvCP = Object.fromEntries(
     Object.entries(process.env).filter(([k]) => k.startsWith('CP_')),
   );
   const loggerInfoSpy = vi.spyOn(logger, 'info');
   const loggerWarnSpy = vi.spyOn(logger, 'warn');
+  const executeProcessSpy = vi.spyOn(executeProcessModule, 'executeProcess');
 
   /* eslint-disable functional/immutable-data, @typescript-eslint/no-dynamic-delete */
   beforeAll(() => {
@@ -34,27 +22,39 @@ describe('runAutorunExecutor', () => {
 
   beforeEach(() => {
     vi.unstubAllEnvs();
+    executeProcessSpy.mockResolvedValue({
+      code: 0,
+      stdout: '',
+      stderr: '',
+      date: new Date().toISOString(),
+      duration: 100,
+    });
   });
 
   afterEach(() => {
     loggerWarnSpy.mockReset();
     loggerInfoSpy.mockReset();
+    executeProcessSpy.mockReset();
   });
 
   afterAll(() => {
     Object.entries(processEnvCP).forEach(([k, v]) => (process.env[k] = v));
   });
   /* eslint-enable functional/immutable-data, @typescript-eslint/no-dynamic-delete */
 
-  it('should call execSync with return result', async () => {
+  it('should call executeProcess with return result', async () => {
     const output = await runAutorunExecutor({}, executorContext('utils'));
     expect(output.success).toBe(true);
     expect(output.command).toMatch('npx @code-pushup/cli');
-    // eslint-disable-next-line n/no-sync
-    expect(execSync).toHaveBeenCalledWith(
-      expect.stringContaining('npx @code-pushup/cli'),
-      { cwd: MEMFS_VOLUME },
-    );
+    expect(executeProcessSpy).toHaveBeenCalledWith({
+      command: 'npx',
+      args: expect.arrayContaining(['@code-pushup/cli']),
+      cwd: MEMFS_VOLUME,
+      observer: {
+        onError: expect.any(Function),
+        onStdout: expect.any(Function),
+      },
+    });
   });
 
   it('should normalize context', async () => {
@@ -67,9 +67,14 @@ describe('runAutorunExecutor', () => {
     );
     expect(output.success).toBe(true);
     expect(output.command).toMatch('utils');
-    // eslint-disable-next-line n/no-sync
-    expect(execSync).toHaveBeenCalledWith(expect.stringContaining('utils'), {
+    expect(executeProcessSpy).toHaveBeenCalledWith({
+      command: 'npx',
+      args: expect.arrayContaining(['@code-pushup/cli']),
       cwd: 'cwd-form-context',
+      observer: {
+        onError: expect.any(Function),
+        onStdout: expect.any(Function),
+      },
     });
   });
 
@@ -114,8 +119,7 @@ describe('runAutorunExecutor', () => {
       { verbose: true },
       { ...executorContext('github-action'), cwd: '<CWD>' },
     );
-    // eslint-disable-next-line n/no-sync
-    expect(execSync).toHaveBeenCalledTimes(1);
+    expect(executeProcessSpy).toHaveBeenCalledTimes(1);
 
     expect(output.command).toMatch('--verbose');
     expect(loggerWarnSpy).toHaveBeenCalledTimes(0);

packages/nx-plugin/src/executors/internal/cli.ts

@@ -1,4 +1,7 @@
-export function createCliCommand(options?: {
+import { logger } from '@nx/devkit';
+import type { ProcessConfig } from '../../internal/execute-process.js';
+
+export function createCliCommandString(options?: {
   args?: Record<string, unknown>;
   command?: string;
   bin?: string;
@@ -9,6 +12,26 @@ export function createCliCommand(options?: {
   )}`;
 }
 
+export function createCliCommandObject(options?: {
+  args?: Record<string, unknown>;
+  command?: string;
+  bin?: string;
+}): ProcessConfig {
+  const { bin = '@code-pushup/cli', command, args } = options ?? {};
+  return {
+    command: 'npx',
+    args: [bin, ...objectToCliArgs({ _: command ?? [], ...args })],
+    observer: {
+      onError: error => {
+        logger.error(error.message);
+      },
+      onStdout: data => {
+        logger.log(data);
+      },
+    },
+  };
+}
+
 type ArgumentValue = number | string | boolean | string[];
 export type CliArgsObject<T extends object = Record<string, ArgumentValue>> =
   T extends never

packages/nx-plugin/src/executors/internal/cli.unit.test.ts

@@ -1,5 +1,9 @@
 import { describe, expect, it } from 'vitest';
-import { createCliCommand, objectToCliArgs } from './cli.js';
+import {
+  createCliCommandObject,
+  createCliCommandString,
+  objectToCliArgs,
+} from './cli.js';
 
 describe('objectToCliArgs', () => {
   it('should empty params', () => {
@@ -86,14 +90,59 @@ describe('objectToCliArgs', () => {
   });
 });
 
-describe('createCliCommand', () => {
+describe('createCliCommandString', () => {
   it('should create command out of object for arguments', () => {
-    const result = createCliCommand({ args: { verbose: true } });
+    const result = createCliCommandString({ args: { verbose: true } });
     expect(result).toBe('npx @code-pushup/cli --verbose');
   });
 
   it('should create command out of object for arguments with positional', () => {
-    const result = createCliCommand({ args: { _: 'autorun', verbose: true } });
+    const result = createCliCommandString({
+      args: { _: 'autorun', verbose: true },
+    });
     expect(result).toBe('npx @code-pushup/cli autorun --verbose');
   });
 });
+
+describe('createCliCommandObject', () => {
+  it('should create command out of object for arguments', () => {
+    expect(createCliCommandObject({ args: { verbose: true } })).toStrictEqual({
+      args: ['@code-pushup/cli', '--verbose'],
+      command: 'npx',
+      observer: {
+        onError: expect.any(Function),
+        onStdout: expect.any(Function),
+      },
+    });
+  });
+
+  it('should create command out of object for arguments with positional', () => {
+    expect(
+      createCliCommandObject({
+        args: { _: 'autorun', verbose: true },
+      }),
+    ).toStrictEqual({
+      args: ['@code-pushup/cli', 'autorun', '--verbose'],
+      command: 'npx',
+      observer: {
+        onError: expect.any(Function),
+        onStdout: expect.any(Function),
+      },
+    });
+  });
+
+  it('should create command out of object for arguments with bin', () => {
+    expect(
+      createCliCommandObject({
+        bin: 'node_modules/@code-pushup/cli/src/bin.js',
+      }),
+    ).toStrictEqual({
+      args: ['node_modules/@code-pushup/cli/src/bin.js'],
+      command: 'npx',
+      observer: {
+        onError: expect.any(Function),
+        onStdout: expect.any(Function),
+      },
+    });
+  });
+});