chore: merge FailureTracker into TestRun, pull more into TestRunOptions (#40150)

Author: dgozman

packages/playwright/src/cli/testActions.ts

@@ -41,6 +41,7 @@ export async function runTests(args: string[], opts: { [key: string]: any }) {
     lastFailed: !!opts.lastFailed,
     testList: opts.testList ? path.resolve(process.cwd(), opts.testList) : undefined,
     testListInvert: opts.testListInvert ? path.resolve(process.cwd(), opts.testListInvert) : undefined,
+    shardWeights: resolveShardWeightsOption(),
   };
 
   // Evaluate project filters against config before starting execution. This enables a consistent error message across run modes
@@ -124,7 +125,6 @@ function overridesFromOptions(options: { [key: string]: any }): ipc.ConfigCLIOve
     retries: options.retries ? parseInt(options.retries, 10) : undefined,
     reporter: resolveReporterOption(options.reporter),
     shard: resolveShardOption(options.shard),
-    shardWeights: resolveShardWeightsOption(),
     timeout: options.timeout ? parseInt(options.timeout, 10) : undefined,
     tsconfig: options.tsconfig ? path.resolve(process.cwd(), options.tsconfig) : undefined,
     ignoreSnapshots: options.ignoreSnapshots ? !!options.ignoreSnapshots : undefined,
@@ -195,7 +195,7 @@ function resolveShardOption(shard?: string): ipc.ConfigCLIOverrides['shard'] {
   return { current, total };
 }
 
-function resolveShardWeightsOption(): ipc.ConfigCLIOverrides['shardWeights'] {
+function resolveShardWeightsOption(): TestRunOptions['shardWeights'] {
   const shardWeights = process.env.PWTEST_SHARD_WEIGHTS;
   if (!shardWeights)
     return undefined;

packages/playwright/src/common/ipc.ts

@@ -30,15 +30,12 @@ export type ConfigCLIOverrides = {
   globalTimeout?: number;
   maxFailures?: number;
   outputDir?: string;
-  preserveOutputDir?: boolean;
   pause?: boolean;
   quiet?: boolean;
   repeatEach?: number;
   retries?: number;
   reporter?: ReporterDescription[];
-  additionalReporters?: ReporterDescription[];
   shard?: { current: number, total: number };
-  shardWeights?: number[];
   timeout?: number;
   tsconfig?: string;
   ignoreSnapshots?: boolean;

packages/playwright/src/runner/dispatcher.ts

@@ -20,16 +20,15 @@ import { eventsHelper } from '@utils/eventsHelper';
 
 import { addSuggestedRebaseline } from './rebase';
 import { WorkerHost } from './workerHost';
-import { FullConfigInternal, ipc, test as testNs } from '../common';
+import { ipc, test as testNs } from '../common';
 import { addLocationAndSnippetToError } from '../reporters/internalReporter';
 import { serializeError } from '../util';
 
 import type { RegisteredListener } from '@utils/eventsHelper';
-import type { FailureTracker } from './failureTracker';
 import type { ProcessExitData } from './processHost';
+import type { TestRun } from './tasks';
 import type { TestGroup } from './testGroups';
 import type { TestError, TestResult, TestStep } from '../../types/testReporter';
-import type { ReporterV2 } from '../reporters/reporterV2';
 
 export type EnvByProjectId = Map<string, Record<string, string | undefined>>;
 
@@ -42,18 +41,14 @@ export class Dispatcher {
   private _finished = new ManualPromise<void>();
   private _isStopped = true;
 
-  private _config: FullConfigInternal;
-  private _reporter: ReporterV2;
-  private _failureTracker: FailureTracker;
+  private _testRun: TestRun;
 
   private _extraEnvByProjectId: EnvByProjectId = new Map();
   private _producedEnvByProjectId: EnvByProjectId = new Map();
 
-  constructor(config: FullConfigInternal, reporter: ReporterV2, failureTracker: FailureTracker) {
-    this._config = config;
-    this._reporter = reporter;
-    this._failureTracker = failureTracker;
-    for (const project of config.projects) {
+  constructor(testRun: TestRun) {
+    this._testRun = testRun;
+    for (const project of testRun.config.projects) {
       if (project.workers)
         this._workerLimitPerProjectId.set(project.id, project.workers);
     }
@@ -97,7 +92,7 @@ export class Dispatcher {
 
     // 3. Claim both the job and the worker slot.
     this._queue.splice(jobIndex, 1);
-    const jobDispatcher = new JobDispatcher(job, this._config, this._reporter, this._failureTracker, () => this.stop().catch(() => {}));
+    const jobDispatcher = new JobDispatcher(job, this._testRun, () => this.stop().catch(() => {}));
     this._workerSlots[workerIndex].jobDispatcher = jobDispatcher;
 
     // 4. Run the job. This is the only async operation.
@@ -132,7 +127,7 @@ export class Dispatcher {
     // 2. Start the worker if it is down.
     let startError;
     if (!worker) {
-      worker = this._createWorker(job, index, ipc.serializeConfig(this._config, true));
+      worker = this._createWorker(job, index, ipc.serializeConfig(this._testRun.config, true));
       this._workerSlots[index].worker = worker;
       worker.on('exit', () => this._workerSlots[index].worker = undefined);
       startError = await worker.start();
@@ -198,10 +193,10 @@ export class Dispatcher {
     this._isStopped = false;
     this._workerSlots = [];
     // 0. Stop right away if we have reached max failures.
-    if (this._failureTracker.hasReachedMaxFailures())
+    if (this._testRun.hasReachedMaxFailures())
       void this.stop();
     // 1. Allocate workers.
-    for (let i = 0; i < this._config.config.workers; i++)
+    for (let i = 0; i < this._testRun.config.config.workers; i++)
       this._workerSlots.push({});
     // 2. Schedule enough jobs.
     for (let i = 0; i < this._workerSlots.length; i++)
@@ -213,15 +208,15 @@ export class Dispatcher {
   }
 
   _createWorker(testGroup: TestGroup, parallelIndex: number, loaderData: ipc.SerializedConfig) {
-    const projectConfig = this._config.projects.find(p => p.id === testGroup.projectId)!;
-    const outputDir = projectConfig.project.outputDir;
+    const project = this._testRun.config.projects.find(p => p.id === testGroup.projectId)!;
+    const pauseAtEnd = this._testRun.topLevelProjects.includes(project) && !!this._testRun.options.pauseAtEnd;
     const worker = new WorkerHost(testGroup, {
       parallelIndex,
       config: loaderData,
       extraEnv: this._extraEnvByProjectId.get(testGroup.projectId) || {},
-      outputDir,
-      pauseOnError: this._failureTracker.pauseOnError(),
-      pauseAtEnd: this._failureTracker.pauseAtEnd(projectConfig),
+      outputDir: project.project.outputDir,
+      pauseOnError: !!this._testRun.options.pauseOnError,
+      pauseAtEnd,
     });
     const handleOutput = (params: ipc.TestOutputPayload) => {
       const chunk = chunkFromParams(params);
@@ -239,17 +234,17 @@ export class Dispatcher {
     worker.on('stdOut', (params: ipc.TestOutputPayload) => {
       const { chunk, test, result } = handleOutput(params);
       result?.stdout.push(chunk);
-      this._reporter.onStdOut?.(chunk, test, result);
+      this._testRun.reporter.onStdOut?.(chunk, test, result);
     });
     worker.on('stdErr', (params: ipc.TestOutputPayload) => {
       const { chunk, test, result } = handleOutput(params);
       result?.stderr.push(chunk);
-      this._reporter.onStdErr?.(chunk, test, result);
+      this._testRun.reporter.onStdErr?.(chunk, test, result);
     });
     worker.on('teardownErrors', (params: ipc.TeardownErrorsPayload) => {
-      this._failureTracker.onWorkerError();
+      this._testRun.hasWorkerErrors = true;
       for (const error of params.fatalErrors)
-        this._reporter.onError?.(error);
+        this._testRun.reporter.onError?.(error);
     });
     worker.on('exit', () => {
       const producedEnv = this._producedEnvByProjectId.get(testGroup.projectId) || {};
@@ -275,9 +270,7 @@ class JobDispatcher {
   jobResult = new ManualPromise<{ newJob?: TestGroup, didFail: boolean }>();
 
   readonly job: TestGroup;
-  private _config: FullConfigInternal;
-  private _reporter: ReporterV2;
-  private _failureTracker: FailureTracker;
+  private _testRun: TestRun;
   private _stopCallback: () => void;
   private _listeners: RegisteredListener[] = [];
   private _failedTests = new Set<testNs.TestCase>();
@@ -288,11 +281,9 @@ class JobDispatcher {
   private _workerIndex = 0;
   private _currentlyRunning: { test: testNs.TestCase, result: TestResult } | undefined;
 
-  constructor(job: TestGroup, config: FullConfigInternal, reporter: ReporterV2, failureTracker: FailureTracker, stopCallback: () => void) {
+  constructor(job: TestGroup, testRun: TestRun, stopCallback: () => void) {
     this.job = job;
-    this._config = config;
-    this._reporter = reporter;
-    this._failureTracker = failureTracker;
+    this._testRun = testRun;
     this._stopCallback = stopCallback;
     this._remainingByTestId = new Map(this.job.tests.map(e => [e.id, e]));
   }
@@ -308,12 +299,12 @@ class JobDispatcher {
     result.parallelIndex = this._parallelIndex;
     result.workerIndex = this._workerIndex;
     result.startTime = new Date(params.startWallTime);
-    this._reporter.onTestBegin?.(test, result);
+    this._testRun.reporter.onTestBegin?.(test, result);
     this._currentlyRunning = { test, result };
   }
 
   private _onTestEnd(params: ipc.TestEndPayload) {
-    if (this._failureTracker.hasReachedMaxFailures()) {
+    if (this._testRun.hasReachedMaxFailures()) {
       // Do not show more than one error to avoid confusion, but report
       // as interrupted to indicate that we did actually start the test.
       params.status = 'interrupted';
@@ -377,7 +368,7 @@ class JobDispatcher {
     };
     steps.set(params.stepId, step);
     (parentStep || result).steps.push(step);
-    this._reporter.onStepBegin?.(test, result, step);
+    this._testRun.reporter.onStepBegin?.(test, result, step);
   }
 
   private _onStepEnd(params: ipc.StepEndPayload) {
@@ -389,7 +380,7 @@ class JobDispatcher {
     const { result, steps, test } = data;
     const step = steps.get(params.stepId);
     if (!step) {
-      this._reporter.onStdErr?.('Internal error: step end without step begin: ' + params.stepId, test, result);
+      this._testRun.reporter.onStdErr?.('Internal error: step end without step begin: ' + params.stepId, test, result);
       return;
     }
     step.duration = params.wallTime - step.startTime.getTime();
@@ -399,7 +390,7 @@ class JobDispatcher {
       addSuggestedRebaseline(step.location!, params.suggestedRebaseline);
     step.annotations = params.annotations;
     steps.delete(params.stepId);
-    this._reporter.onStepEnd?.(test, result, step);
+    this._testRun.reporter.onStepEnd?.(test, result, step);
   }
 
   private _onAttach(params: ipc.AttachmentPayload) {
@@ -420,7 +411,7 @@ class JobDispatcher {
       if (step)
         step.attachments.push(attachment);
       else
-        this._reporter.onStdErr?.('Internal error: step id not found: ' + params.stepId);
+        this._testRun.reporter.onStdErr?.('Internal error: step id not found: ' + params.stepId);
     }
   }
 
@@ -432,7 +423,7 @@ class JobDispatcher {
       result = runData.result;
     } else {
       result = test._appendTestResult();
-      this._reporter.onTestBegin?.(test, result);
+      this._testRun.reporter.onTestBegin?.(test, result);
     }
     result.errors = [...errors];
     result.error = result.errors[0];
@@ -445,7 +436,7 @@ class JobDispatcher {
     for (const test of this._remainingByTestId.values()) {
       if (!testIds.has(test.id))
         continue;
-      if (!this._failureTracker.hasReachedMaxFailures()) {
+      if (!this._testRun.hasReachedMaxFailures()) {
         this._failTestWithErrors(test, errors);
         errors = []; // Only report errors for the first test.
       }
@@ -454,9 +445,9 @@ class JobDispatcher {
     if (errors.length) {
       // We had fatal errors after all tests have passed - most likely in some teardown.
       // Let's just fail the test run.
-      this._failureTracker.onWorkerError();
+      this._testRun.hasWorkerErrors = true;
       for (const error of errors)
-        this._reporter.onError?.(error);
+        this._testRun.reporter.onError?.(error);
     }
   }
 
@@ -592,11 +583,11 @@ class JobDispatcher {
           throw new Error('Test has already stopped');
         const response = await worker.sendCustomMessage({ testId: test.id, request: message.request });
         if (response.error)
-          addLocationAndSnippetToError(this._config.config, response.error);
+          addLocationAndSnippetToError(this._testRun.config.config, response.error);
         return response;
       } catch (e) {
         const error = serializeError(e);
-        addLocationAndSnippetToError(this._config.config, error);
+        addLocationAndSnippetToError(this._testRun.config.config, error);
         return { response: undefined, error };
       }
     };
@@ -605,10 +596,10 @@ class JobDispatcher {
     result.errors = params.errors;
     result.error = result.errors[0];
 
-    void this._reporter.onTestPaused?.(test, result).then(() => {
+    void this._testRun.reporter.onTestPaused?.(test, result).then(() => {
       worker.sendResume({});
     });
-    this._failureTracker.onTestPaused?.({ ...params, sendMessage });
+    this._testRun.onTestPaused({ ...params, sendMessage });
   }
 
   skipWholeJob(): boolean {
@@ -620,10 +611,10 @@ class JobDispatcher {
     // with skipped tests mixed in-between non-skipped. This makes
     // for a better reporter experience.
     const allTestsSkipped = this.job.tests.every(test => test.expectedStatus === 'skipped');
-    if (allTestsSkipped && !this._failureTracker.hasReachedMaxFailures()) {
+    if (allTestsSkipped && !this._testRun.hasReachedMaxFailures()) {
       for (const test of this.job.tests) {
         const result = test._appendTestResult();
-        this._reporter.onTestBegin?.(test, result);
+        this._testRun.reporter.onTestBegin?.(test, result);
         result.status = 'skipped';
         // This must mirror _onTestEnd() above
         result.annotations = [...test.annotations];
@@ -639,13 +630,14 @@ class JobDispatcher {
   }
 
   private _reportTestEnd(test: testNs.TestCase, result: TestResult) {
-    this._reporter.onTestEnd?.(test, result);
-    const hadMaxFailures = this._failureTracker.hasReachedMaxFailures();
-    this._failureTracker.onTestEnd(test, result);
-    if (this._failureTracker.hasReachedMaxFailures()) {
+    this._testRun.reporter.onTestEnd?.(test, result);
+    const hadMaxFailures = this._testRun.hasReachedMaxFailures();
+    // Test is considered failing after the last retry.
+    if (test.outcome() === 'unexpected' && test.results.length > test.retries)
+      ++this._testRun.failedTestCount;
+    if (!hadMaxFailures && this._testRun.hasReachedMaxFailures()) {
       this._stopCallback();
-      if (!hadMaxFailures)
-        this._reporter.onError?.({ message: colors.red(`Testing stopped early after ${this._failureTracker.maxFailures()} maximum allowed failures.`) });
+      this._testRun.reporter.onError?.({ message: colors.red(`Testing stopped early after ${this._testRun.config.config.maxFailures} maximum allowed failures.`) });
     }
   }
 }