Skip to content

Commit

Permalink
feat(cli): detached runner (#4260)
Browse files Browse the repository at this point in the history
  • Loading branch information
noomorph authored Nov 1, 2023
1 parent 81db382 commit 6e87dc1
Show file tree
Hide file tree
Showing 17 changed files with 167 additions and 19 deletions.
7 changes: 7 additions & 0 deletions detox/detox.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,13 @@ declare global {
* @see {DetoxInternals.DetoxTestFileReport#isPermanentFailure}
*/
bail?: boolean;
/**
* When true, tells `detox test` to spawn the test runner in a detached mode.
* This is useful in CI environments, where you want to intercept SIGINT and SIGTERM signals to gracefully shut down the test runner and the device.
* Instead of passing the kill signal to the child process (the test runner), Detox will send an emergency shutdown request to all the workers, and then it will wait for them to finish.
* @default false
*/
detached?: boolean;
/**
* Custom handler to process --inspect-brk CLI flag.
* Use it when you rely on another test runner than Jest to mutate the config.
Expand Down
3 changes: 2 additions & 1 deletion detox/internals.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,9 @@ declare global {
/**
* Workaround for Jest exiting abruptly in --bail mode.
* Makes sure that all workers and their test environments are properly torn down.
* @param [permanent] - forbids further retries
*/
unsafe_conductEarlyTeardown(): Promise<void>;
unsafe_conductEarlyTeardown(permanent?: boolean): Promise<void>;
/**
* Reports to Detox CLI about passed and failed test files.
* The failed test files might be re-run again if
Expand Down
40 changes: 40 additions & 0 deletions detox/local-cli/test.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ if (process.platform === 'win32') {

jest.mock('../src/logger/DetoxLogger');
jest.mock('./utils/jestInternals');
jest.mock('./utils/interruptListeners');

const cp = require('child_process');
const cpSpawn = cp.spawn;
Expand All @@ -18,6 +19,8 @@ const { buildMockCommand, callCli } = require('../__tests__/helpers');

const { DEVICE_LAUNCH_ARGS_DEPRECATION } = require('./testCommand/warnings');

const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms));

describe('CLI', () => {
let _env;
let logger;
Expand Down Expand Up @@ -143,6 +146,38 @@ describe('CLI', () => {
});
});

describe('detached runner', () => {
beforeEach(() => {
detoxConfig.testRunner.detached = true;
});

test('should be able to run as you would normally expect', async () => {
await run();
expect(_.last(cliCall().argv)).toEqual('e2e/config.json');
});

test('should intercept SIGINT and SIGTERM', async () => {
const { subscribe, unsubscribe } = jest.requireMock('./utils/interruptListeners');
const simulateSIGINT = () => subscribe.mock.calls[0][0]();

mockExitCode(1);
mockLongRun(2000);

await Promise.all([
run('--retries 2').catch(_.noop),
sleep(1000).then(() => {
simulateSIGINT();
simulateSIGINT();
expect(unsubscribe).not.toHaveBeenCalled();
}),
]);

expect(unsubscribe).toHaveBeenCalled();
expect(cliCall(0)).not.toBe(null);
expect(cliCall(1)).toBe(null);
});
});

test('should use testRunner.args._ as default specs', async () => {
detoxConfig.testRunner.args._ = ['e2e/sanity'];
await run();
Expand Down Expand Up @@ -620,4 +655,9 @@ describe('CLI', () => {
mockExecutable.options.exitCode = code;
detoxConfig.testRunner.args.$0 = mockExecutable.cmd;
}

function mockLongRun(ms) {
mockExecutable.options.sleep = ms;
detoxConfig.testRunner.args.$0 = mockExecutable.cmd;
}
});
29 changes: 26 additions & 3 deletions detox/local-cli/testCommand/TestRunnerCommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const { escapeSpaces, useForwardSlashes } = require('../../src/utils/shellUtils'
const sleep = require('../../src/utils/sleep');
const AppStartCommand = require('../startCommand/AppStartCommand');
const { markErrorAsLogged } = require('../utils/cliErrorHandling');
const interruptListeners = require('../utils/interruptListeners');

const TestRunnerError = require('./TestRunnerError');

Expand All @@ -28,10 +29,12 @@ class TestRunnerCommand {
const appsConfig = opts.config.apps;

this._argv = runnerConfig.args;
this._detached = runnerConfig.detached;
this._retries = runnerConfig.retries;
this._envHint = this._buildEnvHint(opts.env);
this._startCommands = this._prepareStartCommands(appsConfig, cliConfig);
this._envFwd = {};
this._terminating = false;

if (runnerConfig.forwardEnv) {
this._envFwd = this._buildEnvOverride(cliConfig, deviceConfig);
Expand Down Expand Up @@ -59,16 +62,20 @@ class TestRunnerCommand {
} catch (e) {
launchError = e;

if (this._terminating) {
runsLeft = 0;
}

const failedTestFiles = detox.session.testResults.filter(r => !r.success);

const { bail } = detox.config.testRunner;
if (bail && failedTestFiles.some(r => r.isPermanentFailure)) {
throw e;
runsLeft = 0;
}

const testFilesToRetry = failedTestFiles.filter(r => !r.isPermanentFailure).map(r => r.testFilePath);
if (_.isEmpty(testFilesToRetry)) {
throw e;
if (testFilesToRetry.length === 0) {
runsLeft = 0;
}

if (--runsLeft > 0) {
Expand Down Expand Up @@ -143,6 +150,15 @@ class TestRunnerCommand {
}, _.isUndefined);
}

_onTerminate = () => {
if (this._terminating) {
return;
}

this._terminating = true;
return detox.unsafe_conductEarlyTeardown(true);
};

async _spawnTestRunner() {
const fullCommand = this._buildSpawnArguments().map(escapeSpaces);
const fullCommandWithHint = printEnvironmentVariables(this._envHint) + fullCommand.join(' ');
Expand All @@ -153,6 +169,7 @@ class TestRunnerCommand {
cp.spawn(fullCommand[0], fullCommand.slice(1), {
shell: true,
stdio: 'inherit',
detached: this._detached,
env: _({})
.assign(process.env)
.assign(this._envFwd)
Expand All @@ -162,6 +179,8 @@ class TestRunnerCommand {
})
.on('error', /* istanbul ignore next */ (err) => reject(err))
.on('exit', (code, signal) => {
interruptListeners.unsubscribe(this._onTerminate);

if (code === 0) {
log.trace.end({ success: true });
resolve();
Expand All @@ -175,6 +194,10 @@ class TestRunnerCommand {
reject(markErrorAsLogged(error));
}
});

if (this._detached) {
interruptListeners.subscribe(this._onTerminate);
}
});
}

