From 2e6ef8f622a67084f6165edd5982f57482e44bcb Mon Sep 17 00:00:00 2001
From: Yury Semikhatsky <yurys@chromium.org>
Date: Mon, 25 Apr 2022 09:05:40 -0700
Subject: [PATCH] fix(runner): fail if worker cannot find some of the tests
 (#13666)

---
 packages/playwright-test/src/dispatcher.ts   |  9 +++++-
 packages/playwright-test/src/ipc.ts          |  1 +
 packages/playwright-test/src/workerRunner.ts |  5 +++
 tests/playwright-test/runner.spec.ts         | 34 ++++++++++++++++++++
 4 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/packages/playwright-test/src/dispatcher.ts b/packages/playwright-test/src/dispatcher.ts
index d7f14241c8..e036bcd3df 100644
--- a/packages/playwright-test/src/dispatcher.ts
+++ b/packages/playwright-test/src/dispatcher.ts
@@ -279,7 +279,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) {
+      if (!remaining.length && !failedTestIds.size && !params.fatalErrors.length && !params.skipTestsDueToSetupFailure.length && !params.fatalUnknownTestIds) {
         if (this._isWorkerRedundant(worker))
           worker.stop();
         doneWithJob();
@@ -322,6 +322,13 @@ export class Dispatcher {
         }
       };
 
+      if (params.fatalUnknownTestIds) {
+        const titles = params.fatalUnknownTestIds.map(testId => {
+          const test = this._testById.get(testId)!.test;
+          return test.titlePath().slice(1).join(' > ');
+        });
+        massSkipTestsFromRemaining(new Set(params.fatalUnknownTestIds), [{ message: `Unknown test(s) in worker:\n${titles.join('\n')}` }]);
+      }
       if (params.fatalErrors.length) {
         // In case of fatal errors, report first remaining test as failing with these errors,
         // and all others as skipped.
diff --git a/packages/playwright-test/src/ipc.ts b/packages/playwright-test/src/ipc.ts
index f0a0ef9da9..90881567cd 100644
--- a/packages/playwright-test/src/ipc.ts
+++ b/packages/playwright-test/src/ipc.ts
@@ -78,6 +78,7 @@ export type RunPayload = {
 export type DonePayload = {
   fatalErrors: TestError[];
   skipTestsDueToSetupFailure: string[];  // test ids
+  fatalUnknownTestIds?: string[];
 };
 
 export type TestOutputPayload = {
diff --git a/packages/playwright-test/src/workerRunner.ts b/packages/playwright-test/src/workerRunner.ts
index 090fc536c2..c03c24b828 100644
--- a/packages/playwright-test/src/workerRunner.ts
+++ b/packages/playwright-test/src/workerRunner.ts
@@ -157,6 +157,7 @@ export class WorkerRunner extends EventEmitter {
   async runTestGroup(runPayload: RunPayload) {
     this._runFinished = new ManualPromise<void>();
     const entries = new Map(runPayload.entries.map(e => [ e.testId, e ]));
+    let fatalUnknownTestIds;
     try {
       await this._loadIfNeeded();
       const fileSuite = await this._loader.loadTestFile(runPayload.file, 'worker');
@@ -178,6 +179,9 @@ export class WorkerRunner extends EventEmitter {
           entries.delete(tests[i]._id);
           await this._runTest(tests[i], entry.retry, tests[i + 1]);
         }
+      } else {
+        fatalUnknownTestIds = runPayload.entries.map(e => e.testId);
+        this.stop();
       }
     } catch (e) {
       // In theory, we should run above code without any errors.
@@ -188,6 +192,7 @@ export class WorkerRunner extends EventEmitter {
       const donePayload: DonePayload = {
         fatalErrors: this._fatalErrors,
         skipTestsDueToSetupFailure: [],
+        fatalUnknownTestIds
       };
       for (const test of this._skipRemainingTestsInSuite?.allTests() || []) {
         if (entries.has(test._id))
diff --git a/tests/playwright-test/runner.spec.ts b/tests/playwright-test/runner.spec.ts
index 535d8eec1b..3ae9d3d73d 100644
--- a/tests/playwright-test/runner.spec.ts
+++ b/tests/playwright-test/runner.spec.ts
@@ -248,3 +248,37 @@ test('should teardown workers that are redundant', async ({ runInlineTest }) =>
     '%%worker teardown',
   ]);
 });
+
+test('should not hang if test suites in worker are inconsistent with runner', async ({ runInlineTest }) => {
+  const oldValue = process.env.TEST_WORKER_INDEX;
+  delete process.env.TEST_WORKER_INDEX;
+  const result = await runInlineTest({
+    'playwright.config.ts': `
+      module.exports = { name: 'project-name' };
+    `,
+    'names.js': `
+    exports.getNames = () => {
+      const inWorker = process.env.TEST_WORKER_INDEX !== undefined;
+      if (inWorker)
+        return ['foo'];
+      return ['foo', 'bar', 'baz'];
+    };
+    `,
+    'a.spec.js': `
+      const { test } = pwt;
+      const { getNames } = require('./names');
+      const names = getNames();
+      for (const index in names) {
+        test('Test ' + index + ' - ' + names[index], async () => {
+        });
+      }
+    `,
+  }, { 'workers': 1 });
+  process.env.TEST_WORKER_INDEX = oldValue;
+  expect(result.exitCode).toBe(1);
+  expect(result.passed).toBe(1);
+  expect(result.failed).toBe(1);
+  expect(result.skipped).toBe(1);
+  expect(result.report.suites[0].specs[1].tests[0].results[0].error.message).toBe('Unknown test(s) in worker:\nproject-name > a.spec.js > Test 1 - bar\nproject-name > a.spec.js > Test 2 - baz');
+});
+