Skip to content

Commit

Permalink
fix: re-enable spawnCommandAsync tests
Browse files Browse the repository at this point in the history
  • Loading branch information
maliroteh-sf committed May 30, 2024
1 parent 0539201 commit 8d5a1da
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 28 deletions.
24 changes: 20 additions & 4 deletions src/common/CommonUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -270,10 +270,7 @@ export class CommonUtils {

CommonUtils.logger.debug(`Executing command: '${fullCommand}'`);

const prc = childProcess.spawn(command, args, {
shell: true,
stdio: stdioOptions
});
const prc = CommonUtils.spawnWrapper(command, args, stdioOptions);

prc.stdout?.on('data', (data) => {
capturedStdout.push(data.toString());
Expand Down Expand Up @@ -305,6 +302,25 @@ export class CommonUtils {
});
}

/**
* Used as a wrapper to child_process spawn function solely for unit testing purposes
*
* @param command the command to be forwarded to child_process spawn function
* @param args the arguments to be forwarded to child_process spawn function
* @param stdioOptions the options to be forwarded to child_process spawn function
* @returns the process that is returned as the result of a call to child_process spawn function
*/
public static spawnWrapper(
command: string,
args: string[] = [],
stdioOptions: StdioOptions = ['ignore', 'pipe', 'ignore']
): childProcess.ChildProcess {
return childProcess.spawn(command, args, {
shell: true,
stdio: stdioOptions
});
}

/**
* Launches the desktop browser and navigates to the provided URL.
*
Expand Down
62 changes: 38 additions & 24 deletions test/unit/common/CommonUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import http from 'node:http';
import https from 'node:https';
import path from 'node:path';
import stream from 'node:stream';
import { Messages } from '@salesforce/core';
import * as childProcess from 'node:child_process';
import { Logger, Messages } from '@salesforce/core';
import { TestContext } from '@salesforce/core/testSetup';
import { stubMethod } from '@salesforce/ts-sinon';
import { expect } from 'chai';
Expand Down Expand Up @@ -281,37 +282,50 @@ describe('CommonUtils', () => {
]);
});

// Disabling this test for now. It runs & passes locally but it fails
// in CI b/c the child process errors out with `read ENOTCONN` error.
/* it('spawnCommandAsync', async () => {
const fakeProcess = new ChildProcess();
fakeProcess.stdout = process.stdout;
fakeProcess.stderr = process.stderr;
const mockSpawn = jest.fn((): ChildProcess => fakeProcess);
jest.spyOn(cp, 'spawn').mockImplementation(mockSpawn);
it('spawnCommandAsync handles when child process completes successfully', async () => {
const fakeProcess = new childProcess.ChildProcess();
const spawnStub = stubMethod($$.SANDBOX, CommonUtils, 'spawnWrapper').returns(fakeProcess);
const errorLoggerMock = stubMethod($$.SANDBOX, Logger.prototype, 'error');

setTimeout(() => {
fakeProcess.stdout?.push('Test STDOUT');
fakeProcess.stderr?.push('Test STDERR');
fakeProcess.emit('close', 0);
fakeProcess.kill(0);
}, 2000);
}, 100);

await CommonUtils.spawnCommandAsync('somecommand', ['arg1', 'arg2'], ['ignore', 'inherit', 'pipe']);

const results = await CommonUtils.spawnCommandAsync(
'cmd',
expect(spawnStub.calledOnce);
expect(spawnStub.getCall(0).args).to.deep.equal([
'somecommand',
['arg1', 'arg2'],
['ignore', 'inherit', 'pipe']
);
]);
expect(errorLoggerMock.called).to.be.false;
});

expect(mockSpawn).toHaveBeenCalledWith('cmd', ['arg1', 'arg2'], {
shell: true,
stdio: ['ignore', 'inherit', 'pipe']
});
it('spawnCommandAsync handles when child process completes with error', async () => {
const fakeProcess = new childProcess.ChildProcess();
const spawnStub = stubMethod($$.SANDBOX, CommonUtils, 'spawnWrapper').returns(fakeProcess);
const errorLoggerMock = stubMethod($$.SANDBOX, Logger.prototype, 'error');

setTimeout(() => {
fakeProcess.emit('close', 123);
}, 100);

expect(results.stdout).toBe('Test STDOUT');
expect(results.stderr).toBe('Test STDERR');
});*/
try {
await CommonUtils.spawnCommandAsync('somecommand', ['arg1', 'arg2'], ['ignore', 'inherit', 'pipe']);
} catch (error) {
expect(error).to.be.an('error');
}

expect(spawnStub.calledOnce);
expect(spawnStub.getCall(0).args).to.deep.equal([
'somecommand',
['arg1', 'arg2'],
['ignore', 'inherit', 'pipe']
]);
expect(errorLoggerMock.called).to.be.true;
expect(errorLoggerMock.getCall(0).args[0]).to.contain('somecommand arg1 arg2');
});

function createStats(isFile: boolean): fs.Stats {
return {
Expand Down

0 comments on commit 8d5a1da

Please sign in to comment.