Skip to content

Commit

Permalink
feat: implement cli flag to fail flaky tests
Browse files Browse the repository at this point in the history
  • Loading branch information
Joe-Hendley committed May 9, 2024
1 parent a5d384c commit be62f16
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 1 deletion.
1 change: 1 addition & 0 deletions docs/src/test-cli-js.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ Complete set of Playwright Test options is available in the [configuration file]
| `-c <file>` or `--config <file>`| Configuration file. If not passed, defaults to `playwright.config.ts` or `playwright.config.js` in the current directory. |
| `--debug`| Run tests with Playwright Inspector. Shortcut for `PWDEBUG=1` environment variable and `--timeout=0 --max-failures=1 --headed --workers=1` options.|
| `--forbid-only` | Whether to disallow `test.only`. Useful on CI.|
| `--fail-on-flaky-tests` | Fails test runs that contain flaky tests. By default flaky tests count as successes. |
| `--global-timeout <number>` | Total timeout for the whole test run in milliseconds. By default, there is no global timeout. Learn more about [various timeouts](./test-timeouts.md).|
| `-g <grep>` or `--grep <grep>` | Only run tests matching this regular expression. For example, this will run `'should add to cart'` when passed `-g "add to cart"`. The regular expression will be tested against the string that consists of the test file name, `test.describe` titles if any, test title and all test tags, separated by spaces, e.g. `my-test.spec.ts my-suite my-test @smoke`. The filter does not apply to the tests from dependency projects, i.e. Playwright will still run all tests from [project dependencies](./test-projects.md#dependencies). |
| `--grep-invert <grep>` | Only run tests **not** matching this regular expression. The opposite of `--grep`. The filter does not apply to the tests from dependency projects, i.e. Playwright will still run all tests from [project dependencies](./test-projects.md#dependencies).|
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 @@ -52,6 +52,7 @@ export class FullConfigInternal {
cliProjectFilter?: string[];
cliListOnly = false;
cliPassWithNoTests?: boolean;
cliFailOnFlakyTests?: boolean;
testIdMatcher?: Matcher;
defineConfigWasUsed = false;

Expand Down Expand Up @@ -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
10 changes: 9 additions & 1 deletion packages/playwright/src/common/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,10 @@ export class Suite extends Base {
project(): FullProject | undefined {
return this._fullProject?.project || this.parent?.project();
}

failOnFlakyTests(): boolean | undefined {
return this._fullProject?.fullConfig.cliFailOnFlakyTests || this.parent?.failOnFlakyTests();
}
}

export class TestCase extends Base implements reporterTypes.TestCase {
Expand Down Expand Up @@ -286,7 +290,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._passFlakyTests()) || status === 'skipped';
}

get tags(): string[] {
Expand Down Expand Up @@ -363,4 +367,8 @@ export class TestCase extends Base implements reporterTypes.TestCase {
path.push(...this._tags);
return path.join(' ');
}

_passFlakyTests(): boolean {
return !this.parent.failOnFlakyTests();
}
}
2 changes: 2 additions & 0 deletions packages/playwright/src/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ async function runTests(args: string[], opts: { [key: string]: any }) {
config.cliListOnly = !!opts.list;
config.cliProjectFilter = opts.project || undefined;
config.cliPassWithNoTests = !!opts.passWithNoTests;
config.cliFailOnFlakyTests = !!opts.failOnFlakyTests;

const runner = new Runner(config);
let status: FullResult['status'];
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
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 be62f16

Please sign in to comment.