cli-test: more commands and tests

packages/cli-test/src/cli/commands/app.ts

@@ -46,8 +46,27 @@
   }
 };
 
+/**
+ * `slack app list`
+ * @param appPath path to app
+ * @returns command output
+ */
+export const list = async function appList(appPath: string): Promise<string> {
+  // TODO: (breaking change) separate parameters vs single-param-object
+  const cmd = new SlackCLIProcess('app list');
+  try {
+    const proc = await cmd.execAsync({
+      cwd: appPath,
+    });
+    return proc.output;
+  } catch (error) {
+    throw commandError(error, 'appList');
+  }
+};
+
 // TODO: (breaking change): rename properties of this default export to match actual command names
 export default {
   workspaceDelete: del,
   workspaceInstall: install,
+  workspaceList: list,
 };

packages/cli-test/src/cli/commands/platform.ts

@@ -1,4 +1,3 @@
-import kill from 'tree-kill';
 import logger from '../../utils/logger';
 import { SlackCLIProcess } from '../cli-process';
 import { shell } from '../shell';
@@ -94,14 +93,12 @@
       // Wait for output
       shell.waitForOutput(stringToWait, proc).then(() => {
         // kill the shell process
-        kill(proc.process.pid!, (err) => {
-          if (err) {
-            const msg = `activityTailStop command failed to kill process: ${err}`;
-            logger.warn(msg);
-            reject(new Error(msg));
-          } else {
-            resolve(proc.output);
-          }
+        shell.kill(proc).then(() => {
+          resolve(proc.output);
+        }, (err) => {
+          const msg = `activityTailStop command failed to kill process: ${err}`;
+          logger.warn(msg);
+          reject(new Error(msg));
         });
       }, reject);
     });