Expand Down
15 changes: 15 additions & 0 deletions detox/local-cli/utils/interruptListeners.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
function subscribe(listener) {
process.on('SIGINT', listener);
process.on('SIGTERM', listener);
}

function unsubscribe(listener) {
process.removeListener('SIGINT', listener);
process.removeListener('SIGTERM', listener);
}

module.exports = {
subscribe,
unsubscribe,
};

8 changes: 7 additions & 1 deletion detox/runners/jest/testEnvironment/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ class DetoxCircusEnvironment extends NodeEnvironment {
// @ts-expect-error TS2425
async handleTestEvent(event, state) {
if (detox.session.unsafe_earlyTeardown) {
throw new Error('Detox halted test execution due to an early teardown request');
if (event.name === 'test_fn_start' || event.name === 'hook_start') {
throw new Error('Detox halted test execution due to an early teardown request');
}
}

this._timer.schedule(state.testTimeout != null ? state.testTimeout : this.setupTimeout);
Expand Down Expand Up @@ -107,6 +109,10 @@ class DetoxCircusEnvironment extends NodeEnvironment {
* @protected
*/
async initDetox() {
if (detox.session.unsafe_earlyTeardown) {
throw new Error('Detox halted test execution due to an early teardown request');
}

const opts = {
global: this.global,
workerId: `w${process.env.JEST_WORKER_ID}`,
Expand Down
4 changes: 3 additions & 1 deletion detox/src/configuration/composeRunnerConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ function composeRunnerConfig(opts) {
retries: 0,
inspectBrk: inspectBrkHookDefault,
forwardEnv: false,
detached: false,
bail: false,
jest: {
setupTimeout: 300000,
Expand All @@ -56,8 +57,9 @@ function composeRunnerConfig(opts) {

if (typeof merged.inspectBrk === 'function') {
if (cliConfig.inspectBrk) {
merged.retries = 0;
merged.detached = false;
merged.forwardEnv = true;
merged.retries = 0;
merged.inspectBrk(merged);
}

Expand Down
8 changes: 8 additions & 0 deletions detox/src/configuration/composeRunnerConfig.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ describe('composeRunnerConfig', () => {
},
retries: 0,
bail: false,
detached: false,
forwardEnv: false,
});
});
Expand All @@ -60,6 +61,7 @@ describe('composeRunnerConfig', () => {
},
bail: true,
retries: 1,
detached: true,
forwardEnv: true,
};

Expand All @@ -77,6 +79,7 @@ describe('composeRunnerConfig', () => {
},
bail: true,
retries: 1,
detached: true,
forwardEnv: true,
});
});
Expand All @@ -92,6 +95,7 @@ describe('composeRunnerConfig', () => {
},
bail: true,
retries: 1,
detached: true,
forwardEnv: true,
};

Expand All @@ -109,6 +113,7 @@ describe('composeRunnerConfig', () => {
},
bail: true,
retries: 1,
detached: true,
forwardEnv: true,
});
});
Expand Down Expand Up @@ -222,6 +227,7 @@ describe('composeRunnerConfig', () => {
reportSpecs: true,
},
bail: true,
detached: true,
retries: 1,
};

