From d25120c0ff49872cf539fd33be018ea096f48136 Mon Sep 17 00:00:00 2001
From: Emma Casolin <emma.casolin@matrix.ai>
Date: Mon, 8 Aug 2022 16:42:55 +1000
Subject: [PATCH] fix: switched `exec` from wrapping `execFile` to `spawn`

---
 tests/nat/utils.ts  | 10 ++++----
 tests/utils/exec.ts | 57 +++++++++++++++++++++++++--------------------
 2 files changed, 37 insertions(+), 30 deletions(-)

diff --git a/tests/nat/utils.ts b/tests/nat/utils.ts
index c15a4080d8..8d0c09568b 100644
--- a/tests/nat/utils.ts
+++ b/tests/nat/utils.ts
@@ -2,7 +2,7 @@ import type { ChildProcess } from 'child_process';
 import os from 'os';
 import fs from 'fs';
 import path from 'path';
-import child_process from 'child_process';
+import childProcess from 'child_process';
 import readline from 'readline';
 import Logger, { LogLevel, StreamHandler } from '@matrixai/logger';
 import * as execUtils from '../utils/exec';
@@ -147,7 +147,7 @@ function createUserNamespace(
   logger: Logger = new Logger(createUserNamespace.name),
 ): ChildProcess {
   logger.info('unshare --user --map-root-user');
-  const subprocess = child_process.spawn(
+  const subprocess = childProcess.spawn(
     'unshare',
     ['--user', '--map-root-user'],
     {
@@ -172,7 +172,7 @@ function createNetworkNamespace(
   logger.info(
     `nsenter --target ${usrnsPid.toString()} --user --preserve-credentials unshare --net`,
   );
-  const subprocess = child_process.spawn(
+  const subprocess = childProcess.spawn(
     'nsenter',
     [
       '--target',
@@ -1405,9 +1405,9 @@ async function setupNAT(
 
 export {
   nsenter,
+  setupNAT,
+  setupNATWithSeedNode,
   createUserNamespace,
   createNetworkNamespace,
   setupNetworkNamespaceInterfaces,
-  setupNAT,
-  setupNATWithSeedNode,
 };
diff --git a/tests/utils/exec.ts b/tests/utils/exec.ts
index 2149122597..4779763961 100644
--- a/tests/utils/exec.ts
+++ b/tests/utils/exec.ts
@@ -61,31 +61,40 @@ const generateDockerArgs = (mountPath: string) => [
 ];
 
 /**
- * Wrapper for execFile to make it asynchronous and non-blocking
+ * Execute general (non-Polykey) commands
  */
 async function exec(
   command: string,
   args: Array<string> = [],
+  opts: ExecOpts = { env: {} },
 ): Promise<{
   stdout: string;
   stderr: string;
 }> {
+  const env = {
+    ...process.env,
+    ...opts.env,
+  };
   return new Promise((resolve, reject) => {
-    child_process.execFile(
-      command,
-      args,
-      { windowsHide: true },
-      (error, stdout, stderr) => {
-        if (error) {
-          reject(error);
-        } else {
-          return resolve({
-            stdout,
-            stderr,
-          });
-        }
-      },
-    );
+    let stdout = '',
+      stderr = '';
+    const subprocess = child_process.spawn(command, args, {
+      env,
+      windowsHide: true,
+      shell: false,
+    });
+    subprocess.stdout.on('data', (data) => {
+      stdout += data.toString();
+    });
+    subprocess.stderr.on('data', (data) => {
+      stderr += data.toString();
+    });
+    subprocess.on('exit', () => {
+      resolve({ stdout, stderr });
+    });
+    subprocess.on('error', (e) => {
+      reject(e);
+    });
   });
 }
 
@@ -101,8 +110,6 @@ async function pk(args: Array<string>): Promise<any> {
  * Both stdout and stderr are the entire output including newlines
  * This can only be used serially, because the mocks it relies on are global singletons
  * If it is used concurrently, the mocking side-effects can conflict
- * @param env Augments env for command execution
- * @param cwd Defaults to temporary directory
  */
 async function pkStdio(
   args: Array<string> = [],
@@ -607,19 +614,19 @@ function escapeShellArgs(arg: string): string {
 export {
   tsConfigPath,
   polykeyPath,
-  exec,
-  pk,
-  pkStdio,
   pkExec,
   pkSpawn,
+  pkExpect,
+  pkStdio,
+  exec,
+  pk,
+  setupTestAgent,
+  processExit,
+  expectProcessError,
   pkExecWithShell,
   pkExecWithoutShell,
   pkSpawnWithShell,
   pkSpawnWithoutShell,
-  pkExpect,
-  processExit,
-  expectProcessError,
-  setupTestAgent,
   spawnFile,
   escapeShellArgs,
 };