@@ -192,19 +189,19 @@
     // TODO: teamName param should be changed to something else. 'wait for shutdown' or some such (breaking change)
     return new Promise((resolve, reject) => {
       // kill the shell process
-      kill(proc.process.pid!, (err) => {
-        if (err) {
-          const msg = `runStop command failed to kill process: ${err}`;
-          logger.warn(msg);
-          reject(new Error(msg));
-        } else if (teamName) {
+      shell.kill(proc).then(() => {
+        if (teamName) {
           // TODO: this is messed up. does not match to parameter name at all - team name has nothing to do with this.
           // Check if local app was deleted automatically, if --cleanup was passed to `runStart`
           // Wait for the output to verify process stopped
           shell.waitForOutput(SlackTracerId.SLACK_TRACE_PLATFORM_RUN_STOP, proc).then(resolve, reject);
         } else {
           resolve();
         }
+      }, (err) => {
+        const msg = `runStop command failed to kill process: ${err}`;
+        logger.warn(msg);
+        reject(new Error(msg));
       });
     });
   },

packages/cli-test/src/cli/index.ts

@@ -21,6 +21,7 @@
   app: {
     delete: appCommands.workspaceDelete,
     install: appCommands.workspaceInstall,
+    list: appCommands.workspaceList,
   },
   ...authCommands,
   auth: authCommands,
@@ -71,12 +72,13 @@
     // and isLocalApp are not needed either
     /** Path to app. If not provided, will not interact with any app */
     appPath?: string;
-    /** Team domain where app is installed */
+    /** Team domain or ID where app is installed */
     appTeamID: string;
     isLocalApp?: boolean;
   }): Promise<void> {
     if (appPath) {
       // List instances of app installation if app path provided
+      // TODO: refactor this into standalone workspace list command
       const cmd = new SlackCLIProcess('workspace list');
       const { output: installedAppsOutput } = await cmd.execAsync({ cwd: appPath });
       // If app is installed
@@ -85,7 +87,7 @@
         try {
           await SlackCLI.app.delete(appPath, appTeamID, { isLocalApp });
         } catch (error) {
-          logger.info(`Could not delete gracefully. Error: ${error}`);
+          logger.warn(`stopSession could not delete app gracefully, continuing. Error: ${error}`);
         }
 
         // Delete app.json file. Needed for retries. Otherwise asks for collaborator, if old file is present

packages/cli-test/src/cli/shell.spec.ts

@@ -0,0 +1,93 @@
+import { assert } from 'chai';
+import sinon from 'sinon';
+import child from 'child_process';
+import stream from 'stream';
+import EventEmitter from 'events';
+import { shell } from './shell';
+import type { ShellProcess } from '../utils/types';
+
+describe('shell module', () => {
+  const sandbox = sinon.createSandbox();
+  let spawnSpy: sinon.SinonStub;
+  let spawnProcess: child.ChildProcessWithoutNullStreams;
+  let runSpy: sinon.SinonStub;
+  let runOutput: child.SpawnSyncReturns<Buffer>;
+
+  beforeEach(() => {
+    spawnProcess = new EventEmitter() as child.ChildProcessWithoutNullStreams;
+    spawnProcess.stdout = new EventEmitter() as stream.Readable;
+    spawnProcess.stderr = new EventEmitter() as stream.Readable;
+    spawnProcess.stdin = new stream.Writable();
+    spawnSpy = sandbox.stub(child, 'spawn').returns(spawnProcess);
+    runOutput = { pid: 1337, output: [], stdout: Buffer.from([]), stderr: Buffer.from([]), status: 0, signal: null };
+    runSpy = sandbox.stub(child, 'spawnSync').returns(runOutput);
+    sandbox.stub(shell, 'kill').resolves(true);
+  });
+  afterEach(() => {
+    sandbox.restore();
+  });
+
+  describe('spawnProcess method', () => {
+    it('should invoke `assembleShellEnv` and pass as child_process.spawn `env` parameter', () => {
+      const fakeEnv = { HEY: 'yo' };
+      const assembleSpy = sandbox.stub(shell, 'assembleShellEnv').returns(fakeEnv);
+      const fakeCmd = 'echo "hi"';
+      shell.spawnProcess(fakeCmd);
+      sandbox.assert.calledOnce(assembleSpy);
+      sandbox.assert.calledWithMatch(spawnSpy, fakeCmd, sinon.match({ shell: true, env: fakeEnv }));
+    });
+    it('should raise bubble error details up', () => {
+      spawnSpy.throws(new Error('this is bat country'));
+      assert.throw(() => {
+        shell.spawnProcess('about to explode');
+      }, /this is bat country/);
+    });
+  });
+
+  describe('runCommandSync method', () => {
+    it('should invoke `assembleShellEnv` and pass as child_process.spawnSync `env` parameter', () => {
+      const fakeEnv = { HEY: 'yo' };
+      const assembleSpy = sandbox.stub(shell, 'assembleShellEnv').returns(fakeEnv);
+      const fakeCmd = 'echo "hi"';
+      shell.runCommandSync(fakeCmd);
+      sandbox.assert.calledOnce(assembleSpy);
+      sandbox.assert.calledWithMatch(runSpy, fakeCmd, sinon.match({ shell: true, env: fakeEnv }));
+    });
+    it('should raise bubble error details up', () => {
+      runSpy.throws(new Error('this is bat country'));
+      assert.throw(() => {
+        shell.runCommandSync('about to explode');
+      }, /this is bat country/);
+    });
+  });
+
+  describe('checkIfFinished method', () => {
+    beforeEach(() => {
+    });
+    it('should resolve if underlying process raises a `close` event', (done) => {
+      const proc: ShellProcess = {
+        process: spawnProcess,
+        output: '',
+        finished: true,
+        command: 'echo "hi"',
+      };
+      shell.checkIfFinished(proc).then(done);
+      spawnProcess.emit('close', 0);
+    });
+    it('should reject if underlying process raises an `error` event', (done) => {
+      const proc: ShellProcess = {
+        process: spawnProcess,
+        output: '',
+        finished: true,
+        command: 'echo "hi"',
+      };
+      shell.checkIfFinished(proc).then(() => {
+        assert.fail('checkIfFinished resolved unexpectedly');
+      }, (err) => {
+        assert.include(err.message, 'boom');
+        done();
+      });
+      spawnProcess.emit('error', new Error('boom'));
+    });
+  });
+});

packages/cli-test/src/cli/shell.ts

@@ -1,5 +1,5 @@
-import * as child from 'child_process';
-import kill from 'tree-kill';
+import child from 'child_process';
+import treekill from 'tree-kill';
 import logger from '../utils/logger';
 import { timeouts } from '../utils/constants';
 import type { ShellProcess } from '../utils/types';
@@ -60,7 +60,7 @@
 
       return sh;
     } catch (error) {
-      throw new Error(`runCommandAsync\nFailed to run command.\nCommand: ${command}`);
+      throw new Error(`spawnProcess failed!\nCommand: ${command}\nError: ${error}`);
     }
   },
 
