From 11cb80e56d15491a228c25fce1d9b340da24204b Mon Sep 17 00:00:00 2001 From: Fil Maj Date: Mon, 27 May 2024 18:57:59 +0000 Subject: [PATCH] cli-test: add `app list` command (#1794) --- packages/cli-test/src/cli/commands/app.ts | 19 ++++ .../cli-test/src/cli/commands/platform.ts | 27 +++--- packages/cli-test/src/cli/index.ts | 6 +- packages/cli-test/src/cli/shell.spec.ts | 93 +++++++++++++++++++ packages/cli-test/src/cli/shell.ts | 42 +++++---- 5 files changed, 153 insertions(+), 34 deletions(-) create mode 100644 packages/cli-test/src/cli/shell.spec.ts diff --git a/packages/cli-test/src/cli/commands/app.ts b/packages/cli-test/src/cli/commands/app.ts index 1a76b5798..4532b5574 100644 --- a/packages/cli-test/src/cli/commands/app.ts +++ b/packages/cli-test/src/cli/commands/app.ts @@ -46,8 +46,27 @@ export const install = async function workspaceInstall(appPath: string, teamFlag } }; +/** + * `slack app list` + * @param appPath path to app + * @returns command output + */ +export const list = async function appList(appPath: string): Promise { + // 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, }; diff --git a/packages/cli-test/src/cli/commands/platform.ts b/packages/cli-test/src/cli/commands/platform.ts index 63a7263d2..7bac474ca 100644 --- a/packages/cli-test/src/cli/commands/platform.ts +++ b/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 @@ export default { // 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,12 +189,8 @@ export default { // 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 @@ -205,6 +198,10 @@ export default { } else { resolve(); } + }, (err) => { + const msg = `runStop command failed to kill process: ${err}`; + logger.warn(msg); + reject(new Error(msg)); }); }); }, diff --git a/packages/cli-test/src/cli/index.ts b/packages/cli-test/src/cli/index.ts index 9c1f3ba66..3144aeefc 100644 --- a/packages/cli-test/src/cli/index.ts +++ b/packages/cli-test/src/cli/index.ts @@ -21,6 +21,7 @@ export const SlackCLI = { app: { delete: appCommands.workspaceDelete, install: appCommands.workspaceInstall, + list: appCommands.workspaceList, }, ...authCommands, auth: authCommands, @@ -71,12 +72,13 @@ export const SlackCLI = { // 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 { 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 @@ export const SlackCLI = { 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 diff --git a/packages/cli-test/src/cli/shell.spec.ts b/packages/cli-test/src/cli/shell.spec.ts new file mode 100644 index 000000000..2adefa2f6 --- /dev/null +++ b/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; + + 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')); + }); + }); +}); diff --git a/packages/cli-test/src/cli/shell.ts b/packages/cli-test/src/cli/shell.ts index 2209522fe..7df1f7496 100644 --- a/packages/cli-test/src/cli/shell.ts +++ b/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 @@ export const shell = { 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 @@ export const shell = { // 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 @@ export const shell = { 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 @@ export const shell = { 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 @@ export const shell = { 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 @@ export const shell = { spawnedEnv.SLACK_DISABLE_TELEMETRY = 'true'; return spawnedEnv; }, + kill: async function kill(proc: ShellProcess): Promise { + 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); + } + }); + }); + }, };