Expand All @@ -236,6 +242,7 @@ describe('composeRunnerConfig', () => {
reportSpecs: false,
},
bail: false,
detached: false,
retries: 3,
};

Expand All @@ -256,6 +263,7 @@ describe('composeRunnerConfig', () => {
reportWorkerAssign: true,
},
bail: false,
detached: false,
retries: 3,
forwardEnv: false,
});
Expand Down
4 changes: 2 additions & 2 deletions detox/src/ipc/IPCClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ class IPCClient {
this._sessionState.patch(sessionState);
}

async conductEarlyTeardown() {
const sessionState = await this._emit('conductEarlyTeardown', {});
async conductEarlyTeardown({ permanent }) {
const sessionState = await this._emit('conductEarlyTeardown', { permanent });
this._sessionState.patch(sessionState);
}

Expand Down
8 changes: 5 additions & 3 deletions detox/src/ipc/IPCServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ class IPCServer {
this._ipc.server.emit(socket, 'registerContextDone', {
testResults: this._sessionState.testResults,
testSessionIndex: this._sessionState.testSessionIndex,
unsafe_earlyTeardown: this._sessionState.unsafe_earlyTeardown,
});
}

Expand All @@ -90,10 +91,11 @@ class IPCServer {
}
}

onConductEarlyTeardown(_data = null, socket = null) {
// Note that we don't save `unsafe_earlyTeardown` in the primary session state
// because it's transient and needed only to make the workers quit early.
onConductEarlyTeardown({ permanent }, socket = null) {
const newState = { unsafe_earlyTeardown: true };
if (permanent) {
Object.assign(this._sessionState, newState);
}

if (socket) {
this._ipc.server.emit(socket, 'conductEarlyTeardownDone', newState);
Expand Down
34 changes: 32 additions & 2 deletions detox/src/ipc/ipc.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,36 @@ describe('IPC', () => {
});
});

describe('conductEarlyTeardown', () => {
beforeEach(() => ipcServer.init());

describe('(permanent)', () => {
beforeEach(() => ipcServer.onConductEarlyTeardown({ permanent: true }));

it('should change the session state', async () => {
expect(ipcServer.sessionState.unsafe_earlyTeardown).toEqual(true);
});

it('should pass the session state to the client', async () => {
await ipcClient1.init();
expect(ipcClient1.sessionState.unsafe_earlyTeardown).toEqual(true);
});
});

describe('(transient)', () => {
beforeEach(() => ipcServer.onConductEarlyTeardown({ permanent: false }));

it('should not change the session state', async () => {
expect(ipcServer.sessionState.unsafe_earlyTeardown).toBe(undefined);
});

it('should not pass the session state to the client', async () => {
await ipcClient1.init();
expect(ipcClient1.sessionState.unsafe_earlyTeardown).toBe(undefined);
});
});
});

describe('dispose()', () => {
it('should resolve if there are no connected clients', async () => {
await ipcServer.init();
Expand Down Expand Up @@ -278,7 +308,7 @@ describe('IPC', () => {
expect(ipcClient1.sessionState).toEqual(expect.objectContaining({ unsafe_earlyTeardown: undefined }));
expect(ipcClient2.sessionState).toEqual(expect.objectContaining({ unsafe_earlyTeardown: undefined }));

await ipcClient1.conductEarlyTeardown();
await ipcClient1.conductEarlyTeardown({ permanent: false });
expect(ipcClient1.sessionState).toEqual(expect.objectContaining({ unsafe_earlyTeardown: true }));
await sleep(10); // broadcasting might happen with a delay
expect(ipcClient2.sessionState).toEqual(expect.objectContaining({ unsafe_earlyTeardown: true }));
Expand All @@ -287,7 +317,7 @@ describe('IPC', () => {
it('should broadcast early teardown in all connected clients (from server)', async () => {
expect(ipcClient1.sessionState).toEqual(expect.objectContaining({ unsafe_earlyTeardown: undefined }));
expect(ipcClient2.sessionState).toEqual(expect.objectContaining({ unsafe_earlyTeardown: undefined }));
await ipcServer.onConductEarlyTeardown();
await ipcServer.onConductEarlyTeardown({ permanent: false });
await sleep(10); // broadcasting might happen with a delay
expect(ipcClient1.sessionState).toEqual(expect.objectContaining({ unsafe_earlyTeardown: true }));
expect(ipcClient2.sessionState).toEqual(expect.objectContaining({ unsafe_earlyTeardown: true }));
Expand Down
2 changes: 1 addition & 1 deletion detox/src/realms/DetoxContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ class DetoxContext {
/** @abstract */
[symbols.reportTestResults](_testResults) {}
/** @abstract */
[symbols.conductEarlyTeardown]() {}
[symbols.conductEarlyTeardown](_permanent) {}
/**
* @abstract
* @param {Partial<DetoxInternals.DetoxInitOptions>} _opts
Expand Down
Loading

0 comments on commit 6e87dc1

Please sign in to comment.