diff --git a/packages/playwright/src/common/config.ts b/packages/playwright/src/common/config.ts index 4b6c466cd41e7..6c59da49171be 100644 --- a/packages/playwright/src/common/config.ts +++ b/packages/playwright/src/common/config.ts @@ -253,7 +253,7 @@ function resolveWorkers(workers: string | number): number { } function resolveProjectDependencies(projects: FullProjectInternal[]) { - const teardownSet = new Set(); + const teardownToSetups = new Map(); for (const project of projects) { for (const dependencyName of project.project.dependencies) { const dependencies = projects.filter(p => p.project.name === dependencyName); @@ -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.`); + } } } } @@ -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 { + const reverseDeps = new Map>(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(); + 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; +} diff --git a/packages/playwright/src/runner/projectUtils.ts b/packages/playwright/src/runner/projectUtils.ts index 3ad639a9955ce..4e1d87bfecfb8 100644 --- a/packages/playwright/src/runner/projectUtils.ts +++ b/packages/playwright/src/runner/projectUtils.ts @@ -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 { - const reverseDeps = new Map(projects.map(p => ([p, []]))); - for (const project of projects) { - for (const dep of project.deps) - reverseDeps.get(dep)!.push(project); - } - const result = new Set(); - 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()): Promise { 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)); diff --git a/packages/playwright/src/runner/tasks.ts b/packages/playwright/src/runner/tasks.ts index 96cadb02d923b..00c2a84f38084 100644 --- a/packages/playwright/src/runner/tasks.ts +++ b/packages/playwright/src/runner/tasks.ts @@ -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'; @@ -361,6 +362,9 @@ function createPhasesTask(): Task { } } + 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); }, }; diff --git a/tests/playwright-test/deps.spec.ts b/tests/playwright-test/deps.spec.ts index 26417bd7bc9b0..546a45ac878ef 100644 --- a/tests/playwright-test/deps.spec.ts +++ b/tests/playwright-test/deps.spec.ts @@ -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 = { @@ -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' }, + { 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 }) => {