Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/src/test-reporter-api/class-reporter.md
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,6 @@ Resolved configuration.

The root suite that contains the projects, files and test cases that will run.

The suite reflects `--project`, `--grep`/`--grep-invert` and `.only` filtering, so it only contains tests that match the current invocation. It contains only the top-level projects being run — setup and dependency projects are not included and cannot be excluded from here.
The suite reflects `--project`, `--grep`/`--grep-invert` and `.only` filtering, so it only contains tests that match the current invocation. Setup and dependency projects are readonly and cannot be excluded from here.

The suite ignores the `--shard` argument: it always contains the full, un-sharded corpus. Playwright applies its built-in sharding after [`method: Reporter.preprocessSuite`] returns, unless the returned `implementsSharding` is `true`.
13 changes: 13 additions & 0 deletions packages/playwright/src/common/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export class Suite extends Base {
_fullProject: FullProjectInternal | undefined;
_fileId: string | undefined;
_preprocessing = false;
_isDependency = false;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we unify _isDependency and _preprocessing into a single enum? I'd like Suite to be as independent as possible from the environment around it.

readonly _type: 'root' | 'project' | 'file' | 'describe';

skip: (reason?: string) => void;
Expand Down Expand Up @@ -272,6 +273,8 @@ export class Suite extends Base {
private _modifier(type: 'skip' | 'fixme' | 'fail', location: Location, reason: string | undefined): void {
if (!this._rootSuite()._preprocessing)
throw new Error(`Suite.${type}() can only be called from Reporter.preprocessSuite().`);
if (this._isInDependencyProject())
throw new Error(`Suite.${type}() cannot be called on a setup or teardown project; these always run in full.`);
for (const test of this.allTests())
test._applyPlanAnnotation({ type, description: reason, location });
}
Expand All @@ -281,9 +284,15 @@ export class Suite extends Base {
throw new Error(`Suite.exclude() can only be called from Reporter.preprocessSuite().`);
if (!this.parent)
throw new Error(`Suite.exclude() cannot be called on the root suite.`);
if (this._isInDependencyProject())
throw new Error(`Suite.exclude() cannot be called on a setup or teardown project; these always run in full.`);
this.parent._detach(this);
}

_isInDependencyProject(): boolean {
return this._isDependency || (this.parent?._isInDependencyProject() ?? false);
}

_rootSuite(): Suite {
return this.parent?._rootSuite() ?? this;
}
Expand Down Expand Up @@ -355,6 +364,8 @@ export class TestCase extends Base implements reporterTypes.TestCase {
private _modifier(type: 'skip' | 'fixme' | 'fail', location: Location, reason: string | undefined): void {
if (!this._rootSuite()._preprocessing)
throw new Error(`TestCase.${type}() can only be called from Reporter.preprocessSuite().`);
if (this.parent._isInDependencyProject())
throw new Error(`TestCase.${type}() cannot be called on a setup or teardown project test; these always run in full.`);
this._applyPlanAnnotation({ type, description: reason, location });
}

Expand All @@ -370,6 +381,8 @@ export class TestCase extends Base implements reporterTypes.TestCase {
exclude(): void {
if (!this._rootSuite()._preprocessing)
throw new Error(`TestCase.exclude() can only be called from Reporter.preprocessSuite().`);
if (this.parent._isInDependencyProject())
throw new Error(`TestCase.exclude() cannot be called on a setup or teardown project test; these always run in full.`);
this.parent._detach(this);
}

Expand Down
38 changes: 21 additions & 17 deletions packages/playwright/src/runner/loadUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,12 +165,23 @@ export async function createRootSuite(testRun: TestRun, errors: TestError[], sho
}
}

// Prepend dependency projects without filtration.
for (const [project, type] of projectClosure) {
if (type !== 'dependency')
continue;
const dependencySuite = buildProjectSuite(project, projectSuites.get(project)!);
dependencySuite._isDependency = true;
rootSuite._prependSuite(dependencySuite);
}

const preprocessResult = await testRun.reporter.preprocessSuite(config.config, rootSuite);
// Shard only the top-level projects.
if (config.config.shard && !preprocessResult?.implementsSharding) {
// Create test groups for top-level projects.
const testGroups: TestGroup[] = [];
for (const projectSuite of rootSuite.suites) {
if (projectSuite._isDependency)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of relying on the flag we set on suites, let's make decisions based on the local variables in this method, e.g. projectClosure.get(project) === 'dependency'

continue;
// Split beforeAll-grouped tests into "config.shard.total" groups when needed.
// Later on, we'll re-split them between workers by using "config.workers" instead.
for (const group of createTestGroups(projectSuite, config.config.shard.total))
Expand All @@ -186,26 +197,19 @@ export async function createRootSuite(testRun: TestRun, errors: TestError[], sho
}

// Update project suites, removing empty ones.
suiteUtils.filterTestsRemoveEmptySuites(rootSuite, test => testsInThisShard.has(test));
suiteUtils.filterTestsRemoveEmptySuites(rootSuite, test => test.parent._isInDependencyProject() || testsInThisShard.has(test));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, let's not rely on _isInDependencyProject().

}

if (testRun.postShardTestFilters.length)
suiteUtils.filterTestsRemoveEmptySuites(rootSuite, test => testRun.postShardTestFilters.every(filter => filter(test)));

const topLevelProjects = [];
// Now prepend dependency projects without filtration.
{
// Filtering 'only' and sharding might have reduced the number of top-level projects.
// Build the project closure to only include dependencies that are still needed.
const projectClosure = new Map(buildProjectsClosure(rootSuite.suites.map(suite => suite._fullProject!)));

// Clone file suites for dependency projects.
for (const [project, level] of projectClosure.entries()) {
if (level === 'dependency')
rootSuite._prependSuite(buildProjectSuite(project, projectSuites.get(project)!));
else
topLevelProjects.push(project);
}
suiteUtils.filterTestsRemoveEmptySuites(rootSuite, test => test.parent._isInDependencyProject() || testRun.postShardTestFilters.every(filter => filter(test)));

// Filtering 'only' and sharding might have reduced the number of top-level projects.
// Prune the project closure to only include dependencies that are still needed.
const topLevelProjects = rootSuite.suites.filter(suite => !suite._isDependency).map(suite => suite._fullProject!);
const neededClosure = buildProjectsClosure(topLevelProjects);
for (const projectSuite of [...rootSuite.suites]) {
if (projectSuite._isDependency && !neededClosure.has(projectSuite._fullProject!))
rootSuite._detach(projectSuite);
}

testRun.rootSuite = rootSuite;
Expand Down
3 changes: 1 addition & 2 deletions packages/playwright/types/testReporter.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,7 @@ export interface Reporter {
* @param suite The root suite that contains the projects, files and test cases that will run.
*
* The suite reflects `--project`, `--grep`/`--grep-invert` and `.only` filtering, so it only contains tests that
* match the current invocation. It contains only the top-level projects being run — setup and dependency projects are
* not included and cannot be excluded from here.
* match the current invocation. Setup and dependency projects are readonly and cannot be excluded from here.
*
* The suite ignores the `--shard` argument: it always contains the full, un-sharded corpus. Playwright applies its
* built-in sharding after
Expand Down
46 changes: 35 additions & 11 deletions tests/playwright-test/reporter-preprocess-suite.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -392,15 +392,29 @@ test('multiple reporters declaring implementsSharding throws', async ({ runInlin
expect(result.outputLines.join('\n')).toContain(`Multiple reporters declare 'implementsSharding'`);
});

test('plan.suite contains only top-level projects, not dependency/setup projects', async ({ runInlineTest }) => {
test('plan.suite exposes setup/teardown dependency projects but they are read-only', async ({ runInlineTest }) => {
const result = await runInlineTest({
'reporter.ts': `
class Reporter {
async preprocessSuite(config, suite) {
// The suite only exposes top-level projects, so a reporter has no handle on
// setup/dependency project tests and therefore cannot exclude them.
console.log('%% plan projects: ' + suite.suites.map(s => s.title).join(','));
console.log('%% plan tests: ' + suite.allTests().map(t => t.title).join(','));
const setupTest = suite.allTests().find(t => t.title === 'setup-test');
for (const method of ['skip', 'fixme', 'fail', 'exclude']) {
try {
setupTest[method]();
console.log('%% dep-' + method + ': no-throw');
} catch (e) {
console.log('%% dep-' + method + ': ' + e.message);
}
}
const setupProject = suite.suites.find(s => s.title === 'setup');
try {
setupProject.exclude();
console.log('%% dep-suite-exclude: no-throw');
} catch (e) {
console.log('%% dep-suite-exclude: ' + e.message);
}
}
onTestEnd(test, result) {
console.log('%% ran ' + test.parent.project().name + '/' + test.title);
Expand All @@ -412,7 +426,8 @@ test('plan.suite contains only top-level projects, not dependency/setup projects
module.exports = {
reporter: './reporter.ts',
projects: [
{ name: 'setup', testMatch: /a\\.setup\\.ts/ },
{ name: 'setup', testMatch: /a\\.setup\\.ts/, teardown: 'teardown' },
{ name: 'teardown', testMatch: /a\\.teardown\\.ts/ },
{ name: 'main', testMatch: /a\\.test\\.ts/, dependencies: ['setup'] },
],
};
Expand All @@ -421,18 +436,27 @@ test('plan.suite contains only top-level projects, not dependency/setup projects
import { test } from '@playwright/test';
test('setup-test', async () => {});
`,
'a.teardown.ts': `
import { test } from '@playwright/test';
test('teardown-test', async () => {});
`,
'a.test.ts': `
import { test } from '@playwright/test';
test('main-test', async () => {});
`,
}, { reporter: '', workers: 1 }, undefined, { additionalArgs: ['--project=main'] });

expect(result.exitCode).toBe(0);
// plan only sees the top-level 'main' project; the 'setup' dependency is prepended afterwards.
expect(result.outputLines).toContain('plan projects: main');
// 'setup-test' is absent from the plan suite, proving setup/dependency tests are not exposed.
expect(result.outputLines).toContain('plan tests: main-test');
// Both the dependency and the main project still run.
expect(result.outputLines).toContain('ran setup/setup-test');
expect(result.outputLines).toContain('ran main/main-test');
expect(result.outputLines).toEqual([
'plan projects: teardown,setup,main',
'plan tests: teardown-test,setup-test,main-test',
'dep-skip: TestCase.skip() cannot be called on a setup or teardown project test; these always run in full.',
'dep-fixme: TestCase.fixme() cannot be called on a setup or teardown project test; these always run in full.',
'dep-fail: TestCase.fail() cannot be called on a setup or teardown project test; these always run in full.',
'dep-exclude: TestCase.exclude() cannot be called on a setup or teardown project test; these always run in full.',
'dep-suite-exclude: Suite.exclude() cannot be called on a setup or teardown project; these always run in full.',
'ran setup/setup-test',
'ran main/main-test',
'ran teardown/teardown-test',
]);
});
Loading