Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SCANNPM-39 exit parent process w correct exit code #153

Merged
merged 5 commits into from
Jul 19, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/scan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export async function scan(scanOptions: ScanOptions, cliArgs?: CliArgs) {
await runScan(scanOptions, cliArgs);
} catch (error) {
log(LogLevel.ERROR, `An error occurred: ${error}`);
process.exit(1);
lucas-paulger-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
14 changes: 10 additions & 4 deletions src/scanner-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,17 @@ export function runScannerEngine(

return new Promise<void>((resolve, reject) => {
child.on('exit', code => {
if (code === 0) {
log(LogLevel.DEBUG, 'Scanner engine finished successfully');
resolve();
if (typeof code === 'number') {
lucas-paulger-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
if (code === 0) {
log(LogLevel.DEBUG, 'Scanner engine finished successfully');
resolve();
} else {
reject(new Error(`Scanner engine failed with code ${code}`));
process.exit(code);
7PH marked this conversation as resolved.
Show resolved Hide resolved
}
} else {
reject(new Error(`Scanner engine failed with code ${code}`));
reject(new Error('Scanner engine exited with an unexpected state.'));
process.exit(1);
}
});
});
Expand Down
4 changes: 2 additions & 2 deletions test/unit/mocks/ChildProcessMock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import { ChildProcess, exec, spawn } from 'child_process';

export class ChildProcessMock {
private exitCode: number = 0;
private exitCode: number | null = 0;

private stdout: string = '';
private stderr: string = '';
Expand All @@ -34,7 +34,7 @@ export class ChildProcessMock {
jest.mocked(exec).mockImplementation((this.handleExec as any).bind(this));
}

setExitCode(exitCode: number) {
setExitCode(exitCode: number | null) {
this.exitCode = exitCode;
}

Expand Down
16 changes: 10 additions & 6 deletions test/unit/scan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ jest.mock('../../src/logging', () => ({
import * as java from '../../src/java';
import * as logging from '../../src/logging';
import * as platform from '../../src/platform';
import * as process from '../../src/process';
import * as sonarProcess from '../../src/process';
import { scan } from '../../src/scan';
import * as scannerCli from '../../src/scanner-cli';
import * as scannerEngine from '../../src/scanner-engine';
Expand Down Expand Up @@ -96,7 +96,7 @@ describe('scan', () => {
jest.spyOn(java, 'serverSupportsJREProvisioning').mockResolvedValue(false);
jest.spyOn(scannerEngine, 'runScannerEngine');
jest.spyOn(scannerCli, 'runScannerCli');
jest.spyOn(process, 'locateExecutableFromPath').mockResolvedValue('/bin/sonar-scanner');
jest.spyOn(sonarProcess, 'locateExecutableFromPath').mockResolvedValue('/bin/sonar-scanner');

await scan({ serverUrl: 'http://localhost:9000', localScannerCli: true });

Expand All @@ -107,10 +107,11 @@ describe('scan', () => {
});

it('should fail if local scanner is requested but not found', async () => {
jest.spyOn(process, 'exit').mockImplementation();
jest.spyOn(java, 'serverSupportsJREProvisioning').mockResolvedValue(false);
jest.spyOn(scannerEngine, 'runScannerEngine');
jest.spyOn(scannerCli, 'runScannerCli');
jest.spyOn(process, 'locateExecutableFromPath').mockResolvedValue(null);
jest.spyOn(sonarProcess, 'locateExecutableFromPath').mockResolvedValue(null);

await scan({ serverUrl: 'http://localhost:9000', localScannerCli: true });

Expand All @@ -120,6 +121,7 @@ describe('scan', () => {
logging.LogLevel.ERROR,
expect.stringMatching(/SonarScanner CLI not found in PATH/),
);
expect(process.exit).toHaveBeenCalledWith(1);
});
});

Expand Down Expand Up @@ -155,7 +157,7 @@ describe('scan', () => {
jest.spyOn(java, 'serverSupportsJREProvisioning').mockResolvedValue(true);
jest.spyOn(java, 'fetchJRE');
jest.spyOn(scannerEngine, 'runScannerEngine');
jest.spyOn(process, 'locateExecutableFromPath').mockResolvedValue('/usr/bin/java');
jest.spyOn(sonarProcess, 'locateExecutableFromPath').mockResolvedValue('/usr/bin/java');

await scan({
serverUrl: 'http://localhost:9000',
Expand All @@ -165,16 +167,17 @@ describe('scan', () => {
});

expect(java.fetchJRE).not.toHaveBeenCalled();
expect(process.locateExecutableFromPath).toHaveBeenCalled();
expect(sonarProcess.locateExecutableFromPath).toHaveBeenCalled();
const [javaPath] = (scannerEngine.runScannerEngine as jest.Mock).mock.calls.pop();
expect(javaPath).toBe('/usr/bin/java');
});

it('should fail when skipping JRE provisioning without java in PATH', async () => {
jest.spyOn(process, 'exit').mockImplementation();
jest.spyOn(java, 'serverSupportsJREProvisioning').mockResolvedValue(true);
jest.spyOn(java, 'fetchJRE');
jest.spyOn(scannerEngine, 'runScannerEngine');
jest.spyOn(process, 'locateExecutableFromPath').mockResolvedValue(null);
jest.spyOn(sonarProcess, 'locateExecutableFromPath').mockResolvedValue(null);

await scan({
serverUrl: 'http://localhost:9000',
Expand All @@ -189,6 +192,7 @@ describe('scan', () => {
logging.LogLevel.ERROR,
expect.stringMatching(/Java not found in PATH/),
);
expect(process.exit).toHaveBeenCalledWith(1);
});
});
});
25 changes: 23 additions & 2 deletions test/unit/scanner-engine.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,10 @@ describe('scanner-engine', () => {
]);
});

it('should reject when child process exits with code 1', async () => {
childProcessHandler.setExitCode(1);
it('should exit process (with same code) when child process exits with code 1', async () => {
const CHILD_PROCESS_EXIT_CODE = 1;
const mockExit = jest.spyOn(process, 'exit').mockImplementation();
childProcessHandler.setExitCode(CHILD_PROCESS_EXIT_CODE);

await expect(
runScannerEngine(
Expand All @@ -196,6 +198,24 @@ describe('scanner-engine', () => {
MOCKED_PROPERTIES,
),
).rejects.toBeInstanceOf(Error);

expect(mockExit).toHaveBeenCalledWith(CHILD_PROCESS_EXIT_CODE);
});

it('should exit with code 1 when the child process exits with an unexpected state', async () => {
const mockExit = jest.spyOn(process, 'exit').mockImplementation();
childProcessHandler.setExitCode(null);

await expect(
runScannerEngine(
'/some/path/to/java',
'/some/path/to/scanner-engine',
{},
MOCKED_PROPERTIES,
),
).rejects.toBeInstanceOf(Error);

expect(mockExit).toHaveBeenCalledWith(1);
});

it('should output scanner engine output', async () => {
Expand Down Expand Up @@ -231,6 +251,7 @@ describe('scanner-engine', () => {
});

it('should dump data to file when dumpToFile property is set', async () => {
jest.spyOn(process, 'exit').mockImplementation();
childProcessHandler.setExitCode(1); // Make it so the scanner would fail
const writeFile = jest.spyOn(fsExtra.promises, 'writeFile').mockResolvedValue();

Expand Down