From 79d356f0cc0dc30feef5715467b479ab41e26d47 Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Thu, 2 Jun 2022 21:13:47 -0700
Subject: [PATCH] fix(test runner): when worker exits unexpectedly, fail a
 single test (#14608)

All remaining tests will continue in a new worker.
---
 packages/playwright-test/src/dispatcher.ts | 27 ++++++++++++++++------
 tests/playwright-test/runner.spec.ts       |  9 ++++----
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/packages/playwright-test/src/dispatcher.ts b/packages/playwright-test/src/dispatcher.ts
index 474c65fe4c..1cde571337 100644
--- a/packages/playwright-test/src/dispatcher.ts
+++ b/packages/playwright-test/src/dispatcher.ts
@@ -40,6 +40,13 @@ type TestData = {
   test: TestCase;
   resultByWorkerIndex: Map<number, TestResultData>;
 };
+
+type WorkerExitData = {
+  unexpectedly: boolean;
+  code: number | null;
+  signal: NodeJS.Signals | null;
+};
+
 export class Dispatcher {
   private _workerSlots: { busy: boolean, worker?: Worker }[] = [];
   private _queue: TestGroup[] = [];
@@ -271,7 +278,7 @@ export class Dispatcher {
     };
     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);
       let remaining = [...remainingByTestId.values()];
 
@@ -279,7 +286,7 @@ export class Dispatcher {
       // - there are no remaining
       // - we are here not because something failed
       // - 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))
           worker.stop();
         doneWithJob();
@@ -289,7 +296,7 @@ export class Dispatcher {
       // When worker encounters error, we will stop it and create a new one.
       worker.stop(true /* didFail */);
 
-      const massSkipTestsFromRemaining = (testIds: Set<string>, errors: TestError[]) => {
+      const massSkipTestsFromRemaining = (testIds: Set<string>, errors: TestError[], onlyStartedTests?: boolean) => {
         remaining = remaining.filter(test => {
           if (!testIds.has(test._id))
             return true;
@@ -301,6 +308,8 @@ export class Dispatcher {
             if (runData) {
               result = runData.result;
             } else {
+              if (onlyStartedTests)
+                return true;
               result = data.test._appendTestResult();
               this._reporter.onTestBegin?.(test, result);
             }
@@ -336,6 +345,9 @@ export class Dispatcher {
       }
       // Handle tests that should be skipped because of the setup failure.
       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 serialSuitesWithFailures = new Set<Suite>();
@@ -396,8 +408,9 @@ export class Dispatcher {
     };
     worker.on('done', onDone);
 
-    const onExit = (expectedly: boolean) => {
-      onDone({ skipTestsDueToSetupFailure: [], fatalErrors: expectedly ? [] : [{ value: 'Worker process exited unexpectedly' }] });
+    const onExit = (data: WorkerExitData) => {
+      const unexpectedExitError = data.unexpectedly ? { value: `Worker process exited unexpectedly (code=${data.code}, signal=${data.signal})` } : undefined;
+      onDone({ skipTestsDueToSetupFailure: [], fatalErrors: [], unexpectedExitError });
     };
     worker.on('exit', onExit);
 
@@ -492,9 +505,9 @@ class Worker extends EventEmitter {
       // Can't pipe since piping slows down termination for some reason.
       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.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('message', (message: any) => {
diff --git a/tests/playwright-test/runner.spec.ts b/tests/playwright-test/runner.spec.ts
index 3d01b77917..5f2515db58 100644
--- a/tests/playwright-test/runner.spec.ts
+++ b/tests/playwright-test/runner.spec.ts
@@ -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`);
 });
 
-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({
     'a.spec.js': `
       const { test } = pwt;
       test('passed1', () => {});
       test('passed2', () => {});
       test('failed1', () => { process.exit(0); });
-      test('skipped', () => {});
+      test('passed3', () => {});
+      test('passed4', () => {});
     `
   });
   expect(result.exitCode).toBe(1);
-  expect(result.passed).toBe(2);
+  expect(result.passed).toBe(4);
   expect(result.failed).toBe(1);
-  expect(result.skipped).toBe(1);
+  expect(result.skipped).toBe(0);
   expect(result.output).toContain('Worker process exited unexpectedly');
 });