chore(ui): queue watch runs (#21809)

packages/playwright-core/src/server/browserType.ts

@@ -75,7 +75,7 @@ export abstract class BrowserType extends SdkObject {
     return browser;
   }
 
-  async launchPersistentContext(metadata: CallMetadata, userDataDir: string, options: channels.BrowserTypeLaunchPersistentContextOptions & { useWebSocket?: boolean, ignoreChromiumSwitches?: boolean }): Promise<BrowserContext> {
+  async launchPersistentContext(metadata: CallMetadata, userDataDir: string, options: channels.BrowserTypeLaunchPersistentContextOptions & { useWebSocket?: boolean }): Promise<BrowserContext> {
     options = this._validateLaunchOptions(options);
     const controller = new ProgressController(metadata, this);
     const persistent: channels.BrowserNewContextParams = options;

packages/playwright-core/src/server/chromium/chromium.ts

@@ -283,7 +283,7 @@ export class Chromium extends BrowserType {
       throw new Error('Playwright manages remote debugging connection itself.');
     if (args.find(arg => !arg.startsWith('-')))
       throw new Error('Arguments can not specify page to be opened');
-    const chromeArguments = options.ignoreChromiumSwitches ? [] : [...chromiumSwitches];
+    const chromeArguments = [...chromiumSwitches];
 
     if (os.platform() === 'darwin') {
       // See https://github.com/microsoft/playwright/issues/7362

packages/playwright-core/src/server/recorder/recorderApp.ts

@@ -128,11 +128,8 @@ export class RecorderApp extends EventEmitter implements IRecorderApp {
       channel: findChromiumChannel(sdkLanguage),
       args,
       noDefaultViewport: true,
+      ignoreDefaultArgs: ['--enable-automation'],
       colorScheme: 'no-override',
-      // Moving the mouse while starting Chromium on macOS kills the mouse.
-      // There is no exact switch that we can blame, but removing all reduces the
-      // probability of this happening by a couple of orders.
-      ignoreChromiumSwitches: true,
       headless: !!process.env.PWTEST_CLI_HEADLESS || (isUnderTest() && !headed),
       useWebSocket: !!process.env.PWTEST_RECORDER_PORT,
       handleSIGINT,

packages/playwright-core/src/server/trace/viewer/traceViewer.ts

@@ -86,11 +86,8 @@ export async function showTraceViewer(traceUrls: string[], browserName: string,
     channel: findChromiumChannel(traceViewerPlaywright.options.sdkLanguage),
     args,
     noDefaultViewport: true,
-    // Moving the mouse while starting Chromium on macOS kills the mouse.
-    // There is no exact switch that we can blame, but removing all reduces the
-    // probability of this happening by a couple of orders.
-    ignoreChromiumSwitches: true,
     headless,
+    ignoreDefaultArgs: ['--enable-automation'],
     colorScheme: 'no-override',
     useWebSocket: isUnderTest(),
   });

packages/playwright-core/src/server/types.ts

@@ -149,7 +149,7 @@ export type NormalizedContinueOverrides = {
 
 export type EmulatedSize = { viewport: Size, screen: Size };
 
-export type LaunchOptions = channels.BrowserTypeLaunchOptions & { useWebSocket?: boolean, ignoreChromiumSwitches?: boolean };
+export type LaunchOptions = channels.BrowserTypeLaunchOptions & { useWebSocket?: boolean };
 
 export type ProtocolLogger = (direction: 'send' | 'receive', message: object) => void;
 

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

@@ -96,14 +96,16 @@ class UIMode {
 
   async showUI() {
     this._page = await showTraceViewer([], 'chromium', { app: 'watch.html', headless: isUnderTest() && process.env.PWTEST_HEADED_FOR_TEST !== '1' });
-    process.stdout.write = (chunk: string | Buffer) => {
-      this._dispatchEvent({ method: 'stdio', params: chunkToPayload('stdout', chunk) });
-      return true;
-    };
-    process.stderr.write = (chunk: string | Buffer) => {
-      this._dispatchEvent({ method: 'stdio', params: chunkToPayload('stderr', chunk) });
-      return true;
-    };
+    if (!process.env.PWTEST_DEBUG) {
+      process.stdout.write = (chunk: string | Buffer) => {
+        this._dispatchEvent({ method: 'stdio', params: chunkToPayload('stdout', chunk) });
+        return true;
+      };
+      process.stderr.write = (chunk: string | Buffer) => {
+        this._dispatchEvent({ method: 'stdio', params: chunkToPayload('stderr', chunk) });
+        return true;
+      };
+    }
     const exitPromise = new ManualPromise();
     this._page.on('close', () => exitPromise.resolve());
     let queue = Promise.resolve();
@@ -193,7 +195,7 @@ class UIMode {
       dependenciesForTestFile(fileName).forEach(file => files.add(file));
     }
     const watchedFiles = [...files].sort();
-    if (this._testWatcher && JSON.stringify(this._testWatcher.watchedFiles.toString()) === JSON.stringify(watchedFiles))
+    if (this._testWatcher && JSON.stringify(this._testWatcher.watchedFiles) === JSON.stringify(watchedFiles))
       return;
 
     if (this._testWatcher) {

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

@@ -69,10 +69,11 @@ export const WatchModeView: React.FC<{}> = ({
   const [testModel, setTestModel] = React.useState<TestModel>({ config: undefined, rootSuite: undefined });
   const [progress, setProgress] = React.useState<Progress & { total: number } | undefined>();
   const [selectedItem, setSelectedItem] = React.useState<{ location?: Location, testCase?: TestCase }>({});
-  const [visibleTestIds, setVisibleTestIds] = React.useState<string[]>([]);
+  const [visibleTestIds, setVisibleTestIds] = React.useState<Set<string>>(new Set());
   const [isLoading, setIsLoading] = React.useState<boolean>(false);
   const [runningState, setRunningState] = React.useState<{ testIds: Set<string>, itemSelectedByUser?: boolean } | undefined>();
   const [watchAll, setWatchAll] = useSetting<boolean>('watch-all', false);
+  const runTestPromiseChain = React.useRef(Promise.resolve());
 
   const inputRef = React.useRef<HTMLInputElement>(null);
 
@@ -110,22 +111,26 @@ export const WatchModeView: React.FC<{}> = ({
       setProgress(undefined);
   };
 
-  const runTests = (testIds: string[]) => {
-    // Clear test results.
-    {
-      const testIdSet = new Set(testIds);
-      for (const test of testModel.rootSuite?.allTests() || []) {
-        if (testIdSet.has(test.id))
-          (test as TeleTestCase)._createTestResult('pending');
-      }
-      setTestModel({ ...testModel });
-    }
+  const runTests = React.useCallback((mode: 'queue-if-busy' | 'bounce-if-busy', testIds: Set<string>) => {
+    if (mode === 'bounce-if-busy' && runningState)
+      return;
 
-    const time = '  [' + new Date().toLocaleTimeString() + ']';
-    xtermDataSource.write('\x1B[2m—'.repeat(Math.max(0, xtermSize.cols - time.length)) + time + '\x1B[22m');
-    setProgress({ total: testIds.length, passed: 0, failed: 0, skipped: 0 });
-    setRunningState({ testIds: new Set(testIds) });
-    sendMessage('run', { testIds }).then(() => {
+    runTestPromiseChain.current = runTestPromiseChain.current.then(async () => {
+      // Clear test results.
+      {
+        for (const test of testModel.rootSuite?.allTests() || []) {
+          if (testIds.has(test.id))
+            (test as TeleTestCase)._createTestResult('pending');
+        }
+        setTestModel({ ...testModel });
+      }
+
+      const time = '  [' + new Date().toLocaleTimeString() + ']';
+      xtermDataSource.write('\x1B[2m—'.repeat(Math.max(0, xtermSize.cols - time.length)) + time + '\x1B[22m');
+      setProgress({ total: testIds.size, passed: 0, failed: 0, skipped: 0 });
+      setRunningState({ testIds });
+
+      await sendMessage('run', { testIds: [...testIds] });
       // Clear pending tests in case of interrupt.
       for (const test of testModel.rootSuite?.allTests() || []) {
         if (test.results[0]?.duration === -1)
@@ -134,7 +139,7 @@ export const WatchModeView: React.FC<{}> = ({
       setTestModel({ ...testModel });
       setRunningState(undefined);
     });
-  };
+  }, [runningState, testModel]);
 
   const isRunningTest = !!runningState;
 
@@ -171,7 +176,7 @@ export const WatchModeView: React.FC<{}> = ({
           projectFilters={projectFilters}
           setProjectFilters={setProjectFilters}
           testModel={testModel}
-          runTests={() => runTests(visibleTestIds)} />
+          runTests={() => runTests('bounce-if-busy', visibleTestIds)} />
         <Toolbar noMinHeight={true}>
           {!isRunningTest && !progress && <div className='section-title'>Tests</div>}
           {!isRunningTest && progress && <div data-testid='status-line' className='status-line'>
@@ -180,7 +185,7 @@ export const WatchModeView: React.FC<{}> = ({
           {isRunningTest && progress && <div data-testid='status-line' className='status-line'>
             <div>Running {progress.passed}/{runningState.testIds.size} passed ({(progress.passed / runningState.testIds.size) * 100 | 0}%)</div>
           </div>}
-          <ToolbarButton icon='play' title='Run all' onClick={() => runTests(visibleTestIds)} disabled={isRunningTest || isLoading}></ToolbarButton>
+          <ToolbarButton icon='play' title='Run all' onClick={() => runTests('bounce-if-busy', visibleTestIds)} disabled={isRunningTest || isLoading}></ToolbarButton>
           <ToolbarButton icon='debug-stop' title='Stop' onClick={() => sendMessageNoReply('stop')} disabled={!isRunningTest || isLoading}></ToolbarButton>
         </Toolbar>
         <TestList
@@ -274,11 +279,11 @@ const TestList: React.FC<{
   projectFilters: Map<string, boolean>,
   filterText: string,
   testModel: { rootSuite: Suite | undefined, config: FullConfig | undefined },
-  runTests: (testIds: string[]) => void,
+  runTests: (mode: 'bounce-if-busy' | 'queue-if-busy', testIds: Set<string>) => void,
   runningState?: { testIds: Set<string>, itemSelectedByUser?: boolean },
   watchAll?: boolean,
   isLoading?: boolean,
-  setVisibleTestIds: (testIds: string[]) => void,
+  setVisibleTestIds: (testIds: Set<string>) => void,
   onItemSelected: (item: { testCase?: TestCase, location?: Location }) => void,
 }> = ({ statusFilters, projectFilters, filterText, testModel, runTests, runningState, watchAll, isLoading, onItemSelected, setVisibleTestIds }) => {
   const [treeState, setTreeState] = React.useState<TreeState>({ expandedItems: new Map() });
@@ -302,7 +307,7 @@ const TestList: React.FC<{
       treeItemMap.set(treeItem.id, treeItem);
     };
     visit(rootItem);
-    setVisibleTestIds([...visibleTestIds]);
+    setVisibleTestIds(visibleTestIds);
     return { rootItem, treeItemMap, fileNames };
   }, [filterText, testModel, statusFilters, projectFilters, setVisibleTestIds]);
 
@@ -349,7 +354,7 @@ const TestList: React.FC<{
       const fileNames = new Set<string>();
       for (const itemId of watchedTreeIds.value) {
         const treeItem = treeItemMap.get(itemId)!;
-        const fileName = fileNameForTreeItem(treeItem);
+        const fileName = treeItem.location.file;
         if (fileName)
           fileNames.add(fileName);
       }
@@ -359,29 +364,30 @@ const TestList: React.FC<{
 
   const runTreeItem = (treeItem: TreeItem) => {
     setSelectedTreeItemId(treeItem.id);
-    runTests(collectTestIds(treeItem));
+    runTests('bounce-if-busy', collectTestIds(treeItem));
   };
 
-  runWatchedTests = (fileNames: string[]) => {
+  runWatchedTests = (changedTestFiles: string[]) => {
     const testIds: string[] = [];
-    const set = new Set(fileNames);
+    const set = new Set(changedTestFiles);
     if (watchAll) {
       const visit = (treeItem: TreeItem) => {
-        const fileName = fileNameForTreeItem(treeItem);
+        const fileName = treeItem.location.file;
         if (fileName && set.has(fileName))
           testIds.push(...collectTestIds(treeItem));
-        treeItem.children.forEach(visit);
+        if (treeItem.kind === 'group' && treeItem.subKind === 'folder')
+          treeItem.children.forEach(visit);
       };
       visit(rootItem);
     } else {
       for (const treeId of watchedTreeIds.value) {
         const treeItem = treeItemMap.get(treeId)!;
-        const fileName = fileNameForTreeItem(treeItem);
+        const fileName = treeItem.location.file;
         if (fileName && set.has(fileName))
           testIds.push(...collectTestIds(treeItem));
       }
     }
-    runTests(testIds);
+    runTests('queue-if-busy', new Set(testIds));
   };
 
   return <TestTreeView
@@ -598,25 +604,22 @@ const outputDirForTestCase = (testCase: TestCase): string | undefined => {
   return undefined;
 };
 
-const fileNameForTreeItem = (treeItem?: TreeItem): string | undefined => {
-  return treeItem?.location.file;
-};
-
 const locationToOpen = (treeItem?: TreeItem) => {
   if (!treeItem)
     return;
   return treeItem.location.file + ':' + treeItem.location.line;
 };
 
-const collectTestIds = (treeItem?: TreeItem): string[] => {
+const collectTestIds = (treeItem?: TreeItem): Set<string> => {
+  const testIds = new Set<string>();
   if (!treeItem)
-    return [];
-  const testIds: string[] = [];
+    return testIds;
+
   const visit = (treeItem: TreeItem) => {
     if (treeItem.kind === 'case')
-      testIds.push(...treeItem.tests.map(t => t.id));
+      treeItem.tests.map(t => t.id).forEach(id => testIds.add(id));
     else if (treeItem.kind === 'test')
-      testIds.push(treeItem.id);
+      testIds.add(treeItem.id);
     else
       treeItem.children?.forEach(visit);
   };

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

@@ -21,9 +21,16 @@ import type { TestChildProcess } from '../config/commonFixtures';
 import { cleanEnv, cliEntrypoint, removeFolderAsync, test as base, writeFiles } from './playwright-test-fixtures';
 import type { Files, RunOptions } from './playwright-test-fixtures';
 import type { Browser, Page, TestInfo } from './stable-test-runner';
+import { createGuid } from '../../packages/playwright-core/src/utils/crypto';
+
+type Latch = {
+  blockingCode: string;
+  open: () => void;
+};
 
 type Fixtures = {
   runUITest: (files: Files, env?: NodeJS.ProcessEnv, options?: RunOptions) => Promise<Page>;
+  createLatch: () => Latch;
 };
 
 export function dumpTestTree(page: Page): () => Promise<string> {
@@ -99,6 +106,21 @@ export const test = base
         await testProcess?.close();
         await removeFolderAsync(cacheDir);
       },
+      createLatch: async ({}, use, testInfo) => {
+        await use(() => {
+          const latchFile = path.join(testInfo.project.outputDir, createGuid() + '.latch');
+          return {
+            blockingCode: `await ((${waitForLatch})(${JSON.stringify(latchFile)}))`,
+            open: () => fs.writeFileSync(latchFile, 'ok'),
+          };
+        });
+      },
     });
 
 export { expect } from './stable-test-runner';
+
+async function waitForLatch(latchFile: string) {
+  const fs = require('fs');
+  while (!fs.existsSync(latchFile))
+    await new Promise(f => setTimeout(f, 250));
+}

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

@@ -219,3 +219,46 @@ test('should watch new file', async ({ runUITest, writeFiles }) => {
         ✅ test
   `);
 });
+
+test('should queue watches', async ({ runUITest, writeFiles, createLatch }) => {
+  const latch = createLatch();
+  const page = await runUITest({
+    'a.test.ts': `import { test } from '@playwright/test'; test('test', () => {});`,
+    'b.test.ts': `import { test } from '@playwright/test'; test('test', async () => {
+      ${latch.blockingCode}
+    });`,
+    'c.test.ts': `import { test } from '@playwright/test'; test('test', () => {});`,
+    'd.test.ts': `import { test } from '@playwright/test'; test('test', () => {});`,
+  });
+
+  await expect.poll(dumpTestTree(page), { timeout: 15000 }).toBe(`
+    ▼ ◯ a.test.ts
+        ◯ test
+    ▼ ◯ b.test.ts
+        ◯ test
+    ▼ ◯ c.test.ts
+        ◯ test
+    ▼ ◯ d.test.ts
+        ◯ test
+  `);
+
+  await page.getByTitle('Watch all').click();
+  await page.getByTitle('Run all').click();
+
+  await expect(page.getByTestId('status-line')).toHaveText('Running 1/4 passed (25%)', { timeout: 15000 });
+
+  await writeFiles({
+    'a.test.ts': `import { test } from '@playwright/test'; test('test', () => {});`,
+    'b.test.ts': `import { test } from '@playwright/test'; test('test', () => {});`,
+    'c.test.ts': `import { test } from '@playwright/test'; test('test', () => {});`,
+  });
+
+  // Now watches should not kick in.
+  await new Promise(f => setTimeout(f, 1000));
+  await expect(page.getByTestId('status-line')).toHaveText('Running 1/4 passed (25%)', { timeout: 15000 });
+
+  // Allow test to finish and new watch to  kick in.
+  latch.open();
+
+  await expect(page.getByTestId('status-line')).toHaveText('3/3 passed (100%)', { timeout: 15000 });
+});