chore: remove addFatalError (#20383)

packages/playwright-test/src/fixtures.ts

@@ -20,7 +20,6 @@ import type { FixturesWithLocation, Location, WorkerInfo } from './types';
 import { ManualPromise } from 'playwright-core/lib/utils';
 import type { TestInfoImpl } from './testInfo';
 import type { FixtureDescription, TimeoutManager } from './timeoutManager';
-import { addFatalError } from './globals';
 
 type FixtureScope = 'test' | 'worker';
 type FixtureAuto = boolean | 'all-hooks-included';
@@ -52,6 +51,10 @@ type FixtureRegistration = {
   // Whether this fixture is an option value set from the config.
   fromConfig?: boolean;
 };
+export type LoadError = {
+  message: string;
+  location: Location;
+};
 
 class Fixture {
   runner: FixtureRunner;
@@ -187,9 +190,11 @@ export function isFixtureOption(value: any): value is FixtureTuple {
 export class FixturePool {
   readonly digest: string;
   readonly registrations: Map<string, FixtureRegistration>;
+  private _onLoadError: (error: LoadError) => void;
 
-  constructor(fixturesList: FixturesWithLocation[], parentPool?: FixturePool, disallowWorkerFixtures?: boolean) {
+  constructor(fixturesList: FixturesWithLocation[], onLoadError: (error: LoadError) => void, parentPool?: FixturePool, disallowWorkerFixtures?: boolean) {
     this.registrations = new Map(parentPool ? parentPool.registrations : []);
+    this._onLoadError = onLoadError;
 
     for (const { fixtures, location, fromConfig } of fixturesList) {
       for (const entry of Object.entries(fixtures)) {
@@ -211,11 +216,11 @@ export class FixturePool {
         const previous = this.registrations.get(name);
         if (previous && options) {
           if (previous.scope !== options.scope) {
-            addFatalError(`Fixture "${name}" has already been registered as a { scope: '${previous.scope}' } fixture defined in ${formatLocation(previous.location)}.`, location);
+            this._addLoadError(`Fixture "${name}" has already been registered as a { scope: '${previous.scope}' } fixture defined in ${formatLocation(previous.location)}.`, location);
             continue;
           }
           if (previous.auto !== options.auto) {
-            addFatalError(`Fixture "${name}" has already been registered as a { auto: '${previous.scope}' } fixture defined in ${formatLocation(previous.location)}.`, location);
+            this._addLoadError(`Fixture "${name}" has already been registered as a { auto: '${previous.scope}' } fixture defined in ${formatLocation(previous.location)}.`, location);
             continue;
           }
         } else if (previous) {
@@ -225,11 +230,11 @@ export class FixturePool {
         }
 
         if (!kScopeOrder.includes(options.scope)) {
-          addFatalError(`Fixture "${name}" has unknown { scope: '${options.scope}' }.`, location);
+          this._addLoadError(`Fixture "${name}" has unknown { scope: '${options.scope}' }.`, location);
           continue;
         }
         if (options.scope === 'worker' && disallowWorkerFixtures) {
-          addFatalError(`Cannot use({ ${name} }) in a describe group, because it forces a new worker.\nMake it top-level in the test file or put in the configuration file.`, location);
+          this._addLoadError(`Cannot use({ ${name} }) in a describe group, because it forces a new worker.\nMake it top-level in the test file or put in the configuration file.`, location);
           continue;
         }
 
@@ -242,7 +247,7 @@ export class FixturePool {
           fn = original.fn;
         }
 
-        const deps = fixtureParameterNames(fn, location);
+        const deps = fixtureParameterNames(fn, location, e => this._onLoadError(e));
         const registration: FixtureRegistration = { id: '', name, location, scope: options.scope, fn, auto: options.auto, option: options.option, timeout: options.timeout, customTitle: options.customTitle, deps, super: previous, fromConfig };
         registrationId(registration);
         this.registrations.set(name, registration);
@@ -262,13 +267,13 @@ export class FixturePool {
         const dep = this.resolveDependency(registration, name);
         if (!dep) {
           if (name === registration.name)
-            addFatalError(`Fixture "${registration.name}" references itself, but does not have a base implementation.`, registration.location);
+            this._addLoadError(`Fixture "${registration.name}" references itself, but does not have a base implementation.`, registration.location);
           else
-            addFatalError(`Fixture "${registration.name}" has unknown parameter "${name}".`, registration.location);
+            this._addLoadError(`Fixture "${registration.name}" has unknown parameter "${name}".`, registration.location);
           continue;
         }
         if (kScopeOrder.indexOf(registration.scope) > kScopeOrder.indexOf(dep.scope)) {
-          addFatalError(`${registration.scope} fixture "${registration.name}" cannot depend on a ${dep.scope} fixture "${name}" defined in ${formatLocation(dep.location)}.`, registration.location);
+          this._addLoadError(`${registration.scope} fixture "${registration.name}" cannot depend on a ${dep.scope} fixture "${name}" defined in ${formatLocation(dep.location)}.`, registration.location);
           continue;
         }
         if (!markers.has(dep)) {
@@ -277,7 +282,7 @@ export class FixturePool {
           const index = stack.indexOf(dep);
           const regs = stack.slice(index, stack.length);
           const names = regs.map(r => `"${r.name}"`);
-          addFatalError(`Fixtures ${names.join(' -> ')} -> "${dep.name}" form a dependency cycle: ${regs.map(r => formatLocation(r.location)).join(' -> ')}`, dep.location);
+          this._addLoadError(`Fixtures ${names.join(' -> ')} -> "${dep.name}" form a dependency cycle: ${regs.map(r => formatLocation(r.location)).join(' -> ')}`, dep.location);
           continue;
         }
       }
@@ -297,10 +302,10 @@ export class FixturePool {
   }
 
   validateFunction(fn: Function, prefix: string, location: Location) {
-    for (const name of fixtureParameterNames(fn, location)) {
+    for (const name of fixtureParameterNames(fn, location, e => this._onLoadError(e))) {
       const registration = this.registrations.get(name);
       if (!registration)
-        addFatalError(`${prefix} has unknown parameter "${name}".`, location);
+        this._addLoadError(`${prefix} has unknown parameter "${name}".`, location);
     }
   }
 
@@ -309,6 +314,10 @@ export class FixturePool {
       return registration.super;
     return this.registrations.get(name);
   }
+
+  private _addLoadError(message: string, location: Location) {
+    this._onLoadError({ message, location });
+  }
 }
 
 export class FixtureRunner {
@@ -368,7 +377,7 @@ export class FixtureRunner {
     }
 
     // Install used fixtures.
-    const names = fixtureParameterNames(fn, { file: '<unused>', line: 1, column: 1 });
+    const names = fixtureParameterNames(fn, { file: '<unused>', line: 1, column: 1 }, serializeAndThrowError);
     const params: { [key: string]: any } = {};
     for (const name of names) {
       const registration = this.pool!.registrations.get(name)!;
@@ -404,7 +413,7 @@ export class FixtureRunner {
   }
 
   dependsOnWorkerFixturesOnly(fn: Function, location: Location): boolean {
-    const names = fixtureParameterNames(fn, location);
+    const names = fixtureParameterNames(fn, location, serializeAndThrowError);
     for (const name of names) {
       const registration = this.pool!.registrations.get(name)!;
       if (registration.scope !== 'worker')
@@ -414,17 +423,21 @@ export class FixtureRunner {
   }
 }
 
+function serializeAndThrowError(e: LoadError) {
+  throw new Error(`${formatLocation(e.location!)}: ${e.message}`);
+}
+
 const signatureSymbol = Symbol('signature');
 
-function fixtureParameterNames(fn: Function | any, location: Location): string[] {
+function fixtureParameterNames(fn: Function | any, location: Location, onError: (error: LoadError) => void): string[] {
   if (typeof fn !== 'function')
     return [];
   if (!fn[signatureSymbol])
-    fn[signatureSymbol] = innerFixtureParameterNames(fn, location);
+    fn[signatureSymbol] = innerFixtureParameterNames(fn, location, onError);
   return fn[signatureSymbol];
 }
 
-function innerFixtureParameterNames(fn: Function, location: Location): string[] {
+function innerFixtureParameterNames(fn: Function, location: Location, onError: (error: LoadError) => void): string[] {
   const text = fn.toString();
   const match = text.match(/(?:async)?(?:\s+function)?[^(]*\(([^)]*)/);
   if (!match)
@@ -434,7 +447,7 @@ function innerFixtureParameterNames(fn: Function, location: Location): string[]
     return [];
   const [firstParam] = splitByComma(trimmedParams);
   if (firstParam[0] !== '{' || firstParam[firstParam.length - 1] !== '}')
-    addFatalError('First argument must use the object destructuring pattern: '  + firstParam, location);
+    onError({ message: 'First argument must use the object destructuring pattern: '  + firstParam, location });
   const props = splitByComma(firstParam.substring(1, firstParam.length - 1)).map(prop => {
     const colon = prop.indexOf(':');
     return colon === -1 ? prop : prop.substring(0, colon).trim();

packages/playwright-test/src/globals.ts

@@ -16,8 +16,6 @@
 
 import type { TestInfoImpl } from './testInfo';
 import type { Suite } from './test';
-import type { TestError, Location } from '../types/testReporter';
-import { formatLocation } from './util';
 
 let currentTestInfoValue: TestInfoImpl | null = null;
 export function setCurrentTestInfo(testInfo: TestInfoImpl | null) {
@@ -34,15 +32,3 @@ export function setCurrentlyLoadingFileSuite(suite: Suite | undefined) {
 export function currentlyLoadingFileSuite() {
   return currentFileSuite;
 }
-
-let _fatalErrorSink: ((fatalError: TestError) => void) | undefined;
-export function setFatalErrorSink(fatalErrorSink: (fatalError: TestError) => void) {
-  _fatalErrorSink = fatalErrorSink;
-}
-
-export function addFatalError(message: string, location: Location) {
-  if (_fatalErrorSink)
-    _fatalErrorSink({ message: `Error: ${message}`, location });
-  else
-    throw new Error(`${formatLocation(location)}: ${message}`);
-}

packages/playwright-test/src/loaderMain.ts

@@ -15,11 +15,10 @@
  */
 
 import type { SerializedConfig } from './ipc';
-import type { TestError } from '../reporter';
 import { ConfigLoader } from './configLoader';
 import { ProcessRunner } from './process';
 import { loadTestFilesInProcess } from './testLoader';
-import { setFatalErrorSink } from './globals';
+import type { LoadError } from './fixtures';
 
 export class LoaderMain extends ProcessRunner {
   private _config: SerializedConfig;
@@ -37,8 +36,7 @@ export class LoaderMain extends ProcessRunner {
   }
 
   async loadTestFiles(params: { files: string[] }) {
-    const loadErrors: TestError[] = [];
+    const loadErrors: LoadError[] = [];
-    setFatalErrorSink(error => loadErrors.push(error));
     const configLoader = await this._configLoader();
     const rootSuite = await loadTestFilesInProcess(configLoader.fullConfig(), params.files, loadErrors);
     return { rootSuite: rootSuite._deepSerialize(), loadErrors };

packages/playwright-test/src/poolBuilder.ts

@@ -15,25 +15,29 @@
  */
 
 import { FixturePool, isFixtureOption } from './fixtures';
+import type { LoadError } from './fixtures';
 import type { Suite, TestCase } from './test';
 import type { TestTypeImpl } from './testType';
 import type { Fixtures, FixturesWithLocation, FullProjectInternal } from './types';
+import { formatLocation } from './util';
 
 export class PoolBuilder {
   private _project: FullProjectInternal | undefined;
   private _testTypePools = new Map<TestTypeImpl, FixturePool>();
   private _type: 'loader' | 'worker';
+  private _loadErrors: LoadError[] | undefined;
 
-  static buildForLoader(suite: Suite) {
+  static buildForLoader(suite: Suite, loadErrors: LoadError[]) {
-    new PoolBuilder('loader').buildPools(suite);
+    new PoolBuilder('loader', loadErrors).buildPools(suite);
   }
 
   static createForWorker(project: FullProjectInternal) {
-    return new PoolBuilder('worker', project);
+    return new PoolBuilder('worker', undefined, project);
   }
 
-  private constructor(type: 'loader' | 'worker', project?: FullProjectInternal) {
+  private constructor(type: 'loader' | 'worker', loadErrors?: LoadError[], project?: FullProjectInternal) {
     this._type = type;
+    this._loadErrors = loadErrors;
     this._project = project;
   }
 
@@ -57,7 +61,7 @@ export class PoolBuilder {
 
     for (const parent of parents) {
       if (parent._use.length)
-        pool = new FixturePool(parent._use, pool, parent._type === 'describe');
+        pool = new FixturePool(parent._use, e => this._onLoadError(e), pool, parent._type === 'describe');
       for (const hook of parent._hooks)
         pool.validateFunction(hook.fn, hook.type + ' hook', hook.location);
       for (const modifier of parent._modifiers)
@@ -71,12 +75,19 @@ export class PoolBuilder {
   private _buildTestTypePool(testType: TestTypeImpl): FixturePool {
     if (!this._testTypePools.has(testType)) {
       const fixtures = this._project ? this._applyConfigUseOptions(this._project, testType) : testType.fixtures;
-      const pool = new FixturePool(fixtures);
+      const pool = new FixturePool(fixtures, e => this._onLoadError(e));
       this._testTypePools.set(testType, pool);
     }
     return this._testTypePools.get(testType)!;
   }
 
+  private _onLoadError(e: LoadError): void {
+    if (this._loadErrors)
+      this._loadErrors.push(e);
+    else
+      throw new Error(`${formatLocation(e.location)}: ${e.message}`);
+  }
+
   private _applyConfigUseOptions(project: FullProjectInternal, testType: TestTypeImpl): FixturesWithLocation[] {
     const projectUse = project.use || {};
     const configKeys = new Set(Object.keys(projectUse));

packages/playwright-test/src/reporters/multiplexer.ts

@@ -28,7 +28,6 @@ export class Multiplexer implements Reporter {
   private _reporters: Reporter[];
   private _deferredErrors: TestError[] | null = [];
   private _deferredStdIO: StdIOChunk[] | null = [];
-  hasErrors = false;
   private _config!: FullConfig;
 
   constructor(reporters: Reporter[]) {
@@ -107,8 +106,6 @@ export class Multiplexer implements Reporter {
   }
 
   onError(error: TestError) {
-    this.hasErrors = true;
-
     if (this._deferredErrors) {
       this._deferredErrors.push(error);
       return;

packages/playwright-test/src/runner.ts

@@ -41,13 +41,13 @@ import { Multiplexer } from './reporters/multiplexer';
 import type { TestCase } from './test';
 import { Suite } from './test';
 import type { Config, FullConfigInternal, FullProjectInternal } from './types';
-import { createFileMatcher, createFileMatcherFromFilters, createTitleMatcher, serializeError } from './util';
+import { createFileMatcher, createFileMatcherFromFilters, createTitleMatcher } from './util';
 import type { Matcher, TestFileFilter } from './util';
-import { setFatalErrorSink } from './globals';
 import { buildFileSuiteForProject, filterOnly, filterSuite, filterSuiteWithOnlySemantics, filterTestsRemoveEmptySuites } from './suiteUtils';
 import { LoaderHost } from './loaderHost';
 import { loadTestFilesInProcess } from './testLoader';
 import { TaskRunner } from './taskRunner';
+import type { LoadError } from './fixtures';
 
 const removeFolderAsync = promisify(rimraf);
 const readDirAsync = promisify(fs.readdir);
@@ -240,7 +240,7 @@ export class Runner {
     return filesByProject;
   }
 
-  private async _loadAllTests(options: RunOptions): Promise<{ rootSuite: Suite, testGroups: TestGroup[] }> {
+  private async _loadAllTests(options: RunOptions, errors: TestError[]): Promise<{ rootSuite: Suite, testGroups: TestGroup[] }> {
     const config = this._configLoader.fullConfig();
     const projects = this._collectProjects(options.projectFilter);
     const filesByProject = await this._collectFiles(projects, options.testFileFilters);
@@ -249,10 +249,10 @@ export class Runner {
       files.forEach(file => allTestFiles.add(file));
 
     // Load all tests.
-    const preprocessRoot = await this._loadTests(allTestFiles);
+    const preprocessRoot = await this._loadTests(allTestFiles, errors);
 
     // Complain about duplicate titles.
-    createDuplicateTitlesErrors(config, preprocessRoot).forEach(e => this._reporter.onError(e));
+    errors.push(...createDuplicateTitlesErrors(config, preprocessRoot));
 
     // Filter tests to respect line/column filter.
     filterByFocusedLine(preprocessRoot, options.testFileFilters);
@@ -261,7 +261,7 @@ export class Runner {
     if (config.forbidOnly) {
       const onlyTestsAndSuites = preprocessRoot._getOnlyItems();
       if (onlyTestsAndSuites.length > 0)
-        createForbidOnlyErrors(config, onlyTestsAndSuites).forEach(e => this._reporter.onError(e));
+        errors.push(...createForbidOnlyErrors(config, onlyTestsAndSuites));
     }
 
     // Filter only.
@@ -316,7 +316,7 @@ export class Runner {
     return rootSuite;
   }
 
-  private async _loadTests(testFiles: Set<string>): Promise<Suite> {
+  private async _loadTests(testFiles: Set<string>, errors: TestError[]): Promise<Suite> {
     if (process.env.PW_TEST_OOP_LOADER) {
       const loaderHost = new LoaderHost();
       await loaderHost.start(this._configLoader.serializedConfig());
@@ -326,11 +326,11 @@ export class Runner {
         await loaderHost.stop();
       }
     }
-    const loadErrors: TestError[] = [];
+    const loadErrors: LoadError[] = [];
     try {
       return await loadTestFilesInProcess(this._configLoader.fullConfig(), [...testFiles], loadErrors);
     } finally {
-      loadErrors.forEach(e => this._reporter.onError(e));
+      errors.push(...loadErrors);
     }
   }
 
@@ -390,7 +390,6 @@ export class Runner {
     this._plugins.push(dockerPlugin);
 
     this._reporter = await this._createReporter(options.listOnly);
-    setFatalErrorSink(error => this._reporter.onError(error));
     const taskRunner = new TaskRunner(this._reporter, config.globalTimeout);
 
     // Setup the plugins.
@@ -419,21 +418,14 @@ export class Runner {
 
     // Load tests.
     let loadedTests!: { rootSuite: Suite, testGroups: TestGroup[] };
-    taskRunner.addTask('load tests', async () => {
+    taskRunner.addTask('load tests', async ({ errors }) => {
-      loadedTests = await this._loadAllTests(options);
+      loadedTests = await this._loadAllTests(options, errors);
-      if (this._reporter.hasErrors) {
+      if (errors.length)
-        status = 'failed';
-        taskRunner.stop();
         return;
-      }
 
       // Fail when no tests.
-      if (!loadedTests.rootSuite.allTests().length && !options.passWithNoTests) {
+      if (!loadedTests.rootSuite.allTests().length && !options.passWithNoTests)
-        this._reporter.onError(createNoTestsError());
+        throw new Error(`No tests found`);
-        status = 'failed';
-        taskRunner.stop();
-        return;
-      }
 
       if (!options.listOnly) {
         this._filterForCurrentShard(loadedTests.rootSuite, loadedTests.testGroups);
@@ -444,11 +436,7 @@ export class Runner {
     if (!options.listOnly) {
       taskRunner.addTask('prepare to run', async () => {
         // Remove output directores.
-        if (!await this._removeOutputDirs(options)) {
+        await this._removeOutputDirs(options);
-          status = 'failed';
-          taskRunner.stop();
-          return;
-        }
       });
 
       taskRunner.addTask('plugin begin', async () => {
@@ -509,7 +497,7 @@ export class Runner {
     return status;
   }
 
-  private async _removeOutputDirs(options: RunOptions): Promise<boolean> {
+  private async _removeOutputDirs(options: RunOptions) {
     const config = this._configLoader.fullConfig();
     const outputDirs = new Set<string>();
     for (const p of config.projects) {
@@ -517,23 +505,17 @@ export class Runner {
         outputDirs.add(p.outputDir);
     }
 
-    try {
+    await Promise.all(Array.from(outputDirs).map(outputDir => removeFolderAsync(outputDir).catch(async (error: any) => {
-      await Promise.all(Array.from(outputDirs).map(outputDir => removeFolderAsync(outputDir).catch(async (error: any) => {
+      if ((error as any).code === 'EBUSY') {
-        if ((error as any).code === 'EBUSY') {
+        // We failed to remove folder, might be due to the whole folder being mounted inside a container:
-          // We failed to remove folder, might be due to the whole folder being mounted inside a container:
+        //   https://github.com/microsoft/playwright/issues/12106
-          //   https://github.com/microsoft/playwright/issues/12106
+        // Do a best-effort to remove all files inside of it instead.
-          // Do a best-effort to remove all files inside of it instead.
+        const entries = await readDirAsync(outputDir).catch(e => []);
-          const entries = await readDirAsync(outputDir).catch(e => []);
+        await Promise.all(entries.map(entry => removeFolderAsync(path.join(outputDir, entry))));
-          await Promise.all(entries.map(entry => removeFolderAsync(path.join(outputDir, entry))));
+      } else {
-        } else {
+        throw error;
-          throw error;
+      }
-        }
+    })));
-      })));
-    } catch (e) {
-      this._reporter.onError(serializeError(e));
-      return false;
-    }
-    return true;
   }
 }
 
@@ -807,14 +789,6 @@ function createDuplicateTitlesErrors(config: FullConfigInternal, rootSuite: Suit
   return errors;
 }
 
-function createNoTestsError(): TestError {
-  return createStacklessError(`=================\n no tests found.\n=================`);
-}
-
-function createStacklessError(message: string, location?: TestError['location']): TestError {
-  return { message, location };
-}
-
 function sanitizeConfigForJSON(object: any, visited: Set<any>): any {
   const type = typeof object;
   if (type === 'function' || type === 'symbol')

packages/playwright-test/src/taskRunner.ts

@@ -16,12 +16,12 @@
 
 import { debug } from 'playwright-core/lib/utilsBundle';
 import { ManualPromise, monotonicTime } from 'playwright-core/lib/utils';
-import type { FullResult, Reporter } from '../reporter';
+import type { FullResult, Reporter, TestError } from '../reporter';
 import { SigIntWatcher } from './sigIntWatcher';
 import { serializeError } from './util';
 
 type TaskTeardown = () => Promise<any> | undefined;
-type Task = () => Promise<TaskTeardown | void> | undefined;
+type Task = (params: { errors: TestError[] }) => Promise<TaskTeardown | void> | undefined;
 
 export class TaskRunner {
   private _tasks: { name: string, task: Task }[] = [];
@@ -58,16 +58,22 @@ export class TaskRunner {
           if (this._interrupted)
             break;
           debug('pw:test:task')(`"${name}" started`);
+          const errors: TestError[] = [];
           try {
-            const teardown = await task();
+            const teardown = await task({ errors });
             if (teardown)
               teardownRunner._tasks.unshift({ name: `teardown for ${name}`, task: teardown });
           } catch (e) {
             debug('pw:test:task')(`error in "${name}": `, e);
-            this._reporter.onError?.(serializeError(e));
+            errors.push(serializeError(e));
-            if (!this._isTearDown)
+          } finally {
-              this._interrupted = true;
+            for (const error of errors)
-            this._hasErrors = true;
+              this._reporter.onError?.(error);
+            if (errors.length) {
+              if (!this._isTearDown)
+                this._interrupted = true;
+              this._hasErrors = true;
+            }
           }
           debug('pw:test:task')(`"${name}" finished`);
         }

packages/playwright-test/src/testLoader.ts

@@ -17,6 +17,7 @@
 import path from 'path';
 import type { TestError } from '../reporter';
 import type { FullConfigInternal } from './types';
+import type { LoadError } from './fixtures';
 import { setCurrentlyLoadingFileSuite } from './globals';
 import { PoolBuilder } from './poolBuilder';
 import { Suite } from './test';
@@ -79,7 +80,7 @@ export class TestLoader {
   }
 }
 
-export async function loadTestFilesInProcess(config: FullConfigInternal, testFiles: string[], loadErrors: TestError[]): Promise<Suite> {
+export async function loadTestFilesInProcess(config: FullConfigInternal, testFiles: string[], loadErrors: LoadError[]): Promise<Suite> {
   const testLoader = new TestLoader(config);
   const rootSuite = new Suite('', 'root');
   for (const file of testFiles) {
@@ -87,6 +88,6 @@ export async function loadTestFilesInProcess(config: FullConfigInternal, testFil
     rootSuite._addSuite(fileSuite);
   }
   // Generate hashes.
-  PoolBuilder.buildForLoader(rootSuite);
+  PoolBuilder.buildForLoader(rootSuite, loadErrors);
   return rootSuite;
 }

packages/playwright-test/src/testType.ts

@@ -15,7 +15,7 @@
  */
 
 import { expect } from './expect';
-import { currentlyLoadingFileSuite, currentTestInfo, addFatalError, setCurrentlyLoadingFileSuite } from './globals';
+import { currentlyLoadingFileSuite, currentTestInfo, setCurrentlyLoadingFileSuite } from './globals';
 import { TestCase, Suite } from './test';
 import { wrapFunctionWithLocation } from './transform';
 import type { Fixtures, FixturesWithLocation, Location, TestType } from './types';
@@ -68,15 +68,14 @@ export class TestTypeImpl {
   private _currentSuite(location: Location, title: string): Suite | undefined {
     const suite = currentlyLoadingFileSuite();
     if (!suite) {
-      addFatalError([
+      throw new Error([
         `Playwright Test did not expect ${title} to be called here.`,
         `Most common reasons include:`,
         `- You are calling ${title} in a configuration file.`,
         `- You are calling ${title} in a file that is imported by the configuration file.`,
         `- You have two different versions of @playwright/test. This usually happens`,
         `  when one of the dependencies in your package.json depends on @playwright/test.`,
-      ].join('\n'), location);
+      ].join('\n'));
-      return;
     }
     return suite;
   }
@@ -123,7 +122,7 @@ export class TestTypeImpl {
 
     for (let parent: Suite | undefined = suite; parent; parent = parent.parent) {
       if (parent._parallelMode === 'serial' && child._parallelMode === 'parallel')
-        addFatalError('describe.parallel cannot be nested inside describe.serial', location);
+        throw new Error('describe.parallel cannot be nested inside describe.serial');
     }
 
     setCurrentlyLoadingFileSuite(child);
@@ -152,11 +151,11 @@ export class TestTypeImpl {
 
     if (options.mode !== undefined) {
       if (suite._parallelMode !== 'default')
-        addFatalError('Parallel mode is already assigned for the enclosing scope.', location);
+        throw new Error('Parallel mode is already assigned for the enclosing scope.');
       suite._parallelMode = options.mode;
       for (let parent: Suite | undefined = suite.parent; parent; parent = parent.parent) {
         if (parent._parallelMode === 'serial' && suite._parallelMode === 'parallel')
-          addFatalError('describe.parallel cannot be nested inside describe.serial', location);
+          throw new Error('describe.parallel cannot be nested inside describe.serial');
       }
     }
   }
@@ -182,12 +181,10 @@ export class TestTypeImpl {
     }
 
     const testInfo = currentTestInfo();
-    if (!testInfo) {
+    if (!testInfo)
-      addFatalError(`test.${type}() can only be called inside test, describe block or fixture`, location);
+      throw new Error(`test.${type}() can only be called inside test, describe block or fixture`);
-      return;
-    }
     if (typeof modifierArgs[0] === 'function')
-      addFatalError(`test.${type}() with a function can only be called inside describe block`, location);
+      throw new Error(`test.${type}() with a function can only be called inside describe block`);
     testInfo[type](...modifierArgs as [any, any]);
   }
 
@@ -199,10 +196,8 @@ export class TestTypeImpl {
     }
 
     const testInfo = currentTestInfo();
-    if (!testInfo) {
+    if (!testInfo)
-      addFatalError(`test.setTimeout() can only be called from a test`, location);
+      throw new Error(`test.setTimeout() can only be called from a test`);
-      return;
-    }
     testInfo.setTimeout(timeout);
   }
 
@@ -215,10 +210,8 @@ export class TestTypeImpl {
 
   private async _step<T>(location: Location, title: string, body: () => Promise<T>): Promise<T> {
     const testInfo = currentTestInfo();
-    if (!testInfo) {
+    if (!testInfo)
-      addFatalError(`test.step() can only be called from a test`, location);
+      throw new Error(`test.step() can only be called from a test`);
-      return undefined as any;
-    }
     const step = testInfo._addStep({
       category: 'test.step',
       title,

tests/playwright-test/exit-code.spec.ts

@@ -141,7 +141,7 @@ test('should exit with code 1 if the specified folder does not exist', async ({
     `,
   });
   expect(result.exitCode).toBe(1);
-  expect(result.output).toContain(`no tests found.`);
+  expect(result.output).toContain(`No tests found`);
 });
 
 test('should exit with code 1 if passed a file name', async ({ runInlineTest }) => {
@@ -153,7 +153,7 @@ test('should exit with code 1 if passed a file name', async ({ runInlineTest })
     `,
   });
   expect(result.exitCode).toBe(1);
-  expect(result.output).toContain(`no tests found.`);
+  expect(result.output).toContain(`No tests found`);
 });
 
 test('should exit with code 0 with --pass-with-no-tests', async ({ runInlineTest }) => {

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

@@ -298,7 +298,7 @@ test('should print errors with inconsistent message/stack', async ({ runInlineTe
 test('should print "no tests found" error', async ({ runInlineTest }) => {
   const result = await runInlineTest({ });
   expect(result.exitCode).toBe(1);
-  expect(result.output).toContain('no tests found.');
+  expect(result.output).toContain('No tests found');
 });
 
 test('should not crash on undefined body with manual attachments', async ({ runInlineTest }) => {

tests/playwright-test/reporter.spec.ts

@@ -489,7 +489,7 @@ test('should report no-tests error to reporter', async ({ runInlineTest }) => {
   }, { 'reporter': '' });
 
   expect(result.exitCode).toBe(1);
-  expect(result.output).toContain(`%%got error: =================\n no tests found.`);
+  expect(result.output).toContain(`%%got error: No tests found`);
 });
 
 test('should report require error to reporter', async ({ runInlineTest }) => {