From 39a2e9cf3eece01edb4e348054ea64958eb094a0 Mon Sep 17 00:00:00 2001 From: Jon Hermansen Date: Tue, 11 Feb 2025 21:43:11 -0500 Subject: [PATCH 1/2] feat(config): failOnFlakyTests option This patch adds a configuration option to make the test runner exit with a non-zero status when any test is flaky. The config option does not take precedence over the CLI flag. Fixes #34397 --- docs/src/test-api/class-fullconfig.md | 6 ++ docs/src/test-api/class-testconfig.md | 18 ++++ packages/playwright/src/common/config.ts | 1 + packages/playwright/src/common/ipc.ts | 1 + .../playwright/src/isomorphic/teleReceiver.ts | 1 + .../playwright/src/runner/failureTracker.ts | 6 +- packages/playwright/types/test.d.ts | 25 ++++++ tests/playwright-test/config.spec.ts | 86 +++++++++++++++++++ 8 files changed, 143 insertions(+), 1 deletion(-) diff --git a/docs/src/test-api/class-fullconfig.md b/docs/src/test-api/class-fullconfig.md index 923c9fa858..848e42a926 100644 --- a/docs/src/test-api/class-fullconfig.md +++ b/docs/src/test-api/class-fullconfig.md @@ -10,6 +10,12 @@ Resolved configuration which is accessible via [`property: TestInfo.config`] and Path to the configuration file used to run the tests. The value is an empty string if no config file was used. +## property: FullConfig.failOnFlakyTests +* since: v1.51 +- type: <[boolean]> + +See [`property: TestConfig.failOnFlakyTests`]. + ## property: FullConfig.forbidOnly * since: v1.10 - type: <[boolean]> diff --git a/docs/src/test-api/class-testconfig.md b/docs/src/test-api/class-testconfig.md index 5b89e62b88..fde881a0e4 100644 --- a/docs/src/test-api/class-testconfig.md +++ b/docs/src/test-api/class-testconfig.md @@ -76,6 +76,24 @@ export default defineConfig({ }); ``` +## property: TestConfig.failOnFlakyTests +* since: v1.51 +- type: ?<[boolean]> + +Whether to exit with an error if any tests are marked as flaky. Useful on CI. + +Also available in the [command line](../test-cli.md) with the `--fail-on-flaky-tests` option. + +**Usage** + +```js title="playwright.config.ts" +import { defineConfig } from '@playwright/test'; + +export default defineConfig({ + failOnFlakyTests: process.env.CI ? true : false, +}); +``` + ## property: TestConfig.forbidOnly * since: v1.10 - type: ?<[boolean]> diff --git a/packages/playwright/src/common/config.ts b/packages/playwright/src/common/config.ts index b0bc394873..4398002bb8 100644 --- a/packages/playwright/src/common/config.ts +++ b/packages/playwright/src/common/config.ts @@ -90,6 +90,7 @@ export class FullConfigInternal { this.config = { configFile: resolvedConfigFile, rootDir: pathResolve(configDir, userConfig.testDir) || configDir, + failOnFlakyTests: takeFirst(configCLIOverrides.failOnFlakyTests, userConfig.failOnFlakyTests, false), forbidOnly: takeFirst(configCLIOverrides.forbidOnly, userConfig.forbidOnly, false), fullyParallel: takeFirst(configCLIOverrides.fullyParallel, userConfig.fullyParallel, false), globalSetup: this.globalSetups[0] ?? null, diff --git a/packages/playwright/src/common/ipc.ts b/packages/playwright/src/common/ipc.ts index d1cebd5cb4..10821f931e 100644 --- a/packages/playwright/src/common/ipc.ts +++ b/packages/playwright/src/common/ipc.ts @@ -25,6 +25,7 @@ import type { SerializedCompilationCache } from '../transform/compilationCache' export type ConfigCLIOverrides = { debug?: boolean; + failOnFlakyTests?: boolean; forbidOnly?: boolean; fullyParallel?: boolean; globalTimeout?: number; diff --git a/packages/playwright/src/isomorphic/teleReceiver.ts b/packages/playwright/src/isomorphic/teleReceiver.ts index 91b1a1a434..ade756eead 100644 --- a/packages/playwright/src/isomorphic/teleReceiver.ts +++ b/packages/playwright/src/isomorphic/teleReceiver.ts @@ -592,6 +592,7 @@ export class TeleTestResult implements reporterTypes.TestResult { export type TeleFullProject = reporterTypes.FullProject; export const baseFullConfig: reporterTypes.FullConfig = { + failOnFlakyTests: false, forbidOnly: false, fullyParallel: false, globalSetup: null, diff --git a/packages/playwright/src/runner/failureTracker.ts b/packages/playwright/src/runner/failureTracker.ts index ac62677571..1c274929b1 100644 --- a/packages/playwright/src/runner/failureTracker.ts +++ b/packages/playwright/src/runner/failureTracker.ts @@ -49,7 +49,7 @@ export class FailureTracker { } result(): 'failed' | 'passed' { - return this._hasWorkerErrors || this.hasReachedMaxFailures() || this.hasFailedTests() || (this._config.cliFailOnFlakyTests && this.hasFlakyTests()) ? 'failed' : 'passed'; + return this._hasWorkerErrors || this.hasReachedMaxFailures() || this.hasFailedTests() || (this.failOnFlakyTests() && this.hasFlakyTests()) ? 'failed' : 'passed'; } hasFailedTests() { @@ -63,4 +63,8 @@ export class FailureTracker { maxFailures() { return this._config.config.maxFailures; } + + failOnFlakyTests() { + return this._config.config.failOnFlakyTests || this._config.cliFailOnFlakyTests; + } } diff --git a/packages/playwright/types/test.d.ts b/packages/playwright/types/test.d.ts index ab23b480b2..3d5983afa0 100644 --- a/packages/playwright/types/test.d.ts +++ b/packages/playwright/types/test.d.ts @@ -1096,6 +1096,25 @@ interface TestConfig { }; }; + /** + * Whether to exit with an error if any tests are marked as flaky. Useful on CI. + * + * Also available in the [command line](https://playwright.dev/docs/test-cli) with the `--fail-on-flaky-tests` option. + * + * **Usage** + * + * ```js + * // playwright.config.ts + * import { defineConfig } from '@playwright/test'; + * + * export default defineConfig({ + * failOnFlakyTests: process.env.CI ? true : false, + * }); + * ``` + * + */ + failOnFlakyTests?: boolean; + /** * Whether to exit with an error if any tests or groups are marked as * [test.only(title[, details, body])](https://playwright.dev/docs/api/class-test#test-only) or @@ -1855,6 +1874,12 @@ export interface FullConfig { */ configFile?: string; + /** + * See + * [testConfig.failOnFlakyTests](https://playwright.dev/docs/api/class-testconfig#test-config-fail-on-flaky-tests). + */ + failOnFlakyTests: boolean; + /** * See [testConfig.forbidOnly](https://playwright.dev/docs/api/class-testconfig#test-config-forbid-only). */ diff --git a/tests/playwright-test/config.spec.ts b/tests/playwright-test/config.spec.ts index 518bf33839..a2de239de9 100644 --- a/tests/playwright-test/config.spec.ts +++ b/tests/playwright-test/config.spec.ts @@ -72,6 +72,92 @@ test('should prioritize command line timeout over project timeout', async ({ run expect(result.output).toContain('Test timeout of 500ms exceeded.'); }); +test('should default to failOnFlakyTests false', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.test.js': ` + import { test, expect } from '@playwright/test'; + test('flake', async ({}, testInfo) => { + expect(testInfo.retry).toBe(1); + }); + `, + }); + expect(result.exitCode).not.toBe(0); + expect(result.flaky).toBe(0); +}); + +test('should prioritize command line --fail-on-flaky-tests flag over config failOnFlakyTests', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'playwright.config.ts': ` + module.exports = { + failOnFlakyTests: false + }; + `, + 'a.test.js': ` + import { test, expect } from '@playwright/test'; + test('flake', async ({}, testInfo) => { + expect(testInfo.retry).toBe(1); + }); + `, + }, { 'retries': 1, 'fail-on-flaky-tests': true }); + expect(result.exitCode).not.toBe(0); + expect(result.flaky).toBe(1); +}); + +test('should support failOnFlakyTests config option', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'playwright.config.ts': ` + module.exports = { + failOnFlakyTests: true, + retries: 1 + }; + `, + 'a.test.js': ` + import { test, expect } from '@playwright/test'; + test('flake', async ({}, testInfo) => { + expect(testInfo.retry).toBe(1); + }); + `, + }, { 'retries': 1 }); + expect(result.exitCode).not.toBe(0); + expect(result.flaky).toBe(1); +}); + +test('should support failOnFlakyTests config option + retries CLI flag', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'playwright.config.ts': ` + module.exports = { + failOnFlakyTests: true, + }; + `, + 'a.test.js': ` + import { test, expect } from '@playwright/test'; + test('flake', async ({}, testInfo) => { + expect(testInfo.retry).toBe(1); + }); + `, + }, { 'retries': 1 }); + expect(result.exitCode).not.toBe(0); + expect(result.flaky).toBe(1); +}); + +test('should support fail-on-flaky-tests CLI flag + retries config option', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'playwright.config.ts': ` + module.exports = { + retries: 1, + }; + `, + 'a.test.js': ` + import { test, expect } from '@playwright/test'; + test('flake', async ({}, testInfo) => { + expect(testInfo.retry).toBe(1); + }); + `, + }, { 'fail-on-flaky-tests': true }); + expect(result.exitCode).not.toBe(0); + expect(result.flaky).toBe(1); +}); + test('should read config from --config, resolve relative testDir', async ({ runInlineTest }) => { const result = await runInlineTest({ 'my.config.ts': ` From 914625fe07ca5f2e5b681a006b634061e6ce028d Mon Sep 17 00:00:00 2001 From: Jon Hermansen Date: Mon, 17 Feb 2025 15:21:03 -0500 Subject: [PATCH 2/2] Incorporate feedback from PR --- packages/playwright/src/common/config.ts | 1 - .../playwright/src/runner/failureTracker.ts | 5 +- tests/playwright-test/config.spec.ts | 67 ------------------- 3 files changed, 1 insertion(+), 72 deletions(-) diff --git a/packages/playwright/src/common/config.ts b/packages/playwright/src/common/config.ts index 4398002bb8..0a91d71dbe 100644 --- a/packages/playwright/src/common/config.ts +++ b/packages/playwright/src/common/config.ts @@ -56,7 +56,6 @@ export class FullConfigInternal { cliProjectFilter?: string[]; cliListOnly = false; cliPassWithNoTests?: boolean; - cliFailOnFlakyTests?: boolean; cliLastFailed?: boolean; testIdMatcher?: Matcher; lastFailedTestIdMatcher?: Matcher; diff --git a/packages/playwright/src/runner/failureTracker.ts b/packages/playwright/src/runner/failureTracker.ts index 1c274929b1..afc4548a66 100644 --- a/packages/playwright/src/runner/failureTracker.ts +++ b/packages/playwright/src/runner/failureTracker.ts @@ -49,7 +49,7 @@ export class FailureTracker { } result(): 'failed' | 'passed' { - return this._hasWorkerErrors || this.hasReachedMaxFailures() || this.hasFailedTests() || (this.failOnFlakyTests() && this.hasFlakyTests()) ? 'failed' : 'passed'; + return this._hasWorkerErrors || this.hasReachedMaxFailures() || this.hasFailedTests() || (this._config.config.failOnFlakyTests && this.hasFlakyTests()) ? 'failed' : 'passed'; } hasFailedTests() { @@ -64,7 +64,4 @@ export class FailureTracker { return this._config.config.maxFailures; } - failOnFlakyTests() { - return this._config.config.failOnFlakyTests || this._config.cliFailOnFlakyTests; - } } diff --git a/tests/playwright-test/config.spec.ts b/tests/playwright-test/config.spec.ts index a2de239de9..04cafe16ea 100644 --- a/tests/playwright-test/config.spec.ts +++ b/tests/playwright-test/config.spec.ts @@ -72,37 +72,6 @@ test('should prioritize command line timeout over project timeout', async ({ run expect(result.output).toContain('Test timeout of 500ms exceeded.'); }); -test('should default to failOnFlakyTests false', async ({ runInlineTest }) => { - const result = await runInlineTest({ - 'a.test.js': ` - import { test, expect } from '@playwright/test'; - test('flake', async ({}, testInfo) => { - expect(testInfo.retry).toBe(1); - }); - `, - }); - expect(result.exitCode).not.toBe(0); - expect(result.flaky).toBe(0); -}); - -test('should prioritize command line --fail-on-flaky-tests flag over config failOnFlakyTests', async ({ runInlineTest }) => { - const result = await runInlineTest({ - 'playwright.config.ts': ` - module.exports = { - failOnFlakyTests: false - }; - `, - 'a.test.js': ` - import { test, expect } from '@playwright/test'; - test('flake', async ({}, testInfo) => { - expect(testInfo.retry).toBe(1); - }); - `, - }, { 'retries': 1, 'fail-on-flaky-tests': true }); - expect(result.exitCode).not.toBe(0); - expect(result.flaky).toBe(1); -}); - test('should support failOnFlakyTests config option', async ({ runInlineTest }) => { const result = await runInlineTest({ 'playwright.config.ts': ` @@ -122,42 +91,6 @@ test('should support failOnFlakyTests config option', async ({ runInlineTest }) expect(result.flaky).toBe(1); }); -test('should support failOnFlakyTests config option + retries CLI flag', async ({ runInlineTest }) => { - const result = await runInlineTest({ - 'playwright.config.ts': ` - module.exports = { - failOnFlakyTests: true, - }; - `, - 'a.test.js': ` - import { test, expect } from '@playwright/test'; - test('flake', async ({}, testInfo) => { - expect(testInfo.retry).toBe(1); - }); - `, - }, { 'retries': 1 }); - expect(result.exitCode).not.toBe(0); - expect(result.flaky).toBe(1); -}); - -test('should support fail-on-flaky-tests CLI flag + retries config option', async ({ runInlineTest }) => { - const result = await runInlineTest({ - 'playwright.config.ts': ` - module.exports = { - retries: 1, - }; - `, - 'a.test.js': ` - import { test, expect } from '@playwright/test'; - test('flake', async ({}, testInfo) => { - expect(testInfo.retry).toBe(1); - }); - `, - }, { 'fail-on-flaky-tests': true }); - expect(result.exitCode).not.toBe(0); - expect(result.flaky).toBe(1); -}); - test('should read config from --config, resolve relative testDir', async ({ runInlineTest }) => { const result = await runInlineTest({ 'my.config.ts': `