diff --git a/packages/playwright-test/src/runner/dispatcher.ts b/packages/playwright-test/src/runner/dispatcher.ts index 76ad8a0819..82f0caeeca 100644 --- a/packages/playwright-test/src/runner/dispatcher.ts +++ b/packages/playwright-test/src/runner/dispatcher.ts @@ -25,6 +25,7 @@ import { WorkerHost } from './workerHost'; import type { TestGroup } from './testGroups'; import type { FullConfigInternal } from '../common/config'; import type { ReporterV2 } from '../reporters/reporterV2'; +import type { FailureTracker } from './failureTracker'; type TestResultData = { result: TestResult; @@ -47,15 +48,15 @@ export class Dispatcher { private _testById = new Map(); private _config: FullConfigInternal; private _reporter: ReporterV2; - private _hasWorkerErrors = false; - private _failureCount = 0; + private _failureTracker: FailureTracker; private _extraEnvByProjectId: EnvByProjectId = new Map(); private _producedEnvByProjectId: EnvByProjectId = new Map(); - constructor(config: FullConfigInternal, reporter: ReporterV2) { + constructor(config: FullConfigInternal, reporter: ReporterV2, failureTracker: FailureTracker) { this._config = config; this._reporter = reporter; + this._failureTracker = failureTracker; } private _processFullySkippedJobs() { @@ -196,6 +197,9 @@ export class Dispatcher { } this._isStopped = false; this._workerSlots = []; + // 0. Stop right away if we have reached max failures. + if (this._failureTracker.hasReachedMaxFailures()) + void this.stop(); // 1. Allocate workers. for (let i = 0; i < this._config.config.workers; i++) this._workerSlots.push({ busy: false }); @@ -248,7 +252,7 @@ export class Dispatcher { const onTestEnd = (params: TestEndPayload) => { runningTest = false; remainingByTestId.delete(params.testId); - if (this._hasReachedMaxFailures()) { + if (this._failureTracker.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'; @@ -352,7 +356,7 @@ export class Dispatcher { remaining = remaining.filter(test => { if (!testIds.has(test.id)) return true; - if (!this._hasReachedMaxFailures()) { + if (!this._failureTracker.hasReachedMaxFailures()) { const data = this._testById.get(test.id)!; const runData = data.resultByWorkerIndex.get(worker.workerIndex); // There might be a single test that has started but has not finished yet. @@ -377,7 +381,7 @@ export class Dispatcher { 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._hasWorkerErrors = true; + this._failureTracker.onWorkerError(); for (const error of errors) this._reporter.onError(error); } @@ -496,7 +500,7 @@ export class Dispatcher { this._reporter.onStdErr(chunk, test, result); }); worker.on('teardownErrors', (params: TeardownErrorsPayload) => { - this._hasWorkerErrors = true; + this._failureTracker.onWorkerError(); for (const error of params.fatalErrors) this._reporter.onError(error); }); @@ -519,23 +523,12 @@ export class Dispatcher { this._checkFinished(); } - private _hasReachedMaxFailures() { - const maxFailures = this._config.config.maxFailures; - return maxFailures > 0 && this._failureCount >= maxFailures; - } - private _reportTestEnd(test: TestCase, result: TestResult) { - if (result.status !== 'skipped' && result.status !== test.expectedStatus) - ++this._failureCount; this._reporter.onTestEnd(test, result); - const maxFailures = this._config.config.maxFailures; - if (maxFailures && this._failureCount === maxFailures) + this._failureTracker.onTestEnd(test, result); + if (this._failureTracker.hasReachedMaxFailures()) this.stop().catch(e => {}); } - - hasWorkerErrors(): boolean { - return this._hasWorkerErrors; - } } function chunkFromParams(params: TestOutputPayload): string | Buffer { diff --git a/packages/playwright-test/src/runner/failureTracker.ts b/packages/playwright-test/src/runner/failureTracker.ts new file mode 100644 index 0000000000..fc2202b3ea --- /dev/null +++ b/packages/playwright-test/src/runner/failureTracker.ts @@ -0,0 +1,54 @@ +/** + * Copyright Microsoft Corporation. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import type { TestResult } from '../../types/testReporter'; +import type { FullConfigInternal } from '../common/config'; +import type { Suite, TestCase } from '../common/test'; + +export class FailureTracker { + private _failureCount = 0; + private _hasWorkerErrors = false; + private _rootSuite: Suite | undefined; + + constructor(private _config: FullConfigInternal) { + } + + onRootSuite(rootSuite: Suite) { + this._rootSuite = rootSuite; + } + + onTestEnd(test: TestCase, result: TestResult) { + if (result.status !== 'skipped' && result.status !== test.expectedStatus) + ++this._failureCount; + } + + onWorkerError() { + this._hasWorkerErrors = true; + } + + hasReachedMaxFailures() { + const maxFailures = this._config.config.maxFailures; + return maxFailures > 0 && this._failureCount >= maxFailures; + } + + hasWorkerErrors() { + return this._hasWorkerErrors; + } + + result(): 'failed' | 'passed' { + return this._hasWorkerErrors || this._rootSuite?.allTests().some(test => !test.ok()) ? 'failed' : 'passed'; + } +} diff --git a/packages/playwright-test/src/runner/runner.ts b/packages/playwright-test/src/runner/runner.ts index 6fa8bb3c6e..55fb66f2d6 100644 --- a/packages/playwright-test/src/runner/runner.ts +++ b/packages/playwright-test/src/runner/runner.ts @@ -87,9 +87,7 @@ export class Runner { } const taskStatus = await taskRunner.run(testRun, deadline); - let status: FullResult['status'] = 'passed'; - if (testRun.phases.find(p => p.dispatcher.hasWorkerErrors()) || testRun.rootSuite?.allTests().some(test => !test.ok())) - status = 'failed'; + let status: FullResult['status'] = testRun.failureTracker.result(); if (status === 'passed' && taskStatus !== 'passed') status = taskStatus; await reporter.onEnd({ status }); diff --git a/packages/playwright-test/src/runner/tasks.ts b/packages/playwright-test/src/runner/tasks.ts index 96982eebc0..fb89bd261c 100644 --- a/packages/playwright-test/src/runner/tasks.ts +++ b/packages/playwright-test/src/runner/tasks.ts @@ -29,6 +29,7 @@ import { collectProjectsAndTestFiles, createRootSuite, loadFileSuites, loadGloba import type { Matcher } from '../util'; import type { Suite } from '../common/test'; import { buildDependentProjects, buildTeardownToSetupsMap } from './projectUtils'; +import { FailureTracker } from './failureTracker'; const readDirAsync = promisify(fs.readdir); @@ -46,6 +47,7 @@ export type Phase = { export class TestRun { readonly reporter: ReporterV2; readonly config: FullConfigInternal; + readonly failureTracker: FailureTracker; rootSuite: Suite | undefined = undefined; readonly phases: Phase[] = []; projects: FullProjectInternal[] = []; @@ -55,6 +57,7 @@ export class TestRun { constructor(config: FullConfigInternal, reporter: ReporterV2) { this.config = config; this.reporter = reporter; + this.failureTracker = new FailureTracker(config); } } @@ -190,6 +193,7 @@ function createLoadTask(mode: 'out-of-process' | 'in-process', options: { filter await collectProjectsAndTestFiles(testRun, !!options.doNotRunTestsOutsideProjectFilter, options.additionalFileMatcher); await loadFileSuites(testRun, mode, options.failOnLoadErrors ? errors : softErrors); testRun.rootSuite = await createRootSuite(testRun, options.failOnLoadErrors ? errors : softErrors, !!options.filterOnly); + testRun.failureTracker.onRootSuite(testRun.rootSuite); // Fail when no tests. if (options.failOnLoadErrors && !testRun.rootSuite.allTests().length && !testRun.config.cliPassWithNoTests && !testRun.config.config.shard) throw new Error(`No tests found`); @@ -230,7 +234,7 @@ function createPhasesTask(): Task { processed.add(project); if (phaseProjects.length) { let testGroupsInPhase = 0; - const phase: Phase = { dispatcher: new Dispatcher(testRun.config, testRun.reporter), projects: [] }; + const phase: Phase = { dispatcher: new Dispatcher(testRun.config, testRun.reporter, testRun.failureTracker), projects: [] }; testRun.phases.push(phase); for (const project of phaseProjects) { const projectSuite = projectToSuite.get(project)!; @@ -250,7 +254,7 @@ function createPhasesTask(): Task { function createRunTestsTask(): Task { return { - setup: async ({ phases }) => { + setup: async ({ phases, failureTracker }) => { const successfulProjects = new Set(); const extraEnvByProjectId: EnvByProjectId = new Map(); const teardownToSetups = buildTeardownToSetupsMap(phases.map(phase => phase.projects.map(p => p.project)).flat()); @@ -291,7 +295,7 @@ function createRunTestsTask(): Task { // If the worker broke, fail everything, we have no way of knowing which // projects failed. - if (!dispatcher.hasWorkerErrors()) { + if (!failureTracker.hasWorkerErrors()) { for (const { project, projectSuite } of projects) { const hasFailedDeps = project.deps.some(p => !successfulProjects.has(p)); if (!hasFailedDeps && !projectSuite.allTests().some(test => !test.ok())) diff --git a/packages/playwright-test/src/runner/watchMode.ts b/packages/playwright-test/src/runner/watchMode.ts index 2f7f758fd7..2f860e0f1b 100644 --- a/packages/playwright-test/src/runner/watchMode.ts +++ b/packages/playwright-test/src/runner/watchMode.ts @@ -298,7 +298,7 @@ async function runTests(config: FullConfigInternal, failedTestIdCollector: Set p.dispatcher.hasWorkerErrors()) || hasFailedTests) + if (testRun.failureTracker.hasWorkerErrors() || hasFailedTests) status = 'failed'; if (status === 'passed' && taskStatus !== 'passed') status = taskStatus; diff --git a/tests/playwright-test/max-failures.spec.ts b/tests/playwright-test/max-failures.spec.ts index e6cc044884..f61c8f3fd3 100644 --- a/tests/playwright-test/max-failures.spec.ts +++ b/tests/playwright-test/max-failures.spec.ts @@ -146,3 +146,40 @@ test('max-failures should properly shutdown', async ({ runInlineTest }) => { expect(result.failed).toBe(1); expect(result.output).toContain('expect(false).toBeTruthy()'); }); + +test('max-failures should work across phases', async ({ runInlineTest }) => { + test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/26344' }); + + const result = await runInlineTest({ + 'playwright.config.ts': ` + const config = { + testDir: './', + maxFailures: 1, + projects: [ + { name: 'a', testMatch: ['example.spec.ts'] }, + { name: 'b', testMatch: ['example.spec.ts'], dependencies: ['a'] }, + { name: 'c', testMatch: ['example.spec.ts'], dependencies: ['a'] }, + { name: 'd', testMatch: ['example.spec.ts'], dependencies: ['b'] }, + ] + }; + export default config; + `, + 'example.spec.ts': ` + import { test, expect } from '@playwright/test'; + test('test', () => { + const project = test.info().project.name; + console.log('running ' + project); + if (project === 'c') + throw new Error('failed!'); + }); + `, + }, { workers: 1 }); + expect(result.exitCode).toBe(1); + expect(result.failed).toBe(1); + expect(result.passed).toBe(2); + expect(result.skipped).toBe(1); + expect(result.output).toContain('running a'); + expect(result.output).toContain('running b'); + expect(result.output).toContain('running c'); + expect(result.output).not.toContain('running d'); +});