chore(ct): remove suite dependency by connecting dependency graphs at read time, not write time (#31794)

Broken out of https://github.com/microsoft/playwright/pull/31727 as per
@dgozman's
[request](https://github.com/microsoft/playwright/pull/31727#discussion_r1685793229).

The PR goal is to remove the `suite` argument from the Component
testing's Vite Plugin. `suite` is used to enrich Vite's dependency graph
with information about dependencies between test suites and helper
files. It essentially merges the Vite graph with the
`compilationCache.ts > fileDependencies` graph, and then writes the
result back into `compilationCache.ts > externalDependencies`.

By refactoring this to make the connection on the reading end in
`collectAffectedTestFiles`, we can drop the `suite` parameter.

We didn't yet have a test that depended on the dependency graph being
connected correctly between `fileDependencies` and
`externalDepedencies`, so I've [extended an existing
test](53a539938b)
to capture that.

packages/playwright-ct-core/src/cliOverrides.ts

@@ -18,7 +18,6 @@
 import { affectedTestFiles, cacheDir } from 'playwright/lib/transform/compilationCache';
 import { buildBundle } from './vitePlugin';
 import { resolveDirs } from './viteUtils';
-import type { Suite } from 'playwright/types/testReporter';
 import { runDevServer } from './devServer';
 import type { FullConfigInternal } from 'playwright/lib/common/config';
 import { removeFolderAndLogToConsole } from 'playwright/lib/runner/testServer';
@@ -30,8 +29,8 @@ export async function clearCacheCommand(config: FullConfigInternal) {
   await removeFolderAndLogToConsole(cacheDir);
 }
 
-export async function findRelatedTestFilesCommand(files: string[],  config: FullConfigInternal, suite: Suite) {
+export async function findRelatedTestFilesCommand(files: string[],  config: FullConfigInternal) {
-  await buildBundle(config.config, config.configDir, suite);
+  await buildBundle(config.config, config.configDir);
   return { testFiles: affectedTestFiles(files) };
 }
 

packages/playwright-ct-core/src/vitePlugin.ts

@@ -20,7 +20,7 @@ import type { AddressInfo } from 'net';
 import path from 'path';
 import { assert, calculateSha1, getPlaywrightVersion, isURLAvailable } from 'playwright-core/lib/utils';
 import { debug } from 'playwright-core/lib/utilsBundle';
-import { internalDependenciesForTestFile, setExternalDependencies } from 'playwright/lib/transform/compilationCache';
+import { setExternalDependencies } from 'playwright/lib/transform/compilationCache';
 import { stoppable } from 'playwright/lib/utilsBundle';
 import type { FullConfig, Suite } from 'playwright/types/testReporter';
 import type { PluginContext } from 'rollup';
@@ -49,7 +49,7 @@ export function createPlugin(): TestRunnerPlugin {
     },
 
     begin: async (suite: Suite) => {
-      const result = await buildBundle(config, configDir, suite);
+      const result = await buildBundle(config, configDir);
       if (!result)
         return;
 
@@ -87,7 +87,7 @@ type BuildInfo = {
   }
 };
 
-export async function buildBundle(config: FullConfig, configDir: string, suite: Suite): Promise<{ buildInfo: BuildInfo, viteConfig: Record<string, any> } | null> {
+export async function buildBundle(config: FullConfig, configDir: string): Promise<{ buildInfo: BuildInfo, viteConfig: Record<string, any> } | null> {
   const { registerSourceFile, frameworkPluginFactory } = frameworkConfig(config);
   {
     // Detect a running dev server and use it if available.
@@ -169,23 +169,13 @@ export async function buildBundle(config: FullConfig, configDir: string, suite:
 
   {
     // Update dependencies based on the vite build.
-    for (const projectSuite of suite.suites) {
+    for (const [importingFile, components] of componentsByImportingFile) {
-      for (const fileSuite of projectSuite.suites) {
+      const deps = new Set<string>();
-        // For every test file...
+      for (const component of components) {
-        const testFile = fileSuite.location!.file;
+        for (const d of buildInfo.deps[component])
-        const deps = new Set<string>();
+          deps.add(d);
-        // Collect its JS dependencies (helpers).
-        for (const file of [testFile, ...(internalDependenciesForTestFile(testFile) || [])]) {
-          // For each helper, get all the imported components.
-          for (const componentFile of componentsByImportingFile.get(file) || []) {
-            // For each component, get all the dependencies.
-            for (const d of buildInfo.deps[componentFile] || [])
-              deps.add(d);
-          }
-        }
-        // Now we have test file => all components along with dependencies.
-        setExternalDependencies(testFile, [...deps]);
       }
+      setExternalDependencies(importingFile, [...deps]);
     }
   }
 

packages/playwright/src/runner/runner.ts

@@ -144,7 +144,7 @@ export class Runner {
     const resolvedFiles = (files as string[]).map(file => path.resolve(process.cwd(), file));
     const override = (this._config.config as any)['@playwright/test']?.['cli']?.['find-related-test-files'];
     if (override)
-      return await override(resolvedFiles, this._config, result.suite);
+      return await override(resolvedFiles, this._config);
     return { testFiles: affectedTestFiles(resolvedFiles) };
   }
 }

packages/playwright/src/transform/compilationCache.ts

@@ -211,16 +211,27 @@ export function fileDependenciesForTest() {
   return fileDependencies;
 }
 
-export function collectAffectedTestFiles(dependency: string, testFileCollector: Set<string>) {
+export function collectAffectedTestFiles(changedFile: string, testFileCollector: Set<string>) {
-  if (fileDependencies.has(dependency))
+  const isTestFile = (file: string) => fileDependencies.has(file);
-    testFileCollector.add(dependency);
+
+  if (isTestFile(changedFile))
+    testFileCollector.add(changedFile);
+
   for (const [testFile, deps] of fileDependencies) {
-    if (deps.has(dependency))
+    if (deps.has(changedFile))
       testFileCollector.add(testFile);
   }
-  for (const [testFile, deps] of externalDependencies) {
+
-    if (deps.has(dependency))
+  for (const [importingFile, depsOfImportingFile] of externalDependencies) {
-      testFileCollector.add(testFile);
+    if (depsOfImportingFile.has(changedFile)) {
+      if (isTestFile(importingFile))
+        testFileCollector.add(importingFile);
+
+      for (const [testFile, depsOfTestFile] of fileDependencies) {
+        if (depsOfTestFile.has(importingFile))
+          testFileCollector.add(testFile);
+      }
+    }
   }
 }
 
@@ -237,8 +248,11 @@ export function internalDependenciesForTestFile(filename: string): Set<string> |
 
 export function dependenciesForTestFile(filename: string): Set<string> {
   const result = new Set<string>();
-  for (const dep of fileDependencies.get(filename) || [])
+  for (const testDependency of fileDependencies.get(filename) || []) {
-    result.add(dep);
+    result.add(testDependency);
+    for (const externalDependency of externalDependencies.get(testDependency) || [])
+      result.add(externalDependency);
+  }
   for (const dep of externalDependencies.get(filename) || [])
     result.add(dep);
   return result;

tests/playwright-test/watch.spec.ts

@@ -690,11 +690,15 @@ test('should run CT on indirect deps change', async ({ runWatchTest, writeFiles
       import './button.css';
       export const Button = () => <button>Button</button>;
     `,
+    'src/helper.tsx': `
+      import { Button } from "./button";
+      export const buttonInstance = <Button></Button>
+    `,
     'src/button.spec.tsx': `
       import { test, expect } from '@playwright/experimental-ct-react';
-      import { Button } from './button';
+      import { buttonInstance } from './helper';
       test('pass', async ({ mount }) => {
-        const component = await mount(<Button></Button>);
+        const component = await mount(buttonInstance);
         await expect(component).toHaveText('Button', { timeout: 1000 });
       });
     `,