DOP-4701: Preserve stdout in ShellCommandExecutor

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;
     }

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;
@@ -18,15 +17,28 @@ describe('ShellCommandExecutor Tests', () => {
   });
 
   describe('ShellCommandExecutor Tests', () => {
+    test('ShellCommandExecutor returns correct output on success', async () => {
+      const command = ["echo 'stdout output'", "echo 'stderr output' >&2"];
+      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('stderr output\n');
+      expect(resp.output).toBe('stdout output');
+      expect(resp.status).toBe('success');
+    });
+
     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);
     });
   });
 });