chore: resolve top-level vs dependency after cli filtering (#24216)

packages/playwright-test/src/common/suiteUtils.ts

@@ -39,8 +39,7 @@ export function filterTestsRemoveEmptySuites(suite: Suite, filter: (test: TestCa
   suite._entries = suite._entries.filter(e => entries.has(e)); // Preserve the order.
   return !!suite._entries.length;
 }
-
-export function buildFileSuiteForProject(project: FullProjectInternal, suite: Suite, repeatEachIndex: number): Suite {
+export function bindFileSuiteToProject(project: FullProjectInternal, suite: Suite): Suite {
   const relativeFile = path.relative(project.project.testDir, suite.location!.file).split(path.sep).join('/');
   const fileId = calculateSha1(relativeFile).slice(0, 20);
 
@@ -51,13 +50,10 @@ export function buildFileSuiteForProject(project: FullProjectInternal, suite: Su
   // Assign test properties with project-specific values.
   result.forEachTest((test, suite) => {
     suite._fileId = fileId;
-    const repeatEachIndexSuffix = repeatEachIndex ? ` (repeat:${repeatEachIndex})` : '';
-
     // At the point of the query, suite is not yet attached to the project, so we only get file, describe and test titles.
-    const testIdExpression = `[project=${project.id}]${test.titlePath().join('\x1e')}${repeatEachIndexSuffix}`;
+    const testIdExpression = `[project=${project.id}]${test.titlePath().join('\x1e')}`;
     const testId = fileId + '-' + calculateSha1(testIdExpression).slice(0, 20);
     test.id = testId;
-    test.repeatEachIndex = repeatEachIndex;
     test._projectId = project.id;
 
     // Inherit properties from parent suites.
@@ -79,12 +75,27 @@ export function buildFileSuiteForProject(project: FullProjectInternal, suite: Su
 
     // We only compute / set digest in the runner.
     if (test._poolDigest)
-      test._workerHash = `${project.id}-${test._poolDigest}-${repeatEachIndex}`;
+      test._workerHash = `${project.id}-${test._poolDigest}-0`;
   });
 
   return result;
 }
 
+export function applyRepeatEachIndex(project: FullProjectInternal, fileSuite: Suite, repeatEachIndex: number) {
+  // Assign test properties with project-specific values.
+  fileSuite.forEachTest((test, suite) => {
+    if (repeatEachIndex) {
+      const testIdExpression = `[project=${project.id}]${test.titlePath().join('\x1e')} (repeat:${repeatEachIndex})`;
+      const testId = suite._fileId + '-' + calculateSha1(testIdExpression).slice(0, 20);
+      test.id = testId;
+      test.repeatEachIndex = repeatEachIndex;
+
+      if (test._poolDigest)
+        test._workerHash = `${project.id}-${test._poolDigest}-${repeatEachIndex}`;
+    }
+  });
+}
+
 export function filterOnly(suite: Suite) {
   if (!suite._getOnlyItems().length)
     return;

packages/playwright-test/src/common/test.ts

@@ -97,6 +97,22 @@ export class Suite extends Base implements SuitePrivate {
     return result;
   }
 
+  _hasTests(): boolean {
+    let result = false;
+    const visit = (suite: Suite) => {
+      for (const entry of suite._entries) {
+        if (result)
+          return;
+        if (entry instanceof Suite)
+          visit(entry);
+        else
+          result = true;
+      }
+    };
+    visit(this);
+    return result;
+  }
+
   titlePath(): string[] {
     const titlePath = this.parent ? this.parent.titlePath() : [];
     // Ignore anonymous describe blocks.
@@ -172,6 +188,7 @@ export class Suite extends Base implements SuitePrivate {
       modifiers: this._modifiers.slice(),
       parallelMode: this._parallelMode,
       hooks: this._hooks.map(h => ({ type: h.type, location: h.location })),
+      fileId: this._fileId,
     };
   }
 
@@ -186,6 +203,7 @@ export class Suite extends Base implements SuitePrivate {
     suite._modifiers = data.modifiers;
     suite._parallelMode = data.parallelMode;
     suite._hooks = data.hooks.map((h: any) => ({ type: h.type, location: h.location, fn: () => { } }));
+    suite._fileId = data.fileId;
     return suite;
   }
 
@@ -256,23 +274,33 @@ export class TestCase extends Base implements reporterTypes.TestCase {
   _serialize(): any {
     return {
       kind: 'test',
+      id: this.id,
       title: this.title,
+      retries: this.retries,
+      timeout: this.timeout,
+      expectedStatus: this.expectedStatus,
       location: this.location,
       only: this._only,
       requireFile: this._requireFile,
       poolDigest: this._poolDigest,
-      expectedStatus: this.expectedStatus,
+      workerHash: this._workerHash,
       staticAnnotations: this._staticAnnotations.slice(),
+      projectId: this._projectId,
     };
   }
 
   static _parse(data: any): TestCase {
     const test = new TestCase(data.title, () => {}, rootTestType, data.location);
+    test.id = data.id;
+    test.retries = data.retries;
+    test.timeout = data.timeout;
+    test.expectedStatus = data.expectedStatus;
     test._only = data.only;
     test._requireFile = data.requireFile;
     test._poolDigest = data.poolDigest;
-    test.expectedStatus = data.expectedStatus;
+    test._workerHash = data.workerHash;
     test._staticAnnotations = data.staticAnnotations;
+    test._projectId = data.projectId;
     return test;
   }
 

packages/playwright-test/src/runner/loadUtils.ts

@@ -26,7 +26,7 @@ import type { Matcher, TestFileFilter } from '../util';
 import { buildProjectsClosure, collectFilesForProject, filterProjects } from './projectUtils';
 import type { TestRun } from './tasks';
 import { requireOrImport } from '../transform/transform';
-import { buildFileSuiteForProject, filterByFocusedLine, filterByTestIds, filterOnly, filterTestsRemoveEmptySuites } from '../common/suiteUtils';
+import { applyRepeatEachIndex, bindFileSuiteToProject, filterByFocusedLine, filterByTestIds, filterOnly, filterTestsRemoveEmptySuites } from '../common/suiteUtils';
 import { createTestGroups, filterForShard, type TestGroup } from './testGroups';
 import { dependenciesForTestFile } from '../transform/compilationCache';
 import { sourceMapSupport } from '../utilsBundle';
@@ -73,15 +73,8 @@ export async function collectProjectsAndTestFiles(testRun: TestRun, additionalFi
     }
   }
 
-  // Apply overrides that are only applicable to top-level projects.
-  for (const [project, type] of projectClosure) {
-    if (type === 'top-level')
-      project.project.repeatEach = project.fullConfig.configCLIOverrides.repeatEach ?? project.project.repeatEach;
-  }
-
   testRun.projects = [...filesToRunByProject.keys()];
   testRun.projectFiles = filesToRunByProject;
-  testRun.projectType = projectClosure;
   testRun.projectSuites = new Map();
 }
 
@@ -129,8 +122,10 @@ export async function createRootSuite(testRun: TestRun, errors: TestError[], sho
   const config = testRun.config;
   // Create root suite, where each child will be a project suite with cloned file suites inside it.
   const rootSuite = new Suite('', 'root');
+  const projectSuites = new Map<FullProjectInternal, Suite>();
+  const filteredProjectSuites = new Map<FullProjectInternal, Suite>();
 
-  // First add top-level projects, so that we can filterOnly and shard just top-level.
+  // Filter all the projects using grep, testId, file names.
   {
     // Interpret cli parameters.
     const cliFileFilters = createFileFiltersFromArguments(config.cliArgs);
@@ -138,10 +133,21 @@ export async function createRootSuite(testRun: TestRun, errors: TestError[], sho
     const grepInvertMatcher = config.cliGrepInvert ? createTitleMatcher(forceRegExp(config.cliGrepInvert)) : () => false;
     const cliTitleMatcher = (title: string) => !grepInvertMatcher(title) && grepMatcher(title);
 
-    // Clone file suites for top-level projects.
+    // Filter file suites for all projects.
     for (const [project, fileSuites] of testRun.projectSuites) {
-      if (testRun.projectType.get(project) === 'top-level')
-        rootSuite._addSuite(await createProjectSuite(fileSuites, project, { cliFileFilters, cliTitleMatcher, testIdMatcher: config.testIdMatcher }));
+      const projectSuite = createProjectSuite(project, fileSuites);
+      projectSuites.set(project, projectSuite);
+      const filteredProjectSuite = filterProjectSuite(projectSuite, { cliFileFilters, cliTitleMatcher, testIdMatcher: config.testIdMatcher });
+      filteredProjectSuites.set(project, filteredProjectSuite);
+    }
+  }
+
+  // Add post-filtered top-level projects to the root suite for sharding and 'only' processing.
+  const projectClosure = buildProjectsClosure([...filteredProjectSuites.keys()], project => filteredProjectSuites.get(project)!._hasTests());
+  for (const [project, type] of projectClosure) {
+    if (type === 'top-level') {
+      project.project.repeatEach = project.fullConfig.configCLIOverrides.repeatEach ?? project.project.repeatEach;
+      rootSuite._addSuite(buildProjectSuite(project, filteredProjectSuites.get(project)!));
     }
   }
 
@@ -177,36 +183,26 @@ export async function createRootSuite(testRun: TestRun, errors: TestError[], sho
     filterTestsRemoveEmptySuites(rootSuite, test => testsInThisShard.has(test));
   }
 
-  // Now prepend dependency projects.
+  // Now prepend dependency projects without filtration.
   {
-    // Filtering only and sharding might have reduced the number of top-level projects.
+    // 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, fileSuites] of testRun.projectSuites) {
-      if (testRun.projectType.get(project) === 'dependency' && projectClosure.has(project))
-        rootSuite._prependSuite(await createProjectSuite(fileSuites, project, { cliFileFilters: [], cliTitleMatcher: undefined }));
+    for (const project of projectClosure.keys()) {
+      if (projectClosure.get(project) === 'dependency')
+        rootSuite._prependSuite(buildProjectSuite(project, projectSuites.get(project)!));
     }
   }
 
   return rootSuite;
 }
 
-async function createProjectSuite(fileSuites: Suite[], project: FullProjectInternal, options: { cliFileFilters: TestFileFilter[], cliTitleMatcher?: Matcher, testIdMatcher?: Matcher }): Promise<Suite> {
+function createProjectSuite(project: FullProjectInternal, fileSuites: Suite[]): Suite {
   const projectSuite = new Suite(project.project.name, 'project');
-  projectSuite._fullProject = project;
-  if (project.fullyParallel)
-    projectSuite._parallelMode = 'parallel';
-  for (const fileSuite of fileSuites) {
-    for (let repeatEachIndex = 0; repeatEachIndex < project.project.repeatEach; repeatEachIndex++) {
-      const builtSuite = buildFileSuiteForProject(project, fileSuite, repeatEachIndex);
-      projectSuite._addSuite(builtSuite);
-    }
-  }
-
-  filterByFocusedLine(projectSuite, options.cliFileFilters);
-  filterByTestIds(projectSuite, options.testIdMatcher);
+  for (const fileSuite of fileSuites)
+    projectSuite._addSuite(bindFileSuiteToProject(project, fileSuite));
 
   const grepMatcher = createTitleMatcher(project.project.grep);
   const grepInvertMatcher = project.project.grepInvert ? createTitleMatcher(project.project.grepInvert) : null;
@@ -215,13 +211,49 @@ async function createProjectSuite(fileSuites: Suite[], project: FullProjectInter
     const grepTitle = test.titlePath().join(' ');
     if (grepInvertMatcher?.(grepTitle))
       return false;
-    return grepMatcher(grepTitle) && (!options.cliTitleMatcher || options.cliTitleMatcher(grepTitle));
+    return grepMatcher(grepTitle);
   };
 
   filterTestsRemoveEmptySuites(projectSuite, titleMatcher);
   return projectSuite;
 }
 
+function filterProjectSuite(projectSuite: Suite, options: { cliFileFilters: TestFileFilter[], cliTitleMatcher?: Matcher, testIdMatcher?: Matcher }): Suite {
+  // Fast path.
+  if (!options.cliFileFilters.length && !options.cliTitleMatcher && !options.testIdMatcher)
+    return projectSuite;
+
+  const result = projectSuite._deepClone();
+  if (options.cliFileFilters.length)
+    filterByFocusedLine(result, options.cliFileFilters);
+  if (options.testIdMatcher)
+    filterByTestIds(result, options.testIdMatcher);
+  const titleMatcher = (test: TestCase) => {
+    return !options.cliTitleMatcher || options.cliTitleMatcher(test.titlePath().join(' '));
+  };
+  filterTestsRemoveEmptySuites(result, titleMatcher);
+  return result;
+}
+
+function buildProjectSuite(project: FullProjectInternal, projectSuite: Suite): Suite {
+  const result = new Suite(project.project.name, 'project');
+  result._fullProject = project;
+  if (project.fullyParallel)
+    result._parallelMode = 'parallel';
+
+  for (const fileSuite of projectSuite.suites) {
+    // Fast path for the repeatEach = 0.
+    result._addSuite(fileSuite);
+
+    for (let repeatEachIndex = 1; repeatEachIndex < project.project.repeatEach; repeatEachIndex++) {
+      const clone = fileSuite._deepClone();
+      applyRepeatEachIndex(project, clone, repeatEachIndex);
+      result._addSuite(clone);
+    }
+  }
+  return result;
+}
+
 function createForbidOnlyErrors(onlyTestsAndSuites: (TestCase | Suite)[], forbidOnlyCLIFlag: boolean | undefined, configFilePath: string | undefined): TestError[] {
   const errors: TestError[] = [];
   for (const testOrSuite of onlyTestsAndSuites) {

packages/playwright-test/src/runner/projectUtils.ts

@@ -61,7 +61,7 @@ export function buildTeardownToSetupsMap(projects: FullProjectInternal[]): Map<F
   return result;
 }
 
-export function buildProjectsClosure(projects: FullProjectInternal[]): Map<FullProjectInternal, 'top-level' | 'dependency'> {
+export function buildProjectsClosure(projects: FullProjectInternal[], hasTests?: (project: FullProjectInternal) => boolean): Map<FullProjectInternal, 'top-level' | 'dependency'> {
   const result = new Map<FullProjectInternal, 'top-level' | 'dependency'>();
   const visit = (depth: number, project: FullProjectInternal) => {
     if (depth > 100) {
@@ -69,13 +69,19 @@ export function buildProjectsClosure(projects: FullProjectInternal[]): Map<FullP
       error.stack = '';
       throw error;
     }
+
+    const projectHasTests = hasTests ? hasTests(project) : true;
+    if (!projectHasTests && depth === 0)
+      return;
+
+    if (result.get(project) !== 'dependency')
       result.set(project, depth ? 'dependency' : 'top-level');
-    project.deps.map(visit.bind(undefined, depth + 1));
+
+    for (const dep of project.deps)
+      visit(depth + 1, dep);
     if (project.teardown)
       visit(depth + 1, project.teardown);
   };
-  for (const p of projects)
-    result.set(p, 'top-level');
   for (const p of projects)
     visit(0, p);
   return result;

packages/playwright-test/src/runner/tasks.ts

@@ -52,7 +52,6 @@ export class TestRun {
   readonly phases: Phase[] = [];
   projects: FullProjectInternal[] = [];
   projectFiles: Map<FullProjectInternal, string[]> = new Map();
-  projectType: Map<FullProjectInternal, 'top-level' | 'dependency'> = new Map();
   projectSuites: Map<FullProjectInternal, Suite[]> = new Map();
 
   constructor(config: FullConfigInternal, reporter: ReporterV2) {

packages/playwright-test/src/worker/workerMain.ts

@@ -28,7 +28,7 @@ import { TestInfoImpl } from './testInfo';
 import { TimeoutManager, type TimeSlot } from './timeoutManager';
 import { ProcessRunner } from '../common/process';
 import { loadTestFile } from '../common/testLoader';
-import { buildFileSuiteForProject, filterTestsRemoveEmptySuites } from '../common/suiteUtils';
+import { applyRepeatEachIndex, bindFileSuiteToProject, filterTestsRemoveEmptySuites } from '../common/suiteUtils';
 import { PoolBuilder } from '../common/poolBuilder';
 import type { TestInfoError } from '../../types/test';
 
@@ -202,7 +202,9 @@ export class WorkerMain extends ProcessRunner {
     try {
       await this._loadIfNeeded();
       const fileSuite = await loadTestFile(runPayload.file, this._config.config.rootDir);
-      const suite = buildFileSuiteForProject(this._project, fileSuite, this._params.repeatEachIndex);
+      const suite = bindFileSuiteToProject(this._project, fileSuite);
+      if (this._params.repeatEachIndex)
+        applyRepeatEachIndex(this._project, suite, this._params.repeatEachIndex);
       const hasEntries = filterTestsRemoveEmptySuites(suite, test => entries.has(test.id));
       if (hasEntries) {
         this._poolBuilder.buildPools(suite);

packages/trace-viewer/src/ui/uiModeView.tsx

@@ -883,13 +883,14 @@ function createTree(rootSuite: Suite | undefined, loadErrors: TestError[], proje
       const fileItem = getFileItem(rootItem, fileSuite.location!.file.split(pathSeparator), true, fileMap);
       visitSuite(projectSuite.title, fileSuite, fileItem);
     }
+  }
+
   for (const loadError of loadErrors) {
     if (!loadError.location)
       continue;
     const fileItem = getFileItem(rootItem, loadError.location.file.split(pathSeparator), true, fileMap);
     fileItem.hasLoadErrors = true;
   }
-  }
   return rootItem;
 }
 

tests/page/page-check.spec.ts

@@ -17,28 +17,6 @@
 
 import { test as it, expect } from './pageTest';
 
-it('console.log', async ({ page }) => {
-  await page.setContent(`<input id='checkbox' type='checkbox'></input>`);
-  await page.check('input');
-  expect(await page.evaluate(() => window['checkbox'].checked)).toBe(true);
-
-  await page.evaluate(() => {
-    console.log('1');
-    console.log('2');
-    console.log(window);
-    console.log({ a: 2 });
-  });
-
-  await page.setContent(`<input id='checkbox' type='checkbox' checked></input>`);
-  await page.check('input');
-  expect(await page.evaluate(() => window['checkbox'].checked)).toBe(true);
-
-  await page.setContent(`<input id='checkbox' type='checkbox'></input>`);
-  await page.uncheck('input');
-  expect(await page.evaluate(() => window['checkbox'].checked)).toBe(false);
-
-});
-
 it('should check the box @smoke', async ({ page }) => {
   await page.setContent(`<input id='checkbox' type='checkbox'></input>`);
   await page.check('input');

tests/playwright-test/deps.spec.ts

@@ -584,3 +584,62 @@ test('should only apply --repeat-each to top-level', async ({ runInlineTest }) =
   expect(result.passed).toBe(5);
   expect(result.outputLines).toEqual(['A', 'B', 'B', 'C', 'C']);
 });
+
+test('should run teardown when all projects are top-level at run point', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'playwright.config.ts': `
+      module.exports = {
+        projects: [
+          { name: 'setup', teardown: 'teardown' },
+          { name: 'teardown' },
+          { name: 'project', dependencies: ['setup'] },
+        ],
+      };`,
+    'test.spec.ts': `
+      import { test, expect } from '@playwright/test';
+      test('test', async ({}, testInfo) => {
+        console.log('\\n%%' + testInfo.project.name);
+      });
+    `,
+  }, { workers: 1 }, undefined, { additionalArgs: ['test.spec.ts'] });
+  expect(result.exitCode).toBe(0);
+  expect(result.passed).toBe(3);
+  expect(result.outputLines).toEqual(['setup', 'project', 'teardown']);
+});
+
+test('should not run deps for projects filtered with grep', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'playwright.config.ts': `
+      module.exports = {
+        projects: [
+          { name: 'setupA', teardown: 'teardownA', testMatch: '**/hook.spec.ts' },
+          { name: 'teardownA', testMatch: '**/hook.spec.ts' },
+          { name: 'projectA', dependencies: ['setupA'], testMatch: '**/a.spec.ts' },
+          { name: 'setupB', teardown: 'teardownB', testMatch: '**/hook.spec.ts' },
+          { name: 'teardownB', testMatch: '**/hook.spec.ts' },
+          { name: 'projectB', dependencies: ['setupB'], testMatch: '**/b.spec.ts' },
+        ],
+      };`,
+    'a.spec.ts': `
+      import { test, expect } from '@playwright/test';
+      test('test', async ({}, testInfo) => {
+        console.log('\\n%%' + testInfo.project.name);
+      });
+    `,
+    'hook.spec.ts': `
+      import { test, expect } from '@playwright/test';
+      test('test', async ({}, testInfo) => {
+        console.log('\\n%%' + testInfo.project.name);
+      });
+    `,
+    'b.spec.ts': `
+      import { test, expect } from '@playwright/test';
+      test('test', async ({}, testInfo) => {
+        console.log('\\n%%' + testInfo.project.name);
+      });
+    `,
+  }, { workers: 1 }, undefined, { additionalArgs: ['--grep=b.spec.ts'] });
+  expect(result.exitCode).toBe(0);
+  expect(result.passed).toBe(3);
+  expect(result.outputLines).toEqual(['setupB', 'projectB', 'teardownB']);
+});