fix(test runner): when worker exits unexpectedly, fail a single test (#14608)

All remaining tests will continue in a new worker.
This commit is contained in:
Dmitry Gozman 2022-06-02 21:13:47 -07:00 committed by GitHub
parent 9f57ee337a
commit 79d356f0cc
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 25 additions and 11 deletions

View file

@ -40,6 +40,13 @@ type TestData = {
test: TestCase; test: TestCase;
resultByWorkerIndex: Map<number, TestResultData>; resultByWorkerIndex: Map<number, TestResultData>;
}; };
type WorkerExitData = {
unexpectedly: boolean;
code: number | null;
signal: NodeJS.Signals | null;
};
export class Dispatcher { export class Dispatcher {
private _workerSlots: { busy: boolean, worker?: Worker }[] = []; private _workerSlots: { busy: boolean, worker?: Worker }[] = [];
private _queue: TestGroup[] = []; private _queue: TestGroup[] = [];
@ -271,7 +278,7 @@ export class Dispatcher {
}; };
worker.on('stepEnd', onStepEnd); worker.on('stepEnd', onStepEnd);
const onDone = (params: DonePayload) => { const onDone = (params: DonePayload & { unexpectedExitError?: TestError }) => {
this._queuedOrRunningHashCount.set(worker.hash(), this._queuedOrRunningHashCount.get(worker.hash())! - 1); this._queuedOrRunningHashCount.set(worker.hash(), this._queuedOrRunningHashCount.get(worker.hash())! - 1);
let remaining = [...remainingByTestId.values()]; let remaining = [...remainingByTestId.values()];
@ -279,7 +286,7 @@ export class Dispatcher {
// - there are no remaining // - there are no remaining
// - we are here not because something failed // - we are here not because something failed
// - no unrecoverable worker error // - no unrecoverable worker error
if (!remaining.length && !failedTestIds.size && !params.fatalErrors.length && !params.skipTestsDueToSetupFailure.length && !params.fatalUnknownTestIds) { if (!remaining.length && !failedTestIds.size && !params.fatalErrors.length && !params.skipTestsDueToSetupFailure.length && !params.fatalUnknownTestIds && !params.unexpectedExitError) {
if (this._isWorkerRedundant(worker)) if (this._isWorkerRedundant(worker))
worker.stop(); worker.stop();
doneWithJob(); doneWithJob();
@ -289,7 +296,7 @@ export class Dispatcher {
// When worker encounters error, we will stop it and create a new one. // When worker encounters error, we will stop it and create a new one.
worker.stop(true /* didFail */); worker.stop(true /* didFail */);
const massSkipTestsFromRemaining = (testIds: Set<string>, errors: TestError[]) => { const massSkipTestsFromRemaining = (testIds: Set<string>, errors: TestError[], onlyStartedTests?: boolean) => {
remaining = remaining.filter(test => { remaining = remaining.filter(test => {
if (!testIds.has(test._id)) if (!testIds.has(test._id))
return true; return true;
@ -301,6 +308,8 @@ export class Dispatcher {
if (runData) { if (runData) {
result = runData.result; result = runData.result;
} else { } else {
if (onlyStartedTests)
return true;
result = data.test._appendTestResult(); result = data.test._appendTestResult();
this._reporter.onTestBegin?.(test, result); this._reporter.onTestBegin?.(test, result);
} }
@ -336,6 +345,9 @@ export class Dispatcher {
} }
// Handle tests that should be skipped because of the setup failure. // Handle tests that should be skipped because of the setup failure.
massSkipTestsFromRemaining(new Set(params.skipTestsDueToSetupFailure), []); massSkipTestsFromRemaining(new Set(params.skipTestsDueToSetupFailure), []);
// Handle unexpected worker exit.
if (params.unexpectedExitError)
massSkipTestsFromRemaining(new Set(remaining.map(test => test._id)), [params.unexpectedExitError], true /* onlyStartedTests */);
const retryCandidates = new Set<string>(); const retryCandidates = new Set<string>();
const serialSuitesWithFailures = new Set<Suite>(); const serialSuitesWithFailures = new Set<Suite>();
@ -396,8 +408,9 @@ export class Dispatcher {
}; };
worker.on('done', onDone); worker.on('done', onDone);
const onExit = (expectedly: boolean) => { const onExit = (data: WorkerExitData) => {
onDone({ skipTestsDueToSetupFailure: [], fatalErrors: expectedly ? [] : [{ value: 'Worker process exited unexpectedly' }] }); const unexpectedExitError = data.unexpectedly ? { value: `Worker process exited unexpectedly (code=${data.code}, signal=${data.signal})` } : undefined;
onDone({ skipTestsDueToSetupFailure: [], fatalErrors: [], unexpectedExitError });
}; };
worker.on('exit', onExit); worker.on('exit', onExit);
@ -492,9 +505,9 @@ class Worker extends EventEmitter {
// Can't pipe since piping slows down termination for some reason. // Can't pipe since piping slows down termination for some reason.
stdio: ['ignore', 'ignore', process.env.PW_RUNNER_DEBUG ? 'inherit' : 'ignore', 'ipc'] stdio: ['ignore', 'ignore', process.env.PW_RUNNER_DEBUG ? 'inherit' : 'ignore', 'ipc']
}); });
this.process.on('exit', () => { this.process.on('exit', (code, signal) => {
this.didExit = true; this.didExit = true;
this.emit('exit', this._didSendStop /* expectedly */); this.emit('exit', { unexpectedly: !this._didSendStop, code, signal } as WorkerExitData);
}); });
this.process.on('error', e => {}); // do not yell at a send to dead process. this.process.on('error', e => {}); // do not yell at a send to dead process.
this.process.on('message', (message: any) => { this.process.on('message', (message: any) => {

View file

@ -83,20 +83,21 @@ test('it should not allow a focused test when forbid-only is used', async ({ run
expect(result.output).toContain(`- tests${path.sep}focused-test.spec.js:6 > i-am-focused`); expect(result.output).toContain(`- tests${path.sep}focused-test.spec.js:6 > i-am-focused`);
}); });
test('it should not hang and report results when worker process suddenly exits', async ({ runInlineTest }) => { test('should continue with other tests after worker process suddenly exits', async ({ runInlineTest }) => {
const result = await runInlineTest({ const result = await runInlineTest({
'a.spec.js': ` 'a.spec.js': `
const { test } = pwt; const { test } = pwt;
test('passed1', () => {}); test('passed1', () => {});
test('passed2', () => {}); test('passed2', () => {});
test('failed1', () => { process.exit(0); }); test('failed1', () => { process.exit(0); });
test('skipped', () => {}); test('passed3', () => {});
test('passed4', () => {});
` `
}); });
expect(result.exitCode).toBe(1); expect(result.exitCode).toBe(1);
expect(result.passed).toBe(2); expect(result.passed).toBe(4);
expect(result.failed).toBe(1); expect(result.failed).toBe(1);
expect(result.skipped).toBe(1); expect(result.skipped).toBe(0);
expect(result.output).toContain('Worker process exited unexpectedly'); expect(result.output).toContain('Worker process exited unexpectedly');
}); });