chore: more watch tests (#20797)

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

@@ -18,7 +18,7 @@ import readline from 'readline';
 import { createGuid, ManualPromise } from 'playwright-core/lib/utils';
 import type { FullConfigInternal, FullProjectInternal } from '../common/types';
 import { Multiplexer } from '../reporters/multiplexer';
-import { createFileMatcherFromArguments } from '../util';
+import { createFileMatcher, createFileMatcherFromArguments } from '../util';
 import type { Matcher } from '../util';
 import { createTaskRunnerForWatch, createTaskRunnerForWatchSetup } from './tasks';
 import type { TaskRunnerState } from './tasks';
@@ -26,6 +26,7 @@ import { buildProjectsClosure, filterProjects } from './projectUtils';
 import { clearCompilationCache, collectAffectedTestFiles } from '../common/compilationCache';
 import type { FullResult } from 'packages/playwright-test/reporter';
 import { chokidar } from '../utilsBundle';
+import type { FSWatcher as CFSWatcher } from 'chokidar';
 import { createReporter } from './reporters';
 import { colors } from 'playwright-core/lib/utilsBundle';
 import { enquirer } from '../utilsBundle';
@@ -34,32 +35,74 @@ import { PlaywrightServer } from 'playwright-core/lib/remote/playwrightServer';
 import ListReporter from '../reporters/list';
 
 class FSWatcher {
-  private _dirtyFiles = new Set<string>();
+  private _dirtyTestFiles = new Map<FullProjectInternal, Set<string>>();
   private _notifyDirtyFiles: (() => void) | undefined;
+  private _watcher: CFSWatcher | undefined;
+  private _timer: NodeJS.Timeout | undefined;
 
-  constructor(dirs: string[]) {
+  async update(config: FullConfigInternal) {
-    let timer: NodeJS.Timer;
+    const commandLineFileMatcher = config._internal.cliArgs.length ? createFileMatcherFromArguments(config._internal.cliArgs) : () => true;
-    chokidar.watch(dirs, { ignoreInitial: true }).on('all', async (event, file) => {
+    const projects = filterProjects(config.projects, config._internal.cliProjectFilter);
+    const projectClosure = buildProjectsClosure(projects);
+    const projectFilters = new Map<FullProjectInternal, Matcher>();
+    for (const project of projectClosure) {
+      const testMatch = createFileMatcher(project.testMatch);
+      const testIgnore = createFileMatcher(project.testIgnore);
+      projectFilters.set(project, file => {
+        if (!file.startsWith(project.testDir) || !testMatch(file) || testIgnore(file))
+          return false;
+        return project._internal.type === 'dependency' || commandLineFileMatcher(file);
+      });
+    }
+
+    if (this._timer)
+      clearTimeout(this._timer);
+    if (this._watcher)
+      await this._watcher.close();
+
+    this._watcher = chokidar.watch(projectClosure.map(p => p.testDir), { ignoreInitial: true }).on('all', async (event, file) => {
       if (event !== 'add' && event !== 'change')
         return;
-      this._dirtyFiles.add(file);
+
-      if (timer)
+      const testFiles = new Set<string>();
-        clearTimeout(timer);
+      collectAffectedTestFiles(file, testFiles);
-      timer = setTimeout(() => {
+      const testFileArray = [...testFiles];
+
+      let hasMatches = false;
+      for (const [project, filter] of projectFilters) {
+        const filteredFiles = testFileArray.filter(filter);
+        if (!filteredFiles.length)
+          continue;
+        let set = this._dirtyTestFiles.get(project);
+        if (!set) {
+          set = new Set();
+          this._dirtyTestFiles.set(project, set);
+        }
+        filteredFiles.map(f => set!.add(f));
+        hasMatches = true;
+      }
+
+      if (!hasMatches)
+        return;
+
+      if (this._timer)
+        clearTimeout(this._timer);
+      this._timer = setTimeout(() => {
         this._notifyDirtyFiles?.();
       }, 250);
     });
+
   }
 
-  async onDirtyFiles(): Promise<void> {
+  async onDirtyTestFiles(): Promise<void> {
-    if (this._dirtyFiles.size)
+    if (this._dirtyTestFiles.size)
       return;
     await new Promise<void>(f => this._notifyDirtyFiles = f);
   }
 
-  takeDirtyFiles(): Set<string> {
+  takeDirtyTestFiles(): Map<FullProjectInternal, Set<string>> {
-    const result = this._dirtyFiles;
+    const result = this._dirtyTestFiles;
-    this._dirtyFiles = new Set();
+    this._dirtyTestFiles = new Map();
     return result;
   }
 }
@@ -84,13 +127,12 @@ export async function runWatchModeLoop(config: FullConfigInternal): Promise<Full
     return await globalCleanup();
 
   // Prepare projects that will be watched, set up watcher.
-  const projects = filterProjects(config.projects, config._internal.cliProjectFilter);
-  const projectClosure = buildProjectsClosure(projects);
   const failedTestIdCollector = new Set<string>();
   const originalWorkers = config.workers;
-  const fsWatcher = new FSWatcher(projectClosure.map(p => p.testDir));
+  const fsWatcher = new FSWatcher();
+  await fsWatcher.update(config);
 
-  let lastRun: { type: 'changed' | 'regular' | 'failed', failedTestIds?: Set<string>, dirtyFiles?: Set<string> } = { type: 'regular' };
+  let lastRun: { type: 'changed' | 'regular' | 'failed', failedTestIds?: Set<string>, dirtyTestFiles?: Map<FullProjectInternal, Set<string>> } = { type: 'regular' };
   let result: FullResult['status'] = 'passed';
 
   // Enter the watch loop.
@@ -100,7 +142,7 @@ export async function runWatchModeLoop(config: FullConfigInternal): Promise<Full
     printPrompt();
     const readCommandPromise = readCommand();
     await Promise.race([
-      fsWatcher.onDirtyFiles(),
+      fsWatcher.onDirtyTestFiles(),
       readCommandPromise,
     ]);
     if (!readCommandPromise.isDone())
@@ -109,9 +151,10 @@ export async function runWatchModeLoop(config: FullConfigInternal): Promise<Full
     const command = await readCommandPromise;
 
     if (command === 'changed') {
-      const dirtyFiles = fsWatcher.takeDirtyFiles();
+      const dirtyTestFiles = fsWatcher.takeDirtyTestFiles();
-      await runChangedTests(config, failedTestIdCollector, projectClosure, dirtyFiles);
+      // Resolve files that depend on the changed files.
-      lastRun = { type: 'changed', dirtyFiles };
+      await runChangedTests(config, failedTestIdCollector, dirtyTestFiles);
+      lastRun = { type: 'changed', dirtyTestFiles };
       continue;
     }
 
@@ -132,6 +175,7 @@ export async function runWatchModeLoop(config: FullConfigInternal): Promise<Full
       if (!projectNames)
         continue;
       config._internal.cliProjectFilter = projectNames.length ? projectNames : undefined;
+      await fsWatcher.update(config);
       await runTests(config, failedTestIdCollector);
       lastRun = { type: 'regular' };
       continue;
@@ -149,6 +193,7 @@ export async function runWatchModeLoop(config: FullConfigInternal): Promise<Full
         config._internal.cliArgs = filePattern.split(' ');
       else
         config._internal.cliArgs = [];
+      await fsWatcher.update(config);
       await runTests(config, failedTestIdCollector);
       lastRun = { type: 'regular' };
       continue;
@@ -166,6 +211,7 @@ export async function runWatchModeLoop(config: FullConfigInternal): Promise<Full
         config._internal.cliGrep = testPattern;
       else
         config._internal.cliGrep = undefined;
+      await fsWatcher.update(config);
       await runTests(config, failedTestIdCollector);
       lastRun = { type: 'regular' };
       continue;
@@ -185,7 +231,7 @@ export async function runWatchModeLoop(config: FullConfigInternal): Promise<Full
         await runTests(config, failedTestIdCollector, { title: 're-running tests' });
         continue;
       } else if (lastRun.type === 'changed') {
-        await runChangedTests(config, failedTestIdCollector, projectClosure, lastRun.dirtyFiles!, 're-running tests');
+        await runChangedTests(config, failedTestIdCollector, lastRun.dirtyTestFiles!, 're-running tests');
       } else if (lastRun.type === 'failed') {
         config._internal.testIdMatcher = id => lastRun.failedTestIds!.has(id);
         await runTests(config, failedTestIdCollector, { title: 're-running tests' });
@@ -211,30 +257,15 @@ export async function runWatchModeLoop(config: FullConfigInternal): Promise<Full
   return result === 'passed' ? await globalCleanup() : result;
 }
 
-async function runChangedTests(config: FullConfigInternal, failedTestIdCollector: Set<string>, projectClosure: FullProjectInternal[], changedFiles: Set<string>, title?: string) {
+async function runChangedTests(config: FullConfigInternal, failedTestIdCollector: Set<string>, filesByProject: Map<FullProjectInternal, Set<string>>, title?: string) {
-  const commandLineFileMatcher = config._internal.cliArgs.length ? createFileMatcherFromArguments(config._internal.cliArgs) : () => true;
-
-  // Resolve files that depend on the changed files.
   const testFiles = new Set<string>();
-  for (const file of changedFiles)
+  for (const files of filesByProject.values())
-    collectAffectedTestFiles(file, testFiles);
+    files.forEach(f => testFiles.add(f));
-
-  // Collect projects with changes.
-  const filesByProject = new Map<FullProjectInternal, string[]>();
-  for (const project of projectClosure) {
-    const projectFiles: string[] = [];
-    for (const file of testFiles) {
-      if (!file.startsWith(project.testDir))
-        continue;
-      if (project._internal.type === 'dependency' || commandLineFileMatcher(file))
-        projectFiles.push(file);
-    }
-    if (projectFiles.length)
-      filesByProject.set(project, projectFiles);
-  }
 
   // Collect all the affected projects, follow project dependencies.
   // Prepare to exclude all the projects that do not depend on this file, as if they did not exist.
+  const projects = filterProjects(config.projects, config._internal.cliProjectFilter);
+  const projectClosure = buildProjectsClosure(projects);
   const affectedProjects = affectedProjectsClosure(projectClosure, [...filesByProject.keys()]);
   const affectsAnyDependency = [...affectedProjects].some(p => p._internal.type === 'dependency');
   const projectsToIgnore = new Set(projectClosure.filter(p => !affectedProjects.has(p)));
@@ -242,7 +273,7 @@ async function runChangedTests(config: FullConfigInternal, failedTestIdCollector
   // If there are affected dependency projects, do the full run, respect the original CLI.
   // if there are no affected dependency projects, intersect CLI with dirty files
   const additionalFileMatcher = affectsAnyDependency ? () => true : (file: string) => testFiles.has(file);
-  return await runTests(config, failedTestIdCollector, { projectsToIgnore, additionalFileMatcher, title: title || 'files changed' });
+  await runTests(config, failedTestIdCollector, { projectsToIgnore, additionalFileMatcher, title: title || 'files changed' });
 }
 
 async function runTests(config: FullConfigInternal, failedTestIdCollector: Set<string>, options?: {

tests/playwright-test/playwright-test-fixtures.ts

@@ -57,7 +57,7 @@ type TSCResult = {
 type Files = { [key: string]: string | Buffer };
 type Params = { [key: string]: string | number | boolean | string[] };
 
-async function writeFiles(testInfo: TestInfo, files: Files) {
+async function writeFiles(testInfo: TestInfo, files: Files, initial: boolean) {
   const baseDir = testInfo.outputPath();
 
   const headerJS = `
@@ -71,7 +71,7 @@ async function writeFiles(testInfo: TestInfo, files: Files) {
   `;
 
   const hasConfig = Object.keys(files).some(name => name.includes('.config.'));
-  if (!hasConfig) {
+  if (initial && !hasConfig) {
     files = {
       ...files,
       'playwright.config.ts': `
@@ -79,7 +79,7 @@ async function writeFiles(testInfo: TestInfo, files: Files) {
       `,
     };
   }
-  if (!Object.keys(files).some(name => name.includes('package.json'))) {
+  if (initial && !Object.keys(files).some(name => name.includes('package.json'))) {
     files = {
       ...files,
       'package.json': `{ "name": "test-project" }`,
@@ -280,13 +280,13 @@ export const test = base
     .extend<ServerFixtures, ServerWorkerOptions>(serverFixtures)
     .extend<Fixtures>({
       writeFiles: async ({}, use, testInfo) => {
-        await use(files => writeFiles(testInfo, files));
+        await use(files => writeFiles(testInfo, files, false));
       },
 
       runInlineTest: async ({ childProcess }, use, testInfo: TestInfo) => {
         const cacheDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'playwright-test-cache-'));
         await use(async (files: Files, params: Params = {}, env: NodeJS.ProcessEnv = {}, options: RunOptions = {}, beforeRunPlaywrightTest?: ({ baseDir }: { baseDir: string }) => Promise<void>) => {
-          const baseDir = await writeFiles(testInfo, files);
+          const baseDir = await writeFiles(testInfo, files, true);
           if (beforeRunPlaywrightTest)
             await beforeRunPlaywrightTest({ baseDir });
           return await runPlaywrightTest(childProcess, baseDir, params, { ...env, PWTEST_CACHE_DIR: cacheDir }, options);
@@ -298,7 +298,7 @@ export const test = base
         const cacheDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'playwright-test-cache-'));
         let testProcess: TestChildProcess | undefined;
         await use(async (files: Files, env: NodeJS.ProcessEnv = {}, options: RunOptions = {}) => {
-          const baseDir = await writeFiles(testInfo, files);
+          const baseDir = await writeFiles(testInfo, files, true);
           testProcess = watchPlaywrightTest(childProcess, baseDir, { ...env, PWTEST_CACHE_DIR: cacheDir }, options);
           return testProcess;
         });
@@ -308,14 +308,14 @@ export const test = base
 
       runCommand: async ({ childProcess }, use, testInfo: TestInfo) => {
         await use(async (files: Files, args: string[]) => {
-          const baseDir = await writeFiles(testInfo, files);
+          const baseDir = await writeFiles(testInfo, files, true);
           return await runPlaywrightCommand(childProcess, baseDir, args, { });
         });
       },
 
       runTSC: async ({ childProcess }, use, testInfo) => {
         await use(async files => {
-          const baseDir = await writeFiles(testInfo, { 'tsconfig.json': JSON.stringify(TSCONFIG), ...files });
+          const baseDir = await writeFiles(testInfo, { 'tsconfig.json': JSON.stringify(TSCONFIG), ...files }, true);
           const tsc = childProcess({
             command: ['npx', 'tsc', '-p', baseDir],
             cwd: baseDir,

tests/playwright-test/reporter-html.spec.ts

@@ -18,7 +18,7 @@ import fs from 'fs';
 import path from 'path';
 import url from 'url';
 import { test as baseTest, expect, createImage } from './playwright-test-fixtures';
-import type { HttpServer } from '../../packages/playwright-core/lib/utils';
+import type { HttpServer } from '../../packages/playwright-core/src/utils';
 import { startHtmlReportServer } from '../../packages/playwright-test/lib/reporters/html';
 import { spawnAsync } from 'playwright-core/lib/utils';
 
@@ -27,7 +27,7 @@ const test = baseTest.extend<{ showReport: (reportFolder?: string) => Promise<vo
     let server: HttpServer | undefined;
     await use(async (reportFolder?: string) => {
       reportFolder ??=  testInfo.outputPath('playwright-report');
-      server = startHtmlReportServer(reportFolder);
+      server = startHtmlReportServer(reportFolder) as HttpServer;
       const location = await server.start();
       await page.goto(location);
     });

tests/playwright-test/watch.spec.ts

@@ -395,13 +395,95 @@ test('should not trigger on changes to non-tests', async ({ runWatchTest, writeF
   await testProcess.waitForOutput('a.test.ts:5:11 › passes');
   await testProcess.waitForOutput('b.test.ts:5:11 › passes');
   await testProcess.waitForOutput('Waiting for file changes.');
+
   testProcess.clearOutput();
   writeFiles({
     'helper.ts': `
       console.log('helper');
     `,
   });
+
   await new Promise(f => setTimeout(f, 1000));
-  expect(testProcess.output).not.toContain('a.test.ts');
+  expect(testProcess.output).not.toContain('Waiting for file changes.');
-  expect(testProcess.output).not.toContain('b.test.ts');
+});
+
+test('should only watch selected projects', async ({ runWatchTest, writeFiles }) => {
+  const testProcess = await runWatchTest({
+    'playwright.config.ts': `
+      import { defineConfig } from '@playwright/test';
+      export default defineConfig({ projects: [{name: 'foo'}, {name: 'bar'}] });
+    `,
+    'a.test.ts': `
+      pwt.test('passes', () => {});
+    `,
+  }, {}, { additionalArgs: ['--project=foo'] });
+  await testProcess.waitForOutput('npx playwright test --project foo');
+  await testProcess.waitForOutput('[foo] › a.test.ts:5:11 › passes');
+  expect(testProcess.output).not.toContain('[bar]');
+  await testProcess.waitForOutput('Waiting for file changes.');
+
+  testProcess.clearOutput();
+  writeFiles({
+    'a.test.ts': `
+      pwt.test('passes', () => {});
+    `,
+  });
+
+  await testProcess.waitForOutput('npx playwright test --project foo');
+  await testProcess.waitForOutput('[foo] › a.test.ts:5:11 › passes');
+  await testProcess.waitForOutput('Waiting for file changes.');
+  expect(testProcess.output).not.toContain('[bar]');
+});
+
+test('should watch filtered files', async ({ runWatchTest, writeFiles }) => {
+  const testProcess = await runWatchTest({
+    'a.test.ts': `
+      pwt.test('passes', () => {});
+    `,
+    'b.test.ts': `
+      pwt.test('passes', () => {});
+    `,
+  }, {}, { additionalArgs: ['a.test.ts'] });
+  await testProcess.waitForOutput('npx playwright test a.test.ts');
+  await testProcess.waitForOutput('a.test.ts:5:11 › passes');
+  expect(testProcess.output).not.toContain('b.test');
+  await testProcess.waitForOutput('Waiting for file changes.');
+
+  testProcess.clearOutput();
+  writeFiles({
+    'b.test.ts': `
+      pwt.test('passes', () => {});
+    `,
+  });
+
+  await new Promise(f => setTimeout(f, 1000));
+  expect(testProcess.output).not.toContain('Waiting for file changes.');
+});
+
+test('should not watch unfiltered files', async ({ runWatchTest, writeFiles }) => {
+  const testProcess = await runWatchTest({
+    'a.test.ts': `
+      pwt.test('passes', () => {});
+    `,
+    'b.test.ts': `
+      pwt.test('passes', () => {});
+    `,
+  }, {}, { additionalArgs: ['a.test.ts'] });
+  await testProcess.waitForOutput('npx playwright test a.test.ts');
+  await testProcess.waitForOutput('a.test.ts:5:11 › passes');
+  expect(testProcess.output).not.toContain('b.test');
+  await testProcess.waitForOutput('Waiting for file changes.');
+
+  testProcess.clearOutput();
+  writeFiles({
+    'a.test.ts': `
+      pwt.test('passes', () => {});
+    `,
+  });
+
+  testProcess.clearOutput();
+  await testProcess.waitForOutput('npx playwright test a.test.ts (files changed)');
+  await testProcess.waitForOutput('a.test.ts:5:11 › passes');
+  expect(testProcess.output).not.toContain('b.test');
+  await testProcess.waitForOutput('Waiting for file changes.');
 });