Skip to content

Commit

Permalink
feat: implement flag to fail flaky tests
Browse files Browse the repository at this point in the history
  • Loading branch information
Joe-Hendley committed May 1, 2024
1 parent c3d8b22 commit 5e27e31
Show file tree
Hide file tree
Showing 14 changed files with 100 additions and 1 deletion.
6 changes: 6 additions & 0 deletions docs/src/test-api/class-fullproject.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ See [`property: TestProject.repeatEach`].

See [`property: TestProject.retries`].

## property: FullProject.failOnFlakyTests
* since: v1.44
- type: <[boolean]>

See [`property: TestProject.failOnFlakyTests`].

## property: FullProject.teardown
* since: v1.34
- type: ?<[string]>
Expand Down
6 changes: 6 additions & 0 deletions docs/src/test-api/class-test.md
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,12 @@ Execution mode. Learn more about the execution modes [here](../test-parallel.md)

The number of retries for each test.

### option: Test.describe.configure.failOnFlakyTests
* since: v1.44
- `failOnFlakyTests` <[boolean]>

Fails flaky tests.

### option: Test.describe.configure.timeout
* since: v1.28
- `timeout` <[int]>
Expand Down
16 changes: 16 additions & 0 deletions docs/src/test-api/class-testconfig.md
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,22 @@ export default defineConfig({
});
```

## property: TestConfig.failOnFlakyTests
* since: v1.44
- type: ?<[boolean]>

Counts tests that initially fail but subsequently pass as failed. By default 'flaky' tests are considered as passing.

**Usage**

```js title="playwright.config.ts"
import { defineConfig } from '@playwright/test';

export default failOnFlakyTests({
failOnFlakyTests: true,
});
```

## property: TestConfig.shard
* since: v1.10
- type: ?<[null]|[Object]>
Expand Down
6 changes: 6 additions & 0 deletions docs/src/test-api/class-testproject.md
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,12 @@ Use [`method: Test.describe.configure`] to change the number of retries for a sp

Use [`property: TestConfig.retries`] to change this option for all projects.

## property: TestProject.failOnFlakyTests
* since: v1.44
- type: ?<[boolean]>


Counts tests that initially fail but subsequently pass as failed. By default 'flaky' tests are considered as passing.

## property: TestProject.teardown
* since: v1.34
Expand Down
2 changes: 2 additions & 0 deletions packages/playwright/src/common/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ export class FullProjectInternal {
// project is top-level vs dependency. See collectProjectsAndTestFiles in loadUtils.
repeatEach: takeFirst(projectConfig.repeatEach, config.repeatEach, 1),
retries: takeFirst(configCLIOverrides.retries, projectConfig.retries, config.retries, 0),
failOnFlakyTests: takeFirst(configCLIOverrides.failOnFlakyTests, projectConfig.failOnFlakyTests, config.failOnFlakyTests, false),
metadata: takeFirst(projectConfig.metadata, config.metadata, {}),
name: takeFirst(projectConfig.name, config.name, ''),
testDir,
Expand All @@ -187,6 +188,7 @@ export class FullProjectInternal {
dependencies: projectConfig.dependencies || [],
teardown: projectConfig.teardown,
};

this.fullyParallel = takeFirst(configCLIOverrides.fullyParallel, projectConfig.fullyParallel, config.fullyParallel, undefined);
this.expect = takeFirst(projectConfig.expect, config.expect, {});
if (this.expect.toHaveScreenshot?.stylePath) {
Expand Down
5 changes: 5 additions & 0 deletions packages/playwright/src/common/configLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,11 @@ function validateProject(file: string, project: Project, title: string) {
throw errorWithFile(file, `${title}.retries must be a non-negative number`);
}

if ('failOnFlakyTests' in project && project.failOnFlakyTests !== undefined) {
if (typeof project.failOnFlakyTests !== 'boolean')
throw errorWithFile(file, `${title}.failOnFlakyTests must be a boolean`);
}

if ('testDir' in project && project.testDir !== undefined) {
if (typeof project.testDir !== 'string')
throw errorWithFile(file, `${title}.testDir must be a string`);
Expand Down
1 change: 1 addition & 0 deletions packages/playwright/src/common/ipc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export type ConfigCLIOverrides = {
quiet?: boolean;
repeatEach?: number;
retries?: number;
failOnFlakyTests?: boolean;
reporter?: ReporterDescription[];
additionalReporters?: ReporterDescription[];
shard?: { current: number, total: number };
Expand Down
4 changes: 4 additions & 0 deletions packages/playwright/src/common/suiteUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export function bindFileSuiteToProject(project: FullProjectInternal, suite: Suit
// Inherit properties from parent suites.
let inheritedRetries: number | undefined;
let inheritedTimeout: number | undefined;
let inheritedfailOnFlakyTests: boolean | undefined;
test.annotations = [];
for (let parentSuite: Suite | undefined = suite; parentSuite; parentSuite = parentSuite.parent) {
if (parentSuite._staticAnnotations.length)
Expand All @@ -69,9 +70,12 @@ export function bindFileSuiteToProject(project: FullProjectInternal, suite: Suit
inheritedRetries = parentSuite._retries;
if (inheritedTimeout === undefined && parentSuite._timeout !== undefined)
inheritedTimeout = parentSuite._timeout;
if (inheritedfailOnFlakyTests === undefined && parentSuite._failOnFlakyTests !== undefined)
inheritedfailOnFlakyTests = parentSuite._failOnFlakyTests;
}
test.retries = inheritedRetries ?? project.project.retries;
test.timeout = inheritedTimeout ?? project.project.timeout;
test.failOnFlakyTests = inheritedfailOnFlakyTests ?? project.project.failOnFlakyTests;
test.annotations.push(...test._staticAnnotations);

// Skip annotations imply skipped expectedStatus.
Expand Down
7 changes: 6 additions & 1 deletion packages/playwright/src/common/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export class Suite extends Base {
_hooks: { type: 'beforeEach' | 'afterEach' | 'beforeAll' | 'afterAll', fn: Function, title: string, location: Location }[] = [];
_timeout: number | undefined;
_retries: number | undefined;
_failOnFlakyTests: boolean | undefined;
// Annotations known statically before running the test, e.g. `test.describe.skip()` or `test.describe({ annotation }, body)`.
_staticAnnotations: Annotation[] = [];
// Explicitly declared tags that are not a part of the title.
Expand Down Expand Up @@ -220,6 +221,7 @@ export class Suite extends Base {
suite._requireFile = data.requireFile;
suite._timeout = data.timeout;
suite._retries = data.retries;
suite._failOnFlakyTests = data.failOnFlakyTests;
suite._staticAnnotations = data.staticAnnotations;
suite._tags = data.tags;
suite._modifiers = data.modifiers;
Expand Down Expand Up @@ -254,6 +256,7 @@ export class TestCase extends Base implements reporterTypes.TestCase {
timeout = 0;
annotations: Annotation[] = [];
retries = 0;
failOnFlakyTests = false;
repeatEachIndex = 0;

_testType: TestTypeImpl;
Expand Down Expand Up @@ -286,7 +289,7 @@ export class TestCase extends Base implements reporterTypes.TestCase {

ok(): boolean {
const status = this.outcome();
return status === 'expected' || status === 'flaky' || status === 'skipped';
return status === 'expected' || (status === 'flaky' && !this.failOnFlakyTests) || status === 'skipped';
}

get tags(): string[] {
Expand All @@ -299,6 +302,7 @@ export class TestCase extends Base implements reporterTypes.TestCase {
id: this.id,
title: this.title,
retries: this.retries,
failOnFlakyTests: this.failOnFlakyTests,
timeout: this.timeout,
expectedStatus: this.expectedStatus,
location: this.location,
Expand All @@ -317,6 +321,7 @@ export class TestCase extends Base implements reporterTypes.TestCase {
const test = new TestCase(data.title, () => {}, rootTestType, data.location);
test.id = data.id;
test.retries = data.retries;
test.failOnFlakyTests = data.failOnFlakyTests;
test.timeout = data.timeout;
test.expectedStatus = data.expectedStatus;
test._only = data.only;
Expand Down
2 changes: 2 additions & 0 deletions packages/playwright/src/isomorphic/teleReceiver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export type JsonProject = {
outputDir: string;
repeatEach: number;
retries: number;
failOnFlakyTests: boolean;
suites: JsonSuite[];
teardown?: string;
// This is relative to root dir.
Expand Down Expand Up @@ -316,6 +317,7 @@ export class TeleReporterReceiver {
outputDir: this._absolutePath(project.outputDir),
repeatEach: project.repeatEach,
retries: project.retries,
failOnFlakyTests: project.failOnFlakyTests,
testDir: this._absolutePath(project.testDir),
testIgnore: parseRegexPatterns(project.testIgnore),
testMatch: parseRegexPatterns(project.testMatch),
Expand Down
2 changes: 2 additions & 0 deletions packages/playwright/src/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ function overridesFromOptions(options: { [key: string]: any }): ConfigCLIOverrid
quiet: options.quiet ? options.quiet : undefined,
repeatEach: options.repeatEach ? parseInt(options.repeatEach, 10) : undefined,
retries: options.retries ? parseInt(options.retries, 10) : undefined,
failOnFlakyTests: options.failOnFlakyTests ? true : undefined,
reporter: resolveReporterOption(options.reporter),
shard: shardPair ? { current: shardPair[0], total: shardPair[1] } : undefined,
timeout: options.timeout ? parseInt(options.timeout, 10) : undefined,
Expand Down Expand Up @@ -336,6 +337,7 @@ const testOptions: [string, string][] = [
['--browser <browser>', `Browser to use for tests, one of "all", "chromium", "firefox" or "webkit" (default: "chromium")`],
['-c, --config <file>', `Configuration file, or a test directory with optional "playwright.config.{m,c}?{js,ts}"`],
['--debug', `Run tests with Playwright Inspector. Shortcut for "PWDEBUG=1" environment variable and "--timeout=0 --max-failures=1 --headed --workers=1" options`],
['--fail-on-flaky-tests', `Fail if any test is flagged as flaky (default: false)`],
['--forbid-only', `Fail if test.only is called (default: false)`],
['--fully-parallel', `Run all tests in parallel (default: false)`],
['--global-timeout <timeout>', `Maximum time this test suite can run in milliseconds (default: unlimited)`],
Expand Down
1 change: 1 addition & 0 deletions packages/playwright/src/reporters/teleEmitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ export class TeleReporterEmitter implements ReporterV2 {
outputDir: this._relativePath(project.outputDir),
repeatEach: project.repeatEach,
retries: project.retries,
failOnFlakyTests: project.failOnFlakyTests,
testDir: this._relativePath(project.testDir),
testIgnore: serializeRegexPatterns(project.testIgnore),
testMatch: serializeRegexPatterns(project.testMatch),
Expand Down
30 changes: 30 additions & 0 deletions packages/playwright/types/test.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,12 @@ interface TestProject<TestArgs = {}, WorkerArgs = {}> {
};
};

/**
* Counts tests that initially fail but subsequently pass as failed. By default 'flaky' tests are considered as
* passing.
*/
failOnFlakyTests?: boolean;

/**
* Playwright Test runs tests in parallel. In order to achieve that, it runs several worker processes that run at the
* same time. By default, **test files** are run in parallel. Tests in a single file are run in order, in the same
Expand Down Expand Up @@ -644,6 +650,12 @@ export interface FullProject<TestArgs = {}, WorkerArgs = {}> {
*/
dependencies: Array<string>;

/**
* See
* [testProject.failOnFlakyTests](https://playwright.dev/docs/api/class-testproject#test-project-fail-on-flaky-tests).
*/
failOnFlakyTests: boolean;

/**
* See [testProject.grep](https://playwright.dev/docs/api/class-testproject#test-project-grep).
*/
Expand Down Expand Up @@ -1017,6 +1029,24 @@ interface TestConfig<TestArgs = {}, WorkerArgs = {}> {
};
};

/**
* Counts tests that initially fail but subsequently pass as failed. By default 'flaky' tests are considered as
* passing.
*
* **Usage**
*
* ```js
* // playwright.config.ts
* import { defineConfig } from '@playwright/test';
*
* export default failOnFlakyTests({
* failOnFlakyTests: true,
* });
* ```
*
*/
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
Expand Down
13 changes: 13 additions & 0 deletions tests/playwright-test/exit-code.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,19 @@ test('should allow flaky', async ({ runInlineTest }) => {
expect(result.flaky).toBe(1);
});

test('failOnFlakyTests flag disallows flaky', async ({ runInlineTest }) => {
const result = await runInlineTest({
'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 fail on unexpected pass', async ({ runInlineTest }) => {
const { exitCode, failed, output } = await runInlineTest({
'unexpected-pass.spec.js': `
Expand Down

0 comments on commit 5e27e31

Please sign in to comment.