Skip to content

chore: allow dependencies on teardown project #38664

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yury-s wants to merge 1 commit into
base: main
Choose a base branch
from
+97 −34
Open

chore: allow dependencies on teardown project #38664

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
45 changes: 40 additions & 5 deletions packages/playwright/src/common/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ function resolveWorkers(workers: string | number): number {
}

function resolveProjectDependencies(projects: FullProjectInternal[]) {
const teardownSet = new Set<FullProjectInternal>();
const teardownToSetups = new Map<FullProjectInternal, FullProjectInternal[]>();
for (const project of projects) {
for (const dependencyName of project.project.dependencies) {
const dependencies = projects.filter(p => p.project.name === dependencyName);
Expand All @@ -271,17 +271,27 @@ function resolveProjectDependencies(projects: FullProjectInternal[]) {
throw new Error(`Project teardowns should have unique names, reading ${project.project.teardown}`);
const teardown = teardowns[0];
project.teardown = teardown;
teardownSet.add(teardown);
const setups = teardownToSetups.get(teardown) || [];
setups.push(project);
teardownToSetups.set(teardown, setups);
}
}
for (const teardown of teardownSet) {
for (const [teardown] of teardownToSetups) {
if (teardown.deps.length)
throw new Error(`Teardown project ${teardown.project.name} must not have dependencies`);
}

for (const project of projects) {
for (const dep of project.deps) {
if (teardownSet.has(dep))
throw new Error(`Project ${project.project.name} must not depend on a teardown project ${dep.project.name}`);
if (!teardownToSetups.has(dep))
continue;
const closure = buildDependentProjects([...teardownToSetups.get(dep)!], projects);
for (const otherDep of project.deps) {
if (teardownToSetups.has(otherDep))
continue;
if (closure.has(otherDep))
throw new Error(`Project "${project.project.name}" depends on "${otherDep.project.name}" and on teardown project "${dep.project.name}" which creates a circular dependency. Remove one of the dependencies to fix the problem.`);
}
}
}
}
Expand Down Expand Up @@ -316,3 +326,28 @@ const configInternalSymbol = Symbol('configInternalSymbol');
export function getProjectId(project: FullProject): string {
return (project as any).__projectId!;
}

export function buildDependentProjects(forProjects: FullProjectInternal[], projects: FullProjectInternal[]): Set<FullProjectInternal> {
const reverseDeps = new Map<FullProjectInternal, Set<FullProjectInternal>>(projects.map(p => ([p, new Set()])));
for (const project of projects) {
for (const dep of project.deps)
reverseDeps.get(dep)!.add(project);
}
const result = new Set<FullProjectInternal>();
const visit = (depth: number, project: FullProjectInternal) => {
if (depth > 100) {
const error = new Error('Circular dependency detected between projects.');
error.stack = '';
throw error;
}
result.add(project);
for (const reverseDep of reverseDeps.get(project)!)
visit(depth + 1, reverseDep);
// Teadrdown project may have reverse dependencies, we ignore them here.
if (project.teardown)
result.add(project.teardown);
};
for (const forProject of forProjects)
visit(0, forProject);
return result;
}
24 changes: 0 additions & 24 deletions packages/playwright/src/runner/projectUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,30 +120,6 @@ export function findTopLevelProjects(config: FullConfigInternal): FullProjectInt
return [...closure].filter(entry => entry[1] === 'top-level').map(entry => entry[0]);
}

export function buildDependentProjects(forProjects: FullProjectInternal[], projects: FullProjectInternal[]): Set<FullProjectInternal> {
const reverseDeps = new Map<FullProjectInternal, FullProjectInternal[]>(projects.map(p => ([p, []])));
for (const project of projects) {
for (const dep of project.deps)
reverseDeps.get(dep)!.push(project);
}
const result = new Set<FullProjectInternal>();
const visit = (depth: number, project: FullProjectInternal) => {
if (depth > 100) {
const error = new Error('Circular dependency detected between projects.');
error.stack = '';
throw error;
}
result.add(project);
for (const reverseDep of reverseDeps.get(project)!)
visit(depth + 1, reverseDep);
if (project.teardown)
visit(depth + 1, project.teardown);
};
for (const forProject of forProjects)
visit(0, forProject);
return result;
}

export async function collectFilesForProject(project: FullProjectInternal, fsCache = new Map<string, string[]>()): Promise<string[]> {
const extensions = new Set(['.js', '.ts', '.mjs', '.mts', '.cjs', '.cts', '.jsx', '.tsx', '.mjsx', '.mtsx', '.cjsx', '.ctsx']);
const testFileExtension = (file: string) => extensions.has(path.extname(file));
Expand Down
6 changes: 5 additions & 1 deletion packages/playwright/src/runner/tasks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ import { debug } from 'playwright-core/lib/utilsBundle';
import { Dispatcher } from './dispatcher';
import { FailureTracker } from './failureTracker';
import { collectProjectsAndTestFiles, createRootSuite, loadFileSuites, loadGlobalHook, loadTestList } from './loadUtils';
import { buildDependentProjects, buildTeardownToSetupsMap, filterProjects } from './projectUtils';
import { buildTeardownToSetupsMap, filterProjects } from './projectUtils';
import { buildDependentProjects } from '../common/config';
import { applySuggestedRebaselines, clearSuggestedRebaselines } from './rebase';
import { TaskRunner } from './taskRunner';
import { detectChangedTestFiles } from './vcs';
Expand Down Expand Up @@ -361,6 +362,9 @@ function createPhasesTask(): Task<TestRun> {
}
}

if (processed.size !== projectToSuite.size)
throw new Error(`Circular dependency detected between projects: ${[...projectToSuite.keys()].filter(p => !processed.has(p)).map(p => p.project.name).join(', ')}`);

testRun.config.config.metadata.actualWorkers = Math.min(testRun.config.config.workers, maxConcurrentTestGroups);
},
};
Expand Down
56 changes: 52 additions & 4 deletions tests/playwright-test/deps.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ test('should run teardown after failure', async ({ runInlineTest }) => {
expect(result.outputLines).toEqual(['A', 'D']);
});

test('should complain about teardown being a dependency', async ({ runInlineTest }) => {
test('should allow dependency on a teardown project', async ({ runInlineTest }) => {
const result = await runInlineTest({
'playwright.config.ts': `
module.exports = {
Expand All @@ -515,11 +515,59 @@ test('should complain about teardown being a dependency', async ({ runInlineTest
};`,
'test.spec.ts': `
import { test, expect } from '@playwright/test';
test('test', () => {});
test('test', async ({}, testInfo) => {
console.log('\\n%%' + testInfo.project.name);
});
`,
});
}, { workers: 1 });
expect(result.exitCode).toBe(0);
expect(result.passed).toBe(3);
expect(result.outputLines).toEqual(['A', 'B', 'C']);
});

test('should allow dependency on a multipleteardown project', async ({ runInlineTest }) => {
const result = await runInlineTest({
'playwright.config.ts': `
module.exports = {
projects: [
{ name: 'A', teardown: 'A_teardown' },
Copy link
Member

@pavelfeldman pavelfeldman Dec 31, 2025

Choose a reason for hiding this comment

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

why not:

          { name: 'A' },
          { name: 'A_full', dependencies: ['A'] },
          { name: 'B', dependencies: ['A_full'] },
          { name: 'B_full', dependencies: ['B'] },
          { name: 'C', dependencies: ['A_full', 'B_full'] },

Copy link
Member Author

@yury-s yury-s Dec 31, 2025

Choose a reason for hiding this comment

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

You'd need to manually implement teardown and update the deps when projects depending on A change.

{ name: 'B', dependencies: ['A'], teardown: 'B_teardown' },
{ name: 'A_teardown' },
{ name: 'B_teardown' },
{ name: 'C', dependencies: ['A_teardown', 'B_teardown'] },
],
};`,
'test.spec.ts': `
import { test, expect } from '@playwright/test';
test('test', async ({}, testInfo) => {
console.log('\\n%%' + testInfo.project.name);
});
`,
}, { workers: 1 });
expect(result.exitCode).toBe(0);
expect(result.passed).toBe(5);
expect(result.outputLines).toEqual(['A', 'B', 'B_teardown', 'A_teardown', 'C']);
});

test('should complain about dependency on a project and its teardown', async ({ runInlineTest }) => {
const result = await runInlineTest({
'playwright.config.ts': `
module.exports = {
projects: [
{ name: 'A', teardown: 'B' },
{ name: 'B' },
{ name: 'C', dependencies: [ 'A', 'B'] },
],
};`,
'test.spec.ts': `
import { test, expect } from '@playwright/test';
test('test', async ({}, testInfo) => {
console.log('\\n%%' + testInfo.project.name);
});
`,
}, { workers: 1 });
expect(result.exitCode).toBe(1);
expect(result.output).toContain(`Project C must not depend on a teardown project B`);
expect(result.output).toContain(`Error: Project "C" depends on "A" and on teardown project "B" which creates a circular dependency. Remove one of the dependencies to fix the problem.`);
});

test('should complain about teardown having a dependency', async ({ runInlineTest }) => {
Expand Down