chore: do not run all on watch (#20758)

packages/playwright-test/src/cli.ts

@@ -167,9 +167,8 @@ async function runTests(args: string[], opts: { [key: string]: any }) {
   config._internal.passWithNoTests = !!opts.passWithNoTests;
 
   const runner = new Runner(config);
-  const status = await runner.runAllTests(!!opts.watch);
+  const status = opts.watch ? await runner.watchAllTests() : await runner.runAllTests();
   await stopProfiling(undefined);
-
   if (status === 'interrupted')
     process.exit(130);
   process.exit(status === 'passed' ? 0 : 1);

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

@@ -47,7 +47,7 @@ export class Runner {
     return report;
   }
 
-  async runAllTests(watchMode: boolean): Promise<FullResult['status']> {
+  async runAllTests(): Promise<FullResult['status']> {
     const config = this._config;
     const listOnly = config._internal.listOnly;
     const deadline = config.globalTimeout ? monotonicTime() + config.globalTimeout : 0;
@@ -55,9 +55,9 @@ export class Runner {
     // Legacy webServer support.
     webServerPluginsForConfig(config).forEach(p => config._internal.plugins.push({ factory: p }));
 
-    const reporter = await createReporter(config, listOnly ? 'list' : watchMode ? 'watch' : 'run');
+    const reporter = await createReporter(config, listOnly ? 'list' : 'run');
     const taskRunner = listOnly ? createTaskRunnerForList(config, reporter)
-      : createTaskRunner(config, reporter, watchMode);
+      : createTaskRunner(config, reporter);
 
     const context: TaskRunnerState = {
       config,
@@ -78,16 +78,12 @@ export class Runner {
 
     const taskStatus = await taskRunner.run(context, deadline);
     let status: FullResult['status'] = 'passed';
-    const failedTests = context.rootSuite?.allTests().filter(test => !test.ok()) || [];
+    if (context.phases.find(p => p.dispatcher.hasWorkerErrors()) || context.rootSuite?.allTests().some(test => !test.ok()))
-    if (context.phases.find(p => p.dispatcher.hasWorkerErrors()) || failedTests.length)
       status = 'failed';
     if (status === 'passed' && taskStatus !== 'passed')
       status = taskStatus;
     await reporter.onExit({ status });
 
-    if (watchMode)
-      status = await runWatchModeLoop(config, failedTests);
-
     // Calling process.exit() might truncate large stdout/stderr output.
     // See https://github.com/nodejs/node/issues/6456.
     // See https://github.com/nodejs/node/issues/12921
@@ -95,6 +91,12 @@ export class Runner {
     await new Promise<void>(resolve => process.stderr.write('', () => resolve()));
     return status;
   }
+
+  async watchAllTests(): Promise<FullResult['status']> {
+    const config = this._config;
+    webServerPluginsForConfig(config).forEach(p => config._internal.plugins.push({ factory: p }));
+    return await runWatchModeLoop(config);
+  }
 }
 
 function sanitizeConfigForJSON(object: any, visited: Set<any>): any {

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

@@ -45,11 +45,17 @@ export class TaskRunner<Context> {
   }
 
   async run(context: Context, deadline: number): Promise<FullResult['status']> {
+    const { status, cleanup } = await this.runDeferCleanup(context, deadline);
+    const teardownStatus = await cleanup();
+    return status === 'passed' ? teardownStatus : status;
+  }
+
+  async runDeferCleanup(context: Context, deadline: number): Promise<{ status: FullResult['status'], cleanup: () => Promise<FullResult['status']> }> {
     const sigintWatcher = new SigIntWatcher();
     const timeoutWatcher = new TimeoutWatcher(deadline);
     const teardownRunner = new TaskRunner(this._reporter, this._globalTimeoutForError);
     teardownRunner._isTearDown = true;
-    try {
+
     let currentTaskName: string | undefined;
 
     const taskLoop = async () => {
@@ -85,6 +91,9 @@ export class TaskRunner<Context> {
       timeoutWatcher.promise,
     ]);
 
+    sigintWatcher.disarm();
+    timeoutWatcher.disarm();
+
     // Prevent subsequent tasks from running.
     this._interrupted = true;
 
@@ -98,13 +107,8 @@ export class TaskRunner<Context> {
       status = 'failed';
     }
 
-      return status;
+    const cleanup = () => teardownRunner.runDeferCleanup(context, deadline).then(r => r.status);
-    } finally {
+    return { status, cleanup };
-      sigintWatcher.disarm();
-      timeoutWatcher.disarm();
-      if (!this._isTearDown)
-        await teardownRunner.run(context, deadline);
-    }
   }
 }
 

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

@@ -50,27 +50,36 @@ export type TaskRunnerState = {
   }[];
 };
 
-export function createTaskRunner(config: FullConfigInternal, reporter: Multiplexer, doNotTeardown: boolean): TaskRunner<TaskRunnerState> {
+export function createTaskRunner(config: FullConfigInternal, reporter: Multiplexer): TaskRunner<TaskRunnerState> {
   const taskRunner = new TaskRunner<TaskRunnerState>(reporter, config.globalTimeout);
-
+  addGlobalSetupTasks(taskRunner, config);
-  for (const plugin of config._internal.plugins)
-    taskRunner.addTask('plugin setup', createPluginSetupTask(plugin, doNotTeardown));
-  if (config.globalSetup || config.globalTeardown)
-    taskRunner.addTask('global setup', createGlobalSetupTask(doNotTeardown));
   taskRunner.addTask('load tests', createLoadTask('in-process'));
-  taskRunner.addTask('clear output', createRemoveOutputDirsTask());
+  addRunTasks(taskRunner, config);
-  addCommonTasks(taskRunner, config);
+  return taskRunner;
+}
+
+export function createTaskRunnerForWatchSetup(config: FullConfigInternal, reporter: Multiplexer): TaskRunner<TaskRunnerState> {
+  const taskRunner = new TaskRunner<TaskRunnerState>(reporter, 0);
+  addGlobalSetupTasks(taskRunner, config);
   return taskRunner;
 }
 
 export function createTaskRunnerForWatch(config: FullConfigInternal, reporter: Multiplexer, projectsToIgnore?: Set<FullProjectInternal>, additionalFileMatcher?: Matcher): TaskRunner<TaskRunnerState> {
-  const taskRunner = new TaskRunner<TaskRunnerState>(reporter, config.globalTimeout);
+  const taskRunner = new TaskRunner<TaskRunnerState>(reporter, 0);
   taskRunner.addTask('load tests', createLoadTask('out-of-process', projectsToIgnore, additionalFileMatcher));
-  addCommonTasks(taskRunner, config);
+  addRunTasks(taskRunner, config);
   return taskRunner;
 }
 
-function addCommonTasks(taskRunner: TaskRunner<TaskRunnerState>, config: FullConfigInternal) {
+function addGlobalSetupTasks(taskRunner: TaskRunner<TaskRunnerState>, config: FullConfigInternal) {
+  for (const plugin of config._internal.plugins)
+    taskRunner.addTask('plugin setup', createPluginSetupTask(plugin));
+  if (config.globalSetup || config.globalTeardown)
+    taskRunner.addTask('global setup', createGlobalSetupTask());
+  taskRunner.addTask('clear output', createRemoveOutputDirsTask());
+}
+
+function addRunTasks(taskRunner: TaskRunner<TaskRunnerState>, config: FullConfigInternal) {
   taskRunner.addTask('create tasks', createTestGroupsTask());
   taskRunner.addTask('report begin', async ({ reporter, rootSuite }) => {
     reporter.onBegin?.(config, rootSuite!);
@@ -93,14 +102,14 @@ export function createTaskRunnerForList(config: FullConfigInternal, reporter: Mu
   return taskRunner;
 }
 
-function createPluginSetupTask(plugin: TestRunnerPluginRegistration, doNotTeardown: boolean): Task<TaskRunnerState> {
+function createPluginSetupTask(plugin: TestRunnerPluginRegistration): Task<TaskRunnerState> {
   return async ({ config, reporter }) => {
     if (typeof plugin.factory === 'function')
       plugin.instance = await plugin.factory();
     else
       plugin.instance = plugin.factory;
     await plugin.instance?.setup?.(config, config._internal.configDir, reporter);
-    return doNotTeardown ? undefined : () => plugin.instance?.teardown?.();
+    return () => plugin.instance?.teardown?.();
   };
 }
 
@@ -111,12 +120,12 @@ function createPluginBeginTask(plugin: TestRunnerPluginRegistration): Task<TaskR
   };
 }
 
-function createGlobalSetupTask(doNotTeardown: boolean): Task<TaskRunnerState> {
+function createGlobalSetupTask(): Task<TaskRunnerState> {
   return async ({ config }) => {
     const setupHook = config.globalSetup ? await loadGlobalHook(config, config.globalSetup) : undefined;
     const teardownHook = config.globalTeardown ? await loadGlobalHook(config, config.globalTeardown) : undefined;
     const globalSetupResult = setupHook ? await setupHook(config) : undefined;
-    return doNotTeardown ? undefined : async () => {
+    return async () => {
       if (typeof globalSetupResult === 'function')
         await globalSetupResult();
       await teardownHook?.(config);

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

@@ -20,13 +20,13 @@ import type { FullConfigInternal, FullProjectInternal } from '../common/types';
 import { Multiplexer } from '../reporters/multiplexer';
 import { createFileMatcherFromArguments } from '../util';
 import type { Matcher } from '../util';
-import { createTaskRunnerForWatch } from './tasks';
+import { createTaskRunnerForWatch, createTaskRunnerForWatchSetup } from './tasks';
 import type { TaskRunnerState } from './tasks';
 import { buildProjectsClosure, filterProjects } from './projectUtils';
 import { clearCompilationCache, collectAffectedTestFiles } from '../common/compilationCache';
-import type { FullResult, TestCase } from 'packages/playwright-test/reporter';
+import type { FullResult } from 'packages/playwright-test/reporter';
 import chokidar from 'chokidar';
-import { WatchModeReporter } from './reporters';
+import { createReporter, WatchModeReporter } from './reporters';
 import { colors } from 'playwright-core/lib/utilsBundle';
 import { enquirer } from '../utilsBundle';
 import { separator } from '../reporters/base';
@@ -63,11 +63,24 @@ class FSWatcher {
   }
 }
 
-export async function runWatchModeLoop(config: FullConfigInternal, failedTests: TestCase[]): Promise<FullResult['status']> {
+export async function runWatchModeLoop(config: FullConfigInternal): Promise<FullResult['status']> {
+  // Perform global setup.
+  const reporter = await createReporter(config, 'watch');
+  const context: TaskRunnerState = {
+    config,
+    reporter,
+    phases: [],
+  };
+  const taskRunner = createTaskRunnerForWatchSetup(config, reporter);
+  reporter.onConfigure(config);
+  const { status, cleanup: globalCleanup } = await taskRunner.runDeferCleanup(context, 0);
+  if (status !== 'passed')
+    return await globalCleanup();
+
   const projects = filterProjects(config.projects, config._internal.cliProjectFilter);
   const projectClosure = buildProjectsClosure(projects);
   config._internal.passWithNoTests = true;
-  const failedTestIdCollector = new Set(failedTests.map(t => t.id));
+  const failedTestIdCollector = new Set<string>();
 
   const originalCliArgs = config._internal.cliArgs;
   const originalCliGrep = config._internal.cliGrep;
@@ -75,12 +88,14 @@ export async function runWatchModeLoop(config: FullConfigInternal, failedTests:
 
   let lastRun: { type: 'changed' | 'regular' | 'failed', failedTestIds?: Set<string>, dirtyFiles?: Set<string> } = { type: 'regular' };
 
+  let result: FullResult['status'] = 'passed';
+
   const fsWatcher = new FSWatcher(projectClosure.map(p => p.testDir));
   while (true) {
     const sep = separator();
     process.stdout.write(`
 ${sep}
-Waiting for file changes. Press ${colors.bold('h')} for help or ${colors.bold('q')} to quit.
+Waiting for file changes. Press ${colors.bold('a')} to run all, ${colors.bold('q')} to quit or ${colors.bold('h')} for more options.
 `);
     const readCommandPromise = readCommand();
     await Promise.race([
@@ -167,11 +182,15 @@ Waiting for file changes. Press ${colors.bold('h')} for help or ${colors.bold('q
     }
 
     if (command === 'exit')
-      return 'passed';
+      break;
 
-    if (command === 'interrupted')
+    if (command === 'interrupted') {
-      return 'interrupted';
+      result = 'interrupted';
+      break;
     }
+  }
+
+  return result === 'passed' ? await globalCleanup() : result;
 }
 
 async function runChangedTests(config: FullConfigInternal, failedTestIdCollector: Set<string>, projectClosure: FullProjectInternal[], changedFiles: Set<string>) {

tests/playwright-test/global-setup.spec.ts

@@ -366,7 +366,7 @@ test('teardown after error', async ({ runInlineTest }) => {
       pwt.test('test', () => {});
     `,
   });
-  expect(result.exitCode).toBe(0);
+  expect(result.exitCode).toBe(1);
   expect(result.passed).toBe(1);
   const output = result.output;
   expect(output).toContain('Error: failed teardown 1');