From ca970abfdd429a62264ff8f834c3582ce6449fa1 Mon Sep 17 00:00:00 2001 From: peternhale Date: Thu, 16 Feb 2023 09:30:09 -0700 Subject: [PATCH] fix: always return stdout and stderr from execCmd (#406) * fix: always return stdout and stderr from execCmd @W-12556866@ * fix: files go in session/project * refactor: unused vars, guaranteed cwd on options * fix: absolute paths for file locations * test: ut assert absolute paths --------- Co-authored-by: mshanemc --- src/execCmd.ts | 35 ++++++++++++++++++----------------- test/unit/execCmd.test.ts | 16 +++++++++------- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/src/execCmd.ts b/src/execCmd.ts index 29e2a126..a01b9c5b 100644 --- a/src/execCmd.ts +++ b/src/execCmd.ts @@ -34,7 +34,10 @@ type BaseExecOptions = { cli?: CLI; }; -export type ExecCmdOptions = ExecOptions & BaseExecOptions; +export type ExecCmdOptions = ExecOptions & + BaseExecOptions & + // prevent the overriding of our default by passing in an explicitly undefined value for cwd + ({ cwd: string } | { cwd?: never }); // eslint-disable-next-line @typescript-eslint/no-explicit-any type ExcludeMethods = Pick any ? never : K }[keyof T]>>; @@ -63,7 +66,7 @@ export interface ExecCmdResult { execCmdDuration: Duration; } -const buildCmdOptions = (options?: ExecCmdOptions): ExecCmdOptions => { +const buildCmdOptions = (options?: ExecCmdOptions): ExecCmdOptions & { cwd: string } => { const defaults: ExecCmdOptions = { env: { ...process.env, ...options?.env }, cwd: process.cwd(), @@ -155,8 +158,8 @@ const execCmdSync = (cmd: string, options?: ExecCmdOptions): ExecCmdResult const stdoutFile = `${genUniqueString('stdout')}.txt`; const stderrFile = `${genUniqueString('stderr')}.txt`; - const stdoutFileLocation = pathJoin(process.cwd(), stdoutFile); - const stderrFileLocation = pathJoin(process.cwd(), stderrFile); + const stdoutFileLocation = pathJoin(cmdOptions.cwd, stdoutFile); + const stderrFileLocation = pathJoin(cmdOptions.cwd, stderrFile); const result = { shellOutput: new ShellString(''), @@ -167,14 +170,13 @@ const execCmdSync = (cmd: string, options?: ExecCmdOptions): ExecCmdResult const startTime = process.hrtime(); const code = (shelljs.exec(`${cmd} 1> ${stdoutFile} 2> ${stderrFile} `, cmdOptions) as ShellString).code; - if (code === 0) { - result.shellOutput = new ShellString(stripAnsi(fs.readFileSync(stdoutFileLocation, 'utf-8'))); - result.shellOutput.stdout = stripAnsi(result.shellOutput.stdout); - } else { - result.shellOutput = new ShellString(stripAnsi(fs.readFileSync(stderrFileLocation, 'utf-8'))); - // The ShellString constructor sets the argument as stdout, so we strip 'stdout' and set as stderr - result.shellOutput.stderr = stripAnsi(result.shellOutput.stdout); - } + // capture the output for both stdout and stderr + result.shellOutput = new ShellString(stripAnsi(fs.readFileSync(stdoutFileLocation, 'utf-8'))); + result.shellOutput.stdout = stripAnsi(result.shellOutput.stdout); + const shellStringForStderr = new ShellString(stripAnsi(fs.readFileSync(stderrFileLocation, 'utf-8'))); + // The ShellString constructor sets the argument as stdout, so we strip 'stdout' and set as stderr + result.shellOutput.stderr = stripAnsi(shellStringForStderr.stdout); + result.shellOutput.code = code; result.execCmdDuration = hrtimeToMillisDuration(process.hrtime(startTime)); @@ -201,10 +203,9 @@ const execCmdAsync = async (cmd: string, options: ExecCmdOptions): Promise { const execCmdDuration = hrtimeToMillisDuration(process.hrtime(startTime)); debug(`Command completed with exit code: ${code}`); @@ -238,7 +239,7 @@ const execCmdAsync = async (cmd: string, options: ExecCmdOptions): Promise ${stdoutFile} 2> ${stderrFile}`, cmdOptions, callback); + shelljs.exec(`${cmd} 1> ${stdoutFileLocation} 2> ${stderrFileLocation}`, cmdOptions, callback); }); }; diff --git a/test/unit/execCmd.test.ts b/test/unit/execCmd.test.ts index 7ce82e5f..a63e8301 100644 --- a/test/unit/execCmd.test.ts +++ b/test/unit/execCmd.test.ts @@ -6,7 +6,7 @@ */ import * as fs from 'fs'; import { join } from 'path'; -import { expect, assert } from 'chai'; +import { expect, assert, config } from 'chai'; import * as sinon from 'sinon'; import { Duration, env } from '@salesforce/kit'; import { stubMethod } from '@salesforce/ts-sinon'; @@ -15,6 +15,8 @@ import { ShellString } from 'shelljs'; import { beforeEach } from 'mocha'; import { execCmd } from '../../src'; +config.truncateThreshold = 0; + describe('execCmd (sync)', () => { const sandbox = sinon.createSandbox(); const cmd = 'force:user:create -a testuser1'; @@ -235,8 +237,8 @@ describe('execCmd (async)', () => { const execStub = stubMethod(sandbox, shelljs, 'exec').yields(0, shellString, ''); await execCmd(cmd, { async: true }); expect(execStub.args[0][0]).to.include(`${binPath} ${cmd}`); - expect(execStub.args[0][0]).to.include('1> stdout'); - expect(execStub.args[0][0]).to.include('2> stderr'); + expect(execStub.args[0][0]).to.match(/1> .*stdout.*txt/); + expect(execStub.args[0][0]).to.match(/2> .*stderr.*txt/); }); it('should accept valid sfdx path in env var', async () => { @@ -249,8 +251,8 @@ describe('execCmd (async)', () => { const execStub = stubMethod(sandbox, shelljs, 'exec').yields(0, shellString, ''); await execCmd(cmd, { async: true }); expect(execStub.args[0][0]).to.include(`${binPath} ${cmd}`); - expect(execStub.args[0][0]).to.include('1> stdout'); - expect(execStub.args[0][0]).to.include('2> stderr'); + expect(execStub.args[0][0]).to.match(/1> .*stdout.*txt/); + expect(execStub.args[0][0]).to.match(/2> .*stderr.*txt/); }); it('should accept valid executable in the system path', async () => { @@ -261,8 +263,8 @@ describe('execCmd (async)', () => { const execStub = stubMethod(sandbox, shelljs, 'exec').yields(0, shellString, ''); await execCmd(cmd, { async: true }); expect(execStub.args[0][0]).to.include(`${binPath} ${cmd}`); - expect(execStub.args[0][0]).to.include('1> stdout'); - expect(execStub.args[0][0]).to.include('2> stderr'); + expect(execStub.args[0][0]).to.match(/1> .*stdout.*txt/); + expect(execStub.args[0][0]).to.match(/2> .*stderr.*txt/); }); it('should error when executable path not found', async () => {