[Bug]: `test.fail` and `addLocatorHandler` causes other test cases to be skipped

[datwaft]

Version

1.50.1

Steps to reproduce

The reproduction example is in https://github.com/datwaft/playwright-skipping-tests-on-test.fail but the important part is the following:

import { expect, test } from "@playwright/test";

test.describe.configure({ mode: "parallel" });

test.beforeEach(async ({ page }, testInfo) => {
  await page.goto("https://playwright.dev/");

  await page.addLocatorHandler(
    page.getByRole("heading", { name: "Installation", level: 1 }),
    async () => {
      console.log(`found in ${testInfo.parallelIndex} thread!`);
      throw new Error("we fail if the installation page is found :(");
    },
  );
});

for (let i = 0; i < 10; ++i) {
  test(`#${i}`, async ({ page }) => {
    test.fail(i === 3, "expected because 3 is evil >:(");
    if (i === 3) {
      await page.getByRole("link", { name: "Get Started" }).click();
      await expect(
        page.getByRole("link", { name: "How to install Playwright" }),
      ).toBeVisible();
    } else {
      await expect(
        page.getByRole("link", { name: "Get Started" }),
      ).toBeVisible();
    }
    await page.waitForTimeout(1000);
  });
}

You can run the reproduction example with:

npm install
npx playwright test

Expected behavior

Like when not using addLocatorHandler the tests after the one that should fail should not be skipped.

$ npx playwright test

Running 10 tests using 5 workers

  ✓  1 [chromium] › tests/proof-of-concept.spec.ts:18:7 › #4 (2.4s)
  ✓  2 [chromium] › tests/proof-of-concept.spec.ts:18:7 › #2 (2.4s)
  ✓  3 [chromium] › tests/proof-of-concept.spec.ts:18:7 › #0 (2.4s)
  ✓  4 [chromium] › tests/proof-of-concept.spec.ts:18:7 › #1 (2.4s)
  ✘  5 [chromium] › tests/proof-of-concept.spec.ts:18:7 › #3 (1.6s)
found in 3 thread!
  ✓  6 [chromium] › tests/proof-of-concept.spec.ts:18:7 › #5 (2.4s)
  ✓  7 [chromium] › tests/proof-of-concept.spec.ts:18:7 › #6 (2.4s)
  ✓  8 [chromium] › tests/proof-of-concept.spec.ts:18:7 › #7 (2.4s)
  ✓  9 [chromium] › tests/proof-of-concept.spec.ts:18:7 › #8 (2.4s)
  ✓  10 [chromium] › tests/proof-of-concept.spec.ts:18:7 › #9 (2.4s)

  10 passed (6.4s)

Actual behavior

$ npx playwright test

Running 10 tests using 5 workers

  ✓  1 [chromium] › tests/proof-of-concept.spec.ts:18:7 › #4 (2.4s)
  ✓  2 [chromium] › tests/proof-of-concept.spec.ts:18:7 › #2 (2.4s)
  ✓  3 [chromium] › tests/proof-of-concept.spec.ts:18:7 › #0 (2.4s)
  ✓  4 [chromium] › tests/proof-of-concept.spec.ts:18:7 › #1 (2.4s)
  ✘  5 [chromium] › tests/proof-of-concept.spec.ts:18:7 › #3 (1.6s)
found in 3 thread!
  -  6 [chromium] › tests/proof-of-concept.spec.ts:18:7 › #5
  -  7 [chromium] › tests/proof-of-concept.spec.ts:18:7 › #6
  -  8 [chromium] › tests/proof-of-concept.spec.ts:18:7 › #7
  -  9 [chromium] › tests/proof-of-concept.spec.ts:18:7 › #8
  -  10 [chromium] › tests/proof-of-concept.spec.ts:18:7 › #9

  5 did not run
  5 passed (3.2s)

Additional context

I found a related bug in #26435 but the reproduction is different.

Environment

System:
    OS: macOS 15.3
    CPU: (10) arm64 Apple M1 Pro
    Memory: 433.52 MB / 16.00 GB
  Binaries:
    Node: 22.12.0 - ~/.local/share/mise/installs/node/22.12.0/bin/node
    npm: 10.9.0 - ~/.local/share/mise/installs/node/22.12.0/bin/npm
    pnpm: 10.3.0 - ~/.local/share/mise/installs/node/22.12.0/bin/pnpm
  Languages:
    Bash: 5.2.37 - /opt/homebrew/bin/bash
  npmPackages:
    @playwright/test: ^1.50.1 => 1.50.

[Skn0tt]

Can reproduce, thanks. It seems to be caused by a combination of throwing an exception in the locator handler and parallel mode.

[Skn0tt]

It can be reduced to this:

test.describe.configure({ mode: 'parallel' });

let throwUncaughtException;
new Promise((resolve, reject) => {
  throwUncaughtException = reject;
});

for (let i = 0; i < 10; ++i) {
  test(`#${i}`, async ({ page }) => {
    test.fail(i === 3, 'expected because 3 is evil >:(');
    if (i === 3) {
      throwUncaughtException(new Error('we should not reach this point'));
    } else {
      expect(1).toBe(1);
      await page.waitForTimeout(1000);
    }
  });
}

addLocatorHandler throws an uncaught exception, and that kills the worker. That skips all following tests. We should either prevent addLocatorHandler from killing the worker, or ensure that uncaught exceptions don't unnecessarily skip tests.

[dgozman]

Over to Adam Gastineau (@agg23) to issue a warning for running locator handlers at the end of a test.