fix(ui): added test in watched file should be run (#31842)

Closes https://github.com/microsoft/playwright/issues/22211

Currently, when the server notifies the UI about changed files, the UI
determines what files to re-run based on an old test list. By listing
tests before that, we make sure that the test list is up-to-date, and
that added tests are included in the next run.

I've also removed the `listChanged` event as discussed in the team sync.
The event isn't used anywhere and fires in exactly the same cases where
`testFilesChanged` fired, so i've folded them into one another. This allowed simplifying `Watcher`.

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

@@ -61,7 +61,7 @@ export async function runDevServer(config: FullConfigInternal): Promise<() => Pr
     projectOutputs.add(p.project.outputDir);
   }
 
-  const globalWatcher = new Watcher('deep', async () => {
+  const globalWatcher = new Watcher(async () => {
     const registry: ComponentRegistry = new Map();
     await populateComponentsFromTests(registry);
     // compare componentRegistry to registry key sets.
@@ -80,6 +80,6 @@ export async function runDevServer(config: FullConfigInternal): Promise<() => Pr
     if (rootModule)
       devServer.moduleGraph.onFileChange(rootModule.file!);
   });
-  globalWatcher.update([...projectDirs], [...projectOutputs], false);
+  await globalWatcher.update([...projectDirs], [...projectOutputs], false);
   return () => Promise.all([devServer.close(), globalWatcher.close()]).then(() => {});
 }

packages/playwright/src/fsWatcher.ts

@@ -21,26 +21,24 @@ export type FSEvent = { event: 'add' | 'addDir' | 'change' | 'unlink' | 'unlinkD
 
 export class Watcher {
   private _onChange: (events: FSEvent[]) => void;
-  private _watchedFiles: string[] = [];
+  private _watchedPaths: string[] = [];
   private _ignoredFolders: string[] = [];
   private _collector: FSEvent[] = [];
   private _fsWatcher: FSWatcher | undefined;
   private _throttleTimer: NodeJS.Timeout | undefined;
-  private _mode: 'flat' | 'deep';
 
-  constructor(mode: 'flat' | 'deep', onChange: (events: FSEvent[]) => void) {
-    this._mode = mode;
+  constructor(onChange: (events: FSEvent[]) => void) {
     this._onChange = onChange;
   }
 
-  update(watchedFiles: string[], ignoredFolders: string[], reportPending: boolean) {
-    if (JSON.stringify([this._watchedFiles, this._ignoredFolders]) === JSON.stringify(watchedFiles, ignoredFolders))
+  async update(watchedPaths: string[], ignoredFolders: string[], reportPending: boolean) {
+    if (JSON.stringify([this._watchedPaths, this._ignoredFolders]) === JSON.stringify(watchedPaths, ignoredFolders))
       return;
 
     if (reportPending)
       this._reportEventsIfAny();
 
-    this._watchedFiles = watchedFiles;
+    this._watchedPaths = watchedPaths;
     this._ignoredFolders = ignoredFolders;
     void this._fsWatcher?.close();
     this._fsWatcher = undefined;
@@ -48,20 +46,18 @@ export class Watcher {
     clearTimeout(this._throttleTimer);
     this._throttleTimer = undefined;
 
-    if (!this._watchedFiles.length)
+    if (!this._watchedPaths.length)
       return;
 
     const ignored = [...this._ignoredFolders, '**/node_modules/**'];
-    this._fsWatcher = chokidar.watch(watchedFiles, { ignoreInitial: true, ignored }).on('all', async (event, file) => {
+    this._fsWatcher = chokidar.watch(watchedPaths, { ignoreInitial: true, ignored }).on('all', async (event, file) => {
       if (this._throttleTimer)
         clearTimeout(this._throttleTimer);
-      if (this._mode === 'flat' && event !== 'add' && event !== 'change')
-        return;
-      if (this._mode === 'deep' && event !== 'add' && event !== 'change' && event !== 'unlink' && event !== 'addDir' && event !== 'unlinkDir')
-        return;
       this._collector.push({ event, file });
       this._throttleTimer = setTimeout(() => this._reportEventsIfAny(), 250);
     });
+
+    await new Promise((resolve, reject) => this._fsWatcher!.once('ready', resolve).once('error', reject));
   }
 
   async close() {

packages/playwright/src/isomorphic/testServerConnection.ts

@@ -23,14 +23,12 @@ export class TestServerConnection implements TestServerInterface, TestServerInte
   readonly onClose: events.Event<void>;
   readonly onReport: events.Event<any>;
   readonly onStdio: events.Event<{ type: 'stderr' | 'stdout'; text?: string | undefined; buffer?: string | undefined; }>;
-  readonly onListChanged: events.Event<void>;
   readonly onTestFilesChanged: events.Event<{ testFiles: string[] }>;
   readonly onLoadTraceRequested: events.Event<{ traceUrl: string }>;
 
   private _onCloseEmitter = new events.EventEmitter<void>();
   private _onReportEmitter = new events.EventEmitter<any>();
   private _onStdioEmitter = new events.EventEmitter<{ type: 'stderr' | 'stdout'; text?: string | undefined; buffer?: string | undefined; }>();
-  private _onListChangedEmitter = new events.EventEmitter<void>();
   private _onTestFilesChangedEmitter = new events.EventEmitter<{ testFiles: string[] }>();
   private _onLoadTraceRequestedEmitter = new events.EventEmitter<{ traceUrl: string }>();
 
@@ -44,7 +42,6 @@ export class TestServerConnection implements TestServerInterface, TestServerInte
     this.onClose = this._onCloseEmitter.event;
     this.onReport = this._onReportEmitter.event;
     this.onStdio = this._onStdioEmitter.event;
-    this.onListChanged = this._onListChangedEmitter.event;
     this.onTestFilesChanged = this._onTestFilesChangedEmitter.event;
     this.onLoadTraceRequested = this._onLoadTraceRequestedEmitter.event;
 
@@ -103,8 +100,6 @@ export class TestServerConnection implements TestServerInterface, TestServerInte
       this._onReportEmitter.fire(params);
     else if (method === 'stdio')
       this._onStdioEmitter.fire(params);
-    else if (method === 'listChanged')
-      this._onListChangedEmitter.fire(params);
     else if (method === 'testFilesChanged')
       this._onTestFilesChangedEmitter.fire(params);
     else if (method === 'loadTraceRequested')

packages/playwright/src/isomorphic/testServerInterface.ts

@@ -118,7 +118,6 @@ export interface TestServerInterface {
 export interface TestServerInterfaceEvents {
   onReport: Event<any>;
   onStdio: Event<{ type: 'stdout' | 'stderr', text?: string, buffer?: string }>;
-  onListChanged: Event<void>;
   onTestFilesChanged: Event<{ testFiles: string[] }>;
   onLoadTraceRequested: Event<{ traceUrl: string }>;
 }
@@ -126,7 +125,6 @@ export interface TestServerInterfaceEvents {
 export interface TestServerInterfaceEventEmitters {
   dispatchEvent(event: 'report', params: ReportEntry): void;
   dispatchEvent(event: 'stdio', params: { type: 'stdout' | 'stderr', text?: string, buffer?: string }): void;
-  dispatchEvent(event: 'listChanged', params: {}): void;
   dispatchEvent(event: 'testFilesChanged', params: { testFiles: string[] }): void;
   dispatchEvent(event: 'loadTraceRequested', params: { traceUrl: string }): void;
 }

packages/playwright/src/runner/testServer.ts

@@ -64,8 +64,12 @@ class TestServer {
 
 class TestServerDispatcher implements TestServerInterface {
   private _configLocation: ConfigLocation;
-  private _globalWatcher: Watcher;
-  private _testWatcher: Watcher;
+
+  private _watcher: Watcher;
+  private _watchedProjectDirs = new Set<string>();
+  private _ignoredProjectOutputs = new Set<string>();
+  private _watchedTestDependencies = new Set<string>();
+
   private _testRun: { run: Promise<reporterTypes.FullResult['status']>, stop: ManualPromise<void> } | undefined;
   readonly transport: Transport;
   private _queue = Promise.resolve();
@@ -86,8 +90,7 @@ class TestServerDispatcher implements TestServerInterface {
           gracefullyProcessExitDoNotHang(0);
       },
     };
-    this._globalWatcher = new Watcher('deep', () => this._dispatchEvent('listChanged', {}));
-    this._testWatcher = new Watcher('flat', events => {
+    this._watcher = new Watcher(events => {
       const collector = new Set<string>();
       events.forEach(f => collectAffectedTestFiles(f.file, collector));
       this._dispatchEvent('testFilesChanged', { testFiles: [...collector] });
@@ -279,24 +282,28 @@ class TestServerDispatcher implements TestServerInterface {
     await taskRunner.reporter.onEnd({ status });
     await taskRunner.reporter.onExit();
 
-    const projectDirs = new Set<string>();
-    const projectOutputs = new Set<string>();
+    this._watchedProjectDirs = new Set();
+    this._ignoredProjectOutputs = new Set();
     for (const p of config.projects) {
-      projectDirs.add(p.project.testDir);
-      projectOutputs.add(p.project.outputDir);
+      this._watchedProjectDirs.add(p.project.testDir);
+      this._ignoredProjectOutputs.add(p.project.outputDir);
     }
 
     const result = await resolveCtDirs(config);
     if (result) {
-      projectDirs.add(result.templateDir);
-      projectOutputs.add(result.outDir);
+      this._watchedProjectDirs.add(result.templateDir);
+      this._ignoredProjectOutputs.add(result.outDir);
     }
 
     if (this._watchTestDirs)
-      this._globalWatcher.update([...projectDirs], [...projectOutputs], false);
+      await this.updateWatcher(false);
     return { report, status };
   }
 
+  private async updateWatcher(reportPending: boolean) {
+    await this._watcher.update([...this._watchedProjectDirs, ...this._watchedTestDependencies], [...this._ignoredProjectOutputs], reportPending);
+  }
+
   async runTests(params: Parameters<TestServerInterface['runTests']>[0]): ReturnType<TestServerInterface['runTests']> {
     let result: Awaited<ReturnType<TestServerInterface['runTests']>> = { status: 'passed' };
     this._queue = this._queue.then(async () => {
@@ -364,12 +371,12 @@ class TestServerDispatcher implements TestServerInterface {
   }
 
   async watch(params: { fileNames: string[]; }) {
-    const files = new Set<string>();
+    this._watchedTestDependencies = new Set();
     for (const fileName of params.fileNames) {
-      files.add(fileName);
-      dependenciesForTestFile(fileName).forEach(file => files.add(file));
+      this._watchedTestDependencies.add(fileName);
+      dependenciesForTestFile(fileName).forEach(file => this._watchedTestDependencies.add(file));
     }
-    this._testWatcher.update([...files], [], true);
+    await this.updateWatcher(true);
   }
 
   async findRelatedTestFiles(params: Parameters<TestServerInterface['findRelatedTestFiles']>[0]): ReturnType<TestServerInterface['findRelatedTestFiles']> {

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

@@ -94,6 +94,7 @@ export const UIModeView: React.FC<{}> = ({
   const [isDisconnected, setIsDisconnected] = React.useState(false);
   const [hasBrowsers, setHasBrowsers] = React.useState(true);
   const [testServerConnection, setTestServerConnection] = React.useState<TestServerConnection>();
+  const [teleSuiteUpdater, setTeleSuiteUpdater] = React.useState<TeleSuiteUpdater>();
   const [settingsVisible, setSettingsVisible] = React.useState(false);
   const [testingOptionsVisible, setTestingOptionsVisible] = React.useState(false);
   const [revealSource, setRevealSource] = React.useState(false);
@@ -191,20 +192,7 @@ export const UIModeView: React.FC<{}> = ({
       pathSeparator,
     });
 
-    const updateList = async () => {
-      commandQueue.current = commandQueue.current.then(async () => {
-        setIsLoading(true);
-        try {
-          const result = await testServerConnection.listTests({ projects: queryParams.projects, locations: queryParams.args, grep: queryParams.grep, grepInvert: queryParams.grepInvert });
-          teleSuiteUpdater.processListReport(result.report);
-        } catch (e) {
-          // eslint-disable-next-line no-console
-          console.log(e);
-        } finally {
-          setIsLoading(false);
-        }
-      });
-    };
+    setTeleSuiteUpdater(teleSuiteUpdater);
 
     setTestModel(undefined);
     setIsLoading(true);
@@ -223,7 +211,6 @@ export const UIModeView: React.FC<{}> = ({
         const result = await testServerConnection.listTests({ projects: queryParams.projects, locations: queryParams.args, grep: queryParams.grep, grepInvert: queryParams.grepInvert });
         teleSuiteUpdater.processListReport(result.report);
 
-        testServerConnection.onListChanged(updateList);
         testServerConnection.onReport(params => {
           teleSuiteUpdater.processTestReportEvent(params);
         });
@@ -336,11 +323,32 @@ export const UIModeView: React.FC<{}> = ({
     });
   }, [projectFilters, runningState, testModel, testServerConnection, runWorkers, runHeaded, runUpdateSnapshots]);
 
-  // Watch implementation.
   React.useEffect(() => {
-    if (!testServerConnection)
+    if (!testServerConnection || !teleSuiteUpdater)
       return;
-    const disposable = testServerConnection.onTestFilesChanged(params => {
+    const disposable = testServerConnection.onTestFilesChanged(async params => {
+      // fetch the new list of tests
+      commandQueue.current = commandQueue.current.then(async () => {
+        setIsLoading(true);
+        try {
+          const result = await testServerConnection.listTests({ projects: queryParams.projects, locations: queryParams.args, grep: queryParams.grep, grepInvert: queryParams.grepInvert });
+          teleSuiteUpdater.processListReport(result.report);
+        } catch (e) {
+          // eslint-disable-next-line no-console
+          console.log(e);
+        } finally {
+          setIsLoading(false);
+        }
+      });
+      await commandQueue.current;
+
+      if (params.testFiles.length === 0)
+        return;
+
+      // run affected watched tests
+      const testModel = teleSuiteUpdater.asModel();
+      const testTree = new TestTree('', testModel.rootSuite, testModel.loadErrors, projectFilters, pathSeparator);
+
       const testIds: string[] = [];
       const set = new Set(params.testFiles);
       if (watchAll) {
@@ -363,7 +371,7 @@ export const UIModeView: React.FC<{}> = ({
       runTests('queue-if-busy', new Set(testIds));
     });
     return () => disposable.dispose();
-  }, [runTests, testServerConnection, testTree, watchAll, watchedTreeIds]);
+  }, [runTests, testServerConnection, watchAll, watchedTreeIds, teleSuiteUpdater, projectFilters]);
 
   // Shortcuts.
   React.useEffect(() => {

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

@@ -125,6 +125,10 @@ function startPlaywrightTest(childProcess: CommonFixtures['childProcess'], baseD
   );
   if (options.additionalArgs)
     args.push(...options.additionalArgs);
+  return startPlaywrightChildProcess(childProcess, baseDir, args, env, options);
+}
+
+function startPlaywrightChildProcess(childProcess: CommonFixtures['childProcess'], baseDir: string, args: string[], env: NodeJS.ProcessEnv, options: RunOptions): TestChildProcess {
   return childProcess({
     command: ['node', cliEntrypoint, ...args],
     env: cleanEnv(env),
@@ -246,6 +250,7 @@ type Fixtures = {
   deleteFile: (file: string) => Promise<void>;
   runInlineTest: (files: Files, params?: Params, env?: NodeJS.ProcessEnv, options?: RunOptions) => Promise<RunResult>;
   runCLICommand: (files: Files, command: string, args?: string[], entryPoint?: string) => Promise<{ stdout: string, stderr: string, exitCode: number }>;
+  startCLICommand: (files: Files, command: string, args?: string[], options?: RunOptions) => Promise<TestChildProcess>;
   runWatchTest: (files: Files, params?: Params, env?: NodeJS.ProcessEnv, options?: RunOptions) => Promise<TestChildProcess>;
   interactWithTestRunner: (files: Files, params?: Params, env?: NodeJS.ProcessEnv, options?: RunOptions) => Promise<TestChildProcess>;
   runTSC: (files: Files) => Promise<TSCResult>;
@@ -287,6 +292,15 @@ export const test = base
         await removeFolders([cacheDir]);
       },
 
+      startCLICommand: async ({ childProcess }, use, testInfo: TestInfo) => {
+        const cacheDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'playwright-test-cache-'));
+        await use(async (files: Files, command: string, args?: string[], options: RunOptions = {}) => {
+          const baseDir = await writeFiles(testInfo, files, true);
+          return startPlaywrightChildProcess(childProcess, baseDir, [command, ...(args || [])], { PWTEST_CACHE_DIR: cacheDir }, options);
+        });
+        await removeFolders([cacheDir]);
+      },
+
       runWatchTest: async ({ interactWithTestRunner }, use, testInfo: TestInfo) => {
         await use((files, params, env, options) => interactWithTestRunner(files, params, { ...env, PWTEST_WATCH: '1' }, options));
       },

tests/playwright-test/test-server-connection.spec.ts

@@ -0,0 +1,97 @@
+/**
+ * Copyright (c) Microsoft Corporation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import { test as baseTest, expect } from './ui-mode-fixtures';
+
+import { TestServerConnection } from '../../packages/playwright/lib/isomorphic/testServerConnection';
+
+class TestServerConnectionUnderTest extends TestServerConnection {
+  events: [string, any][] = [];
+
+  constructor(wsUrl: string) {
+    super(wsUrl);
+    this.onTestFilesChanged(params => this.events.push(['testFilesChanged', params]));
+    this.onStdio(params => this.events.push(['stdio', params]));
+    this.onLoadTraceRequested(params => this.events.push(['loadTraceRequested', params]));
+  }
+}
+
+const test = baseTest.extend<{ testServerConnection: TestServerConnectionUnderTest }>({
+  testServerConnection: async ({ startCLICommand }, use, testInfo) => {
+    testInfo.skip(!globalThis.WebSocket, 'WebSocket not available prior to Node 22.4.0');
+
+    const testServerProcess = await startCLICommand({}, 'test-server');
+
+    await testServerProcess.waitForOutput('Listening on');
+    const line = testServerProcess.output.split('\n').find(l => l.includes('Listening on'));
+    const wsEndpoint = line!.split(' ')[2];
+
+    await use(new TestServerConnectionUnderTest(wsEndpoint));
+
+    await testServerProcess.kill();
+  }
+});
+
+test('file watching', async ({ testServerConnection, writeFiles }, testInfo) => {
+  await writeFiles({
+    'utils.ts': `
+      export const expected = 42;
+      `,
+    'a.test.ts': `
+      import { test } from '@playwright/test';
+      import { expected } from "./utils";
+      test('foo', () => {
+        expect(123).toBe(expected);
+      });
+      `,
+  });
+
+  const tests = await testServerConnection.listTests({});
+  expect(tests.report.map(e => e.method)).toEqual(['onConfigure', 'onProject', 'onBegin', 'onEnd']);
+
+  await testServerConnection.watch({ fileNames: [testInfo.outputPath('a.test.ts')] });
+
+  await writeFiles({
+    'utils.ts': `
+      export const expected = 123;
+      `,
+  });
+
+  await expect.poll(() => testServerConnection.events).toHaveLength(1);
+  expect(testServerConnection.events).toEqual([
+    ['testFilesChanged', { testFiles: [testInfo.outputPath('a.test.ts')] }]
+  ]);
+});
+
+test('stdio interception', async ({ testServerConnection, writeFiles }) => {
+  await testServerConnection.initialize({ interceptStdio: true });
+  await writeFiles({
+    'a.test.ts': `
+      import { test, expect } from '@playwright/test';
+      test('foo', () => {
+        console.log("this goes to stdout");
+        console.error("this goes to stderr");
+        expect(true).toBe(true);
+      });
+      `,
+  });
+
+  const tests = await testServerConnection.runTests({ trace: 'on' });
+  expect(tests).toEqual({ status: 'passed' });
+  await expect.poll(() => testServerConnection.events).toEqual(expect.arrayContaining([
+    ['stdio', { type: 'stderr', text: 'this goes to stderr\n' }],
+    ['stdio', { type: 'stdout', text: 'this goes to stdout\n' }]
+  ]));
+});

tests/playwright-test/ui-mode-test-watch.spec.ts

@@ -220,6 +220,37 @@ test('should watch new file', async ({ runUITest, writeFiles }) => {
   `);
 });
 
+test('should run added test in watched file', async ({ runUITest, writeFiles }) => {
+  const { page } = await runUITest({
+    'a.test.ts': `
+    import { test } from '@playwright/test';
+    test('foo', () => {});
+    `,
+  });
+
+  await page.getByText('a.test.ts').click();
+  await page.getByRole('listitem').filter({ hasText: 'a.test.ts' }).getByTitle('Watch').click();
+
+  await expect.poll(dumpTestTree(page)).toBe(`
+    ▼ ◯ a.test.ts 👁 <=
+        ◯ foo
+  `);
+
+  await writeFiles({
+    'a.test.ts': `
+    import { test } from '@playwright/test';
+    test('foo', () => {});
+    test('bar', () => {});
+    `,
+  });
+
+  await expect.poll(dumpTestTree(page)).toBe(`
+    ▼ ✅ a.test.ts 👁 <=
+        ✅ foo
+        ✅ bar
+  `);
+});
+
 test('should queue watches', async ({ runUITest, writeFiles, createLatch }) => {
   const latch = createLatch();
   const { page } = await runUITest({