@@ -93,7 +93,7 @@
       // TODO: this method only returns stdout and not stderr...
       return this.removeANSIcolors(result.stdout.toString());
     } catch (error) {
-      throw new Error(`runCommandSync\nFailed to run command.\nCommand: ${command}`);
+      throw new Error(`runCommandSync failed!\nCommand: ${command}\nError: ${error}`);
     }
   },
 
@@ -108,12 +108,10 @@
       let timeout: NodeJS.Timeout;
 
       const killIt = (reason: string) => {
-        kill(proc.process.pid!, (err) => {
-          let msg = `${reason}\nCommand: ${proc.command}\nOutput: \n${proc.output}`;
-          if (err) {
-            msg += `\nAdditionally, further attempting to kill the process errored with ${err.message}`;
-          }
-          reject(new Error(msg));
+        shell.kill(proc).then(() => {
+          reject(new Error(`${reason}\nCommand: ${proc.command}, output: ${proc.output}`));
+        }, (err) => {
+          reject(new Error(`${reason}\nCommand: ${proc.command}, output: ${proc.output}\nAlso errored killing process: ${err.message}`));
         });
       };
 
@@ -127,7 +125,7 @@
         clearTimeout(timeout);
         proc.process.off('close', closeHandler);
         logger.error(`CLI Command "${proc.command}" errored with ${err}`);
-        killIt('Command raised an error!');
+        killIt(`Command raised an error: ${err.message}`);
       };
 
       // Timeout the process if necessary
@@ -189,12 +187,11 @@
     return new Promise((resolve, reject) => {
       if (timedOut) {
         // Kill the process
-        kill(proc.process.pid!, (err) => {
-          let msg = `shell.waitForOutput timed out after ${waitedFor} ms. \nExpected output to include: ${expString}\nActual: ${proc.output}`;
-          if (err) {
-            msg += `\nAdditionally, killing the process errored with ${err.message}`;
-          }
-          reject(new Error(msg));
+        const reason = `shell.waitForOutput timed out after ${waitedFor} ms. \nExpected output to include: ${expString}\nActual: ${proc.output}`;
+        shell.kill(proc).then(() => {
+          reject(new Error(`${reason}\nCommand: ${proc.command}, output: ${proc.output}`));
+        }, (err) => {
+          reject(new Error(`${reason}\nCommand: ${proc.command}, output: ${proc.output}\nAlso errored killing process: ${err.message}`));
         });
       } else {
         resolve();
@@ -211,4 +208,15 @@
     spawnedEnv.SLACK_DISABLE_TELEMETRY = 'true';
     return spawnedEnv;
   },
+  kill: async function kill(proc: ShellProcess): Promise<boolean> {
+    return new Promise((resolve, reject) => {
+      treekill(proc.process.pid!, (err) => {
+        if (err) {
+          reject(new Error(`Failed to kill command "${proc.command}": errored with ${err.message}\nOutput: ${proc.output}`));
+        } else {
+          resolve(true);
+        }
+      });
+    });
+  },
 };