From 11eb719d13a832e53acf76bff360530c4ce81299 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Tue, 18 Oct 2022 17:18:45 -0700 Subject: [PATCH] feat(runner): project run: "always" (#18160) Projects marked with `run: 'always'` are non shard-able and run after failures. --- docs/src/test-api/class-testproject.md | 22 ++--- packages/playwright-test/src/dispatcher.ts | 3 +- packages/playwright-test/src/loader.ts | 12 ++- packages/playwright-test/src/runner.ts | 41 ++++++--- packages/playwright-test/types/test.d.ts | 41 +++++---- tests/playwright-test/config.spec.ts | 6 +- tests/playwright-test/stages.spec.ts | 99 ++++++++++++---------- utils/generate_types/overrides-test.d.ts | 3 +- 8 files changed, 119 insertions(+), 108 deletions(-) diff --git a/docs/src/test-api/class-testproject.md b/docs/src/test-api/class-testproject.md index eb9c237b3a..2375b45019 100644 --- a/docs/src/test-api/class-testproject.md +++ b/docs/src/test-api/class-testproject.md @@ -105,12 +105,6 @@ const config: PlaywrightTestConfig = { export default config; ``` -## property: TestProject.canShard -* since: v1.28 -- type: ?<[boolean]> - -If set to false and the tests run with --shard command line option, all tests from this project will run in every shard. If not specified, the project can be split between several shards. - ## property: TestProject.expect * since: v1.10 - type: ?<[Object]> @@ -265,20 +259,20 @@ The maximum number of retry attempts given to failed tests. Learn more about [te Use [`property: TestConfig.retries`] to change this option for all projects. +## property: TestProject.run +* since: v1.28 +- type: ?<[RunMode]<"default"|"always">> + +If set to 'always' the project will always be executed regardless of previous failures in the same test run. If set to 'always' all tests from the project will run in each shard and won't be split. If omitted or set to 'default' the project will be skipped if there are test failures in the projects from the prior [`property: TestProject.stage`]'s. + ## property: TestProject.stage * since: v1.28 - type: ?<[int]> An integer number that defines when the project should run relative to other projects. Each project runs in exactly one stage. By default all projects run in stage 0. Stages with lower number run first. Several projects can run in -each stage. Exeution order between projecs in the same stage is undefined. - -## property: TestProject.stopOnFailure -* since: v1.28 -- type: ?<[boolean]> - -If set to true and the any test in the project fails all subsequent projects in the same playwright test run will -be skipped. +each stage. Execution order between projecs in the same stage is undefined. If any test from a stage fails all tests +from susequent stages are skipped, use [`property: TestProject.run`] to change this behavior. ## property: TestProject.testDir * since: v1.10 diff --git a/packages/playwright-test/src/dispatcher.ts b/packages/playwright-test/src/dispatcher.ts index 3034811761..4c129ce14b 100644 --- a/packages/playwright-test/src/dispatcher.ts +++ b/packages/playwright-test/src/dispatcher.ts @@ -30,8 +30,7 @@ export type TestGroup = { requireFile: string; repeatEachIndex: number; projectId: string; - stopOnFailure: boolean; - canShard: boolean; + run: 'default'|'always'; tests: TestCase[]; watchMode: boolean; }; diff --git a/packages/playwright-test/src/loader.ts b/packages/playwright-test/src/loader.ts index 0235ca41bf..4ef30cde57 100644 --- a/packages/playwright-test/src/loader.ts +++ b/packages/playwright-test/src/loader.ts @@ -278,8 +278,7 @@ export class Loader { const snapshotDir = takeFirst(projectConfig.snapshotDir, config.snapshotDir, testDir); const name = takeFirst(projectConfig.name, config.name, ''); const stage = takeFirst(projectConfig.stage, 0); - const stopOnFailure = takeFirst(projectConfig.stopOnFailure, false); - const canShard = takeFirst(projectConfig.canShard, true); + const run = takeFirst(projectConfig.run, 'default'); let screenshotsDir = takeFirst((projectConfig as any).screenshotsDir, (config as any).screenshotsDir, path.join(testDir, '__screenshots__', process.platform, name)); if (process.env.PLAYWRIGHT_DOCKER) { @@ -299,9 +298,8 @@ export class Loader { metadata: takeFirst(projectConfig.metadata, config.metadata, undefined), name, testDir, + run, stage, - stopOnFailure, - canShard, _respectGitIgnore: respectGitIgnore, snapshotDir, _screenshotsDir: screenshotsDir, @@ -618,9 +616,9 @@ function validateProject(file: string, project: Project, title: string) { throw errorWithFile(file, `${title}.stage must be an integer`); } - if ('stopOnFailure' in project && project.stopOnFailure !== undefined) { - if (typeof project.stopOnFailure !== 'boolean') - throw errorWithFile(file, `${title}.stopOnFailure must be a boolean`); + if ('run' in project && project.run !== undefined) { + if (project.run !== 'default' && project.run !== 'always') + throw errorWithFile(file, `${title}.run must be one of 'default', 'always'.`); } if ('testDir' in project && project.testDir !== undefined) { diff --git a/packages/playwright-test/src/runner.ts b/packages/playwright-test/src/runner.ts index 977db3a0e0..7966503197 100644 --- a/packages/playwright-test/src/runner.ts +++ b/packages/playwright-test/src/runner.ts @@ -347,11 +347,11 @@ export class Runner { return; // Each shard includes: - // - all non shardale tests and + // - all tests from `run: 'always'` projects (non shardale) and // - its portion of the shardable ones. let shardableTotal = 0; for (const projectSuite of rootSuite.suites) { - if (projectSuite.project()!.canShard) + if (projectSuite.project()!.run !== 'always') shardableTotal += projectSuite.allTests().length; } @@ -371,14 +371,14 @@ export class Runner { const shardedStage: TestGroup[] = []; for (const group of stage) { let includeGroupInShard = false; - if (group.canShard) { + if (group.run === 'always') { + includeGroupInShard = true; + } else { // Any test group goes to the shard that contains the first test of this group. // So, this shard gets any group that starts at [from; to) if (current >= from && current < to) includeGroupInShard = true; current += group.tests.length; - } else { - includeGroupInShard = true; } if (includeGroupInShard) { shardedStage.push(group); @@ -448,7 +448,12 @@ export class Runner { let sigintWatcher; let hasWorkerErrors = false; - for (const testGroups of concurrentTestGroups) { + let previousStageFailed = false; + for (let testGroups of concurrentTestGroups) { + if (previousStageFailed) + testGroups = this._skipTestsNotMarkedAsRunAlways(testGroups); + if (!testGroups.length) + continue; const dispatcher = new Dispatcher(this._loader, [...testGroups], this._reporter); sigintWatcher = new SigIntWatcher(); await Promise.race([dispatcher.run(), sigintWatcher.promise()]); @@ -461,11 +466,9 @@ export class Runner { hasWorkerErrors = dispatcher.hasWorkerErrors(); if (hasWorkerErrors) break; - const stopOnFailureGroups = testGroups.filter(group => group.stopOnFailure); - if (stopOnFailureGroups.some(testGroup => testGroup.tests.some(test => !test.ok()))) - break; if (sigintWatcher.hadSignal()) break; + previousStageFailed ||= testGroups.some(testGroup => testGroup.tests.some(test => !test.ok())); } if (sigintWatcher?.hadSignal()) { result.status = 'interrupted'; @@ -482,6 +485,23 @@ export class Runner { return result; } + private _skipTestsNotMarkedAsRunAlways(testGroups: TestGroup[]): TestGroup[] { + const runAlwaysGroups = []; + for (const group of testGroups) { + if (group.run === 'always') { + runAlwaysGroups.push(group); + } else { + for (const test of group.tests) { + const result = test._appendTestResult(); + this._reporter.onTestBegin?.(test, result); + result.status = 'skipped'; + this._reporter.onTestEnd?.(test, result); + } + } + } + return runAlwaysGroups; + } + private async _removeOutputDirs(options: RunOptions): Promise { const config = this._loader.fullConfig(); const outputDirs = new Set(); @@ -771,8 +791,7 @@ function createTestGroups(projectSuites: Suite[], workers: number): TestGroup[] requireFile: test._requireFile, repeatEachIndex: test.repeatEachIndex, projectId: test._projectId, - stopOnFailure: test.parent.project()!.stopOnFailure, - canShard: test.parent.project()!.canShard, + run: test.parent.project()!.run, tests: [], watchMode: false, }; diff --git a/packages/playwright-test/types/test.d.ts b/packages/playwright-test/types/test.d.ts index 9488bcf4bb..fa1579b539 100644 --- a/packages/playwright-test/types/test.d.ts +++ b/packages/playwright-test/types/test.d.ts @@ -259,19 +259,18 @@ export interface FullProject { /** * An integer number that defines when the project should run relative to other projects. Each project runs in exactly one * stage. By default all projects run in stage 0. Stages with lower number run first. Several projects can run in each - * stage. Exeution order between projecs in the same stage is undefined. + * stage. Execution order between projecs in the same stage is undefined. If any test from a stage fails all tests from + * susequent stages are skipped, use [testProject.run](https://playwright.dev/docs/api/class-testproject#test-project-run) + * to change this behavior. */ stage: number; /** - * If set to true and the any test in the project fails all subsequent projects in the same playwright test run will be - * skipped. + * If set to 'always' the project will always be executed regardless of previous failures in the same test run. If set to + * 'always' all tests from the project will run in each shard and won't be split. If omitted or set to 'default' the + * project will be skipped if there are test failures in the projects from the prior + * [testProject.stage](https://playwright.dev/docs/api/class-testproject#test-project-stage)'s. */ - stopOnFailure: boolean; - /** - * If set to false and the tests run with --shard command line option, all tests from this project will run in every shard. - * If not specified, the project can be split between several shards. - */ - canShard: boolean; + run: 'default'|'always'; /** * Directory that will be recursively scanned for test files. Defaults to the directory of the configuration file. * @@ -4303,12 +4302,6 @@ export interface TestError { * */ interface TestProject { - /** - * If set to false and the tests run with --shard command line option, all tests from this project will run in every shard. - * If not specified, the project can be split between several shards. - */ - canShard?: boolean; - /** * Configuration for the `expect` assertion library. * @@ -4481,17 +4474,21 @@ interface TestProject { retries?: number; /** - * An integer number that defines when the project should run relative to other projects. Each project runs in exactly one - * stage. By default all projects run in stage 0. Stages with lower number run first. Several projects can run in each - * stage. Exeution order between projecs in the same stage is undefined. + * If set to 'always' the project will always be executed regardless of previous failures in the same test run. If set to + * 'always' all tests from the project will run in each shard and won't be split. If omitted or set to 'default' the + * project will be skipped if there are test failures in the projects from the prior + * [testProject.stage](https://playwright.dev/docs/api/class-testproject#test-project-stage)'s. */ - stage?: number; + run?: "default"|"always"; /** - * If set to true and the any test in the project fails all subsequent projects in the same playwright test run will be - * skipped. + * An integer number that defines when the project should run relative to other projects. Each project runs in exactly one + * stage. By default all projects run in stage 0. Stages with lower number run first. Several projects can run in each + * stage. Execution order between projecs in the same stage is undefined. If any test from a stage fails all tests from + * susequent stages are skipped, use [testProject.run](https://playwright.dev/docs/api/class-testproject#test-project-run) + * to change this behavior. */ - stopOnFailure?: boolean; + stage?: number; /** * Directory that will be recursively scanned for test files. Defaults to the directory of the configuration file. diff --git a/tests/playwright-test/config.spec.ts b/tests/playwright-test/config.spec.ts index 541d58f133..7e124e4a30 100644 --- a/tests/playwright-test/config.spec.ts +++ b/tests/playwright-test/config.spec.ts @@ -519,12 +519,12 @@ test('should throw when project.stage is not an integer', async ({ runInlineTest expect(result.output).toContain(`config.projects[0].stage must be an integer`); }); -test('should throw when project.stopOnFailure is not a boolean', async ({ runInlineTest }) => { +test('should throw when project.run is not an expected string', async ({ runInlineTest }) => { const result = await runInlineTest({ 'playwright.config.ts': ` module.exports = { projects: [ - { name: 'a', stopOnFailure: 'yes' }, + { name: 'a', run: 'yes' }, ], }; `, @@ -535,5 +535,5 @@ test('should throw when project.stopOnFailure is not a boolean', async ({ runInl }); expect(result.exitCode).toBe(1); - expect(result.output).toContain(`config.projects[0].stopOnFailure must be a boolean`); + expect(result.output).toContain(`config.projects[0].run must be one of 'default', 'always'.`); }); diff --git a/tests/playwright-test/stages.spec.ts b/tests/playwright-test/stages.spec.ts index 8ad32eaa02..0f536403a5 100644 --- a/tests/playwright-test/stages.spec.ts +++ b/tests/playwright-test/stages.spec.ts @@ -208,59 +208,21 @@ test('should work with project filter', async ({ runGroups }, testInfo) => { expect(passed).toBe(3); }); -test('should continue after failures', async ({ runGroups }, testInfo) => { - const projectTemplates = { - 'a': { - stage: 1 - }, - 'b': { - stage: 2 - }, - 'c': { - stage: 2 - }, - 'd': { - stage: 4 - }, - 'e': { - stage: 4 - }, - }; - const configWithFiles = createConfigWithProjects(['a', 'b', 'c', 'd', 'e'], testInfo, projectTemplates); - configWithFiles[`b/b.spec.ts`] = ` - const { test } = pwt; - test('b test', async () => { - expect(1).toBe(2); - });`; - configWithFiles[`d/d.spec.ts`] = ` - const { test } = pwt; - test('d test', async () => { - expect(1).toBe(2); - });`; - const { exitCode, passed, failed, timeline } = await runGroups(configWithFiles); - expect(exitCode).toBe(1); - expect(failed).toBe(2); - expect(passed).toBe(3); - expect(projectNames(timeline)).toEqual(['a', 'b', 'c', 'd', 'e']); - expectRunBefore(timeline, ['a'], ['b', 'c', 'd', 'e']); // 1 < 2 - expectRunBefore(timeline, ['b', 'c'], ['d', 'e']); // 2 < 4 -}); - -test('should support stopOnFailire', async ({ runGroups }, testInfo) => { +test('should skip after failire by default', async ({ runGroups }, testInfo) => { const projectTemplates = { 'a': { stage: 1 }, 'b': { stage: 2, - stopOnFailure: true + run: 'default' }, 'c': { stage: 2 }, 'd': { stage: 4, - stopOnFailure: true // this is not important as the test is skipped + run: 'default' // this is not important as the test is skipped }, 'e': { stage: 4 @@ -280,12 +242,55 @@ test('should support stopOnFailire', async ({ runGroups }, testInfo) => { const { exitCode, passed, failed, skipped, timeline } = await runGroups(configWithFiles); expect(exitCode).toBe(1); expect(failed).toBe(1); - expect(passed).toBeLessThanOrEqual(2); // 'c' may either pass or be skipped. - expect(passed + skipped).toBe(4); - expect(projectNames(timeline)).not.toContainEqual(['d', 'e']); + expect(passed).toBe(2); // 'c' may either pass or be skipped. + expect(skipped).toBe(2); + expect(projectNames(timeline)).toEqual(['a', 'b', 'c', 'd', 'e']); + expectRunBefore(timeline, ['a'], ['b', 'c']); // 1 < 2 + expectRunBefore(timeline, ['b', 'c'], ['d', 'e']); // 2 < 4 }); -test('should split project if no canShard', async ({ runGroups }, testInfo) => { +test('should run after failire if run:always', async ({ runGroups }, testInfo) => { + const projectTemplates = { + 'a': { + stage: 1 + }, + 'b': { + stage: 2, + run: 'default' + }, + 'c': { + stage: 2 + }, + 'd': { + stage: 4, + run: 'always' + }, + 'e': { + stage: 4 + }, + 'f': { + stage: 10, + run: 'always' + }, + }; + const configWithFiles = createConfigWithProjects(['a', 'b', 'c', 'd', 'e', 'f'], testInfo, projectTemplates); + configWithFiles[`b/b.spec.ts`] = ` + const { test } = pwt; + test('b test', async () => { + expect(1).toBe(2); + });`; + const { exitCode, passed, failed, skipped, timeline } = await runGroups(configWithFiles); + expect(exitCode).toBe(1); + expect(passed).toBe(4); + expect(failed).toBe(1); + expect(skipped).toBe(1); + expect(projectNames(timeline)).toEqual(['a', 'b', 'c', 'd', 'e', 'f']); + expectRunBefore(timeline, ['a'], ['b', 'c']); // 1 < 2 + expectRunBefore(timeline, ['b', 'c'], ['d', 'e']); // 2 < 4 + expectRunBefore(timeline, ['d', 'e'], ['f']); // 4 < 10 +}); + +test('should split project if no run: always', async ({ runGroups }, testInfo) => { const files = { 'playwright.config.ts': ` module.exports = { @@ -347,7 +352,7 @@ test('should split project if no canShard', async ({ runGroups }, testInfo) => { } }); -test('should not split project with canShard=false', async ({ runGroups }, testInfo) => { +test('should not split project with run: awlays', async ({ runGroups }, testInfo) => { const files = { 'playwright.config.ts': ` module.exports = { @@ -356,7 +361,7 @@ test('should not split project with canShard=false', async ({ runGroups }, testI stage: 10, name: 'proj-1', testMatch: /.*(a|b).test.ts/, - canShard: false, + run: 'always', }, { stage: 20, diff --git a/utils/generate_types/overrides-test.d.ts b/utils/generate_types/overrides-test.d.ts index 2e1f339aed..f784b81672 100644 --- a/utils/generate_types/overrides-test.d.ts +++ b/utils/generate_types/overrides-test.d.ts @@ -47,8 +47,7 @@ export interface FullProject { repeatEach: number; retries: number; stage: number; - stopOnFailure: boolean; - canShard: boolean; + run: 'default'|'always'; testDir: string; testIgnore: string | RegExp | (string | RegExp)[]; testMatch: string | RegExp | (string | RegExp)[];