Skip to content

Commit

Permalink
fix: always return stdout and stderr from execCmd (#406)
Browse files Browse the repository at this point in the history 
* 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 <[email protected]>
  • Loading branch information
@peternhale @mshanemc
peternhale and mshanemc authored Feb 16, 2023
1 parent 801f19d commit ca970ab
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 24 deletions.
35 changes: 18 additions & 17 deletions src/execCmd.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> = Pick<T, NonNullable<{ [K in keyof T]: T[K] extends (_: any) => any ? never : K }[keyof T]>>;
Expand Down Expand Up @@ -63,7 +66,7 @@ export interface ExecCmdResult<T> {
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(),
Expand Down Expand Up @@ -155,8 +158,8 @@ const execCmdSync = <T>(cmd: string, options?: ExecCmdOptions): ExecCmdResult<T>

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(''),
Expand All @@ -167,14 +170,13 @@ const execCmdSync = <T>(cmd: string, options?: ExecCmdOptions): ExecCmdResult<T>
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));
Expand All @@ -201,10 +203,9 @@ const execCmdAsync = async <T>(cmd: string, options: ExecCmdOptions): Promise<Ex

debug(`Running cmd: ${cmd}`);
debug(`Cmd options: ${inspect(cmdOptions)}`);
const stdoutFile = `${genUniqueString('stdout')}.txt`;
const stderrFile = `${genUniqueString('stderr')}.txt`;
const stdoutFileLocation = pathJoin(process.cwd(), stdoutFile);
const stderrFileLocation = pathJoin(process.cwd(), stderrFile);
// buildCmdOptions will always
const stdoutFileLocation = pathJoin(cmdOptions.cwd, `${genUniqueString('stdout')}.txt`);
const stderrFileLocation = pathJoin(cmdOptions.cwd, `${genUniqueString('stderr')}.txt`);
const callback: ExecCallback = (code, stdout, stderr) => {
const execCmdDuration = hrtimeToMillisDuration(process.hrtime(startTime));
debug(`Command completed with exit code: ${code}`);
Expand Down Expand Up @@ -238,7 +239,7 @@ const execCmdAsync = async <T>(cmd: string, options: ExecCmdOptions): Promise<Ex
};
// Execute the command async in a child process
const startTime = process.hrtime();
shelljs.exec(`${cmd} 1> ${stdoutFile} 2> ${stderrFile}`, cmdOptions, callback);
shelljs.exec(`${cmd} 1> ${stdoutFileLocation} 2> ${stderrFileLocation}`, cmdOptions, callback);
});
};

Expand Down
16 changes: 9 additions & 7 deletions test/unit/execCmd.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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';
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand Down

0 comments on commit ca970ab

Please sign in to comment.