From 45fd9558492b9551f6beaea8c14fec976cf2b930 Mon Sep 17 00:00:00 2001 From: Heli Aldridge Date: Thu, 9 May 2024 11:00:03 -0400 Subject: [PATCH] DOP-4701: Preserve stdout in ShellCommandExecutor --- src/services/commandExecutor.ts | 2 +- .../unit/services/shellCommandExecutor.test.ts | 17 +++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/services/commandExecutor.ts b/src/services/commandExecutor.ts index 9dc2fe04d7..96d31a84d0 100644 --- a/src/services/commandExecutor.ts +++ b/src/services/commandExecutor.ts @@ -65,7 +65,7 @@ export class ShellCommandExecutor implements ICommandExecutor { resp.status = CommandExecutorResponseStatus.success; return resp; } catch (error) { - resp.output = ''; + resp.output = (error.stdout || '').trim(); resp.error = error; resp.status = CommandExecutorResponseStatus.failed; } diff --git a/tests/unit/services/shellCommandExecutor.test.ts b/tests/unit/services/shellCommandExecutor.test.ts index 1d2889a821..186cc86422 100644 --- a/tests/unit/services/shellCommandExecutor.test.ts +++ b/tests/unit/services/shellCommandExecutor.test.ts @@ -1,6 +1,5 @@ import { ShellCommandExecutor } from '../../../src/services/commandExecutor'; import cp from 'child_process'; -jest.mock('child_process'); describe('ShellCommandExecutor Tests', () => { let commandExecutor: ShellCommandExecutor; @@ -19,14 +18,16 @@ describe('ShellCommandExecutor Tests', () => { describe('ShellCommandExecutor Tests', () => { test('ShellCommandExecutor properly throws on system level error', async () => { - cp.exec.mockImplementation((command, options, callback) => { - callback(Error('Test error'), { stdErr: 'invalid command', stdout: 'test_repo_project_snooty_name' }); - }); - const resp = await commandExecutor.execute([]); - expect(resp.error).not.toBe(undefined); - expect(resp.output).toBe(''); + const command = ["echo 'stdout output'", "echo 'stderr output' >&2", 'exit 1']; + const resp = await commandExecutor.execute(command); + + // This is a strange interface, but I'm just here to fix a bug, not change the interface. + // The type of resp.error can be either a string or an Error instance. + expect(resp.error.toString()).toStrictEqual( + "Error: Command failed: echo 'stdout output' && echo 'stderr output' >&2 && exit 1\nstderr output\n" + ); + expect(resp.output).toBe('stdout output'); expect(resp.status).toBe('failed'); - expect(cp.exec).toBeCalledTimes(1); }); }); });