feat: `--debug=end` #36153

Skn0tt · 2025-06-02T08:31:26Z

Adds a --debug=end mode that pauses a test run just before the teardown of the very last test, so that the developer can inspect the test environment. Useful for debugging a failed test, or for interactively writing a test that depends on resources managed by the test runner.

There's a couple of missing pieces that I left out of this PR for focus:

TeleEmitter support
errors in delayed teardown aren't reported yet
Inspector doesn't show the right location

The aspect of this PR that i'm most sceptical about is around _maybeDebugAtEnd. To decide if it should pause, the worker needs to know if there's another test coming after it. The worker doesn't know that though, this knowledge lives in the scheduler on the host. So I had to rebuild it. It feels brittle. The alternative would mean that the worker and scheduler need to synchronize though, and that feels like a higher level of complexity.

🚨 don't merge before 1.53 branch point 🚨

Copilot

Pull Request Overview

Adds a new --debug=end mode that pauses just before tearing down the last test run, letting developers inspect the environment interactively.

Propagate an isLastTest flag from the scheduler into the worker, and use it to trigger a pause at test end
Implement pause/resume logic in TestInfoImpl, adjust test duration to exclude paused time
Expose a new onTestPaused reporter hook and update CLI parsing/documentation for debug modes

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
workerMain.ts	Pass full `TestEntry` (with `isLastTest`) into `_runTest` and tweak duration calculation
testInfo.ts	Add `_maybeDebugAtEnd`, `_pause`/`_resume` logic, track paused time
failureTracker.ts	Introduce `nextFailureReachesMaxFailures` helper
dispatcher.ts	Include `isLastTest` in `runInWorker`, dispatch `testPaused` events
reporterV2.ts	Declare new optional `onTestPaused` method
multiplexer.ts	Forward `onTestPaused` to all registered reporters
internalReporter.ts	Forward `onTestPaused` to the internal reporter
base.ts (TerminalReporter)	Implement `onTestPaused` to skip epilogue and show debug message
program.ts	Disallow `--debug=end` in UI/watch modes; refine `--debug` option parsing
index.ts (fixtures)	Hook page fixture into debug-end mode to pause/resume browser page
common/ipc.ts	Update IPC types: `debug` is now `"begin"
common/config.ts	Adjust default overrides when `debug === 'begin'`
docs/src/test-reporter-api/class-reporter.md	Document the new `Reporter.onTestPaused` hook

Comments suppressed due to low confidence (2)

packages/playwright/src/worker/testInfo.ts:76

[nitpick] The field name _retries is ambiguous; consider renaming it to _maxRetries to clarify that it represents the maximum retry count.

private readonly _retries: number;

docs/src/test-reporter-api/class-reporter.md:272

[nitpick] Parameter bullets under Reporter.onTestPaused need consistent indentation (two spaces) for proper markdown rendering.

- `test` <[TestCase]>

Copilot · 2025-06-02T08:33:46Z

packages/playwright/src/runner/dispatcher.ts

@@ -306,7 +306,17 @@ class JobDispatcher {
    this._currentlyRunning = { test, result };
  }

-  private _onTestEnd(params: TestEndPayload) {
+  private _onTestPaused(params: TestEndPayload) {


[nitpick] Both _onTestPaused and _onTestEnd call _updateTest; consider extracting the shared update logic into a single helper to reduce duplication.

Copilot · 2025-06-02T08:33:46Z

packages/playwright/src/reporters/base.ts

+    void this.onEnd(fakeEnd).then(() => {
+      this._skipEpilogue = true;


Setting _skipEpilogue inside the .then() callback may introduce a race; consider assigning _skipEpilogue = true before calling onEnd to ensure the epilogue is always skipped.

Suggested change

void this.onEnd(fakeEnd).then(() => {

this._skipEpilogue = true;

this._skipEpilogue = true;

void this.onEnd(fakeEnd).then(() => {

Copilot · 2025-06-02T08:33:46Z

packages/playwright/src/program.ts

@@ -173,6 +173,8 @@ async function runTests(args: string[], opts: { [key: string]: any }) {
  if (opts.ui || opts.uiHost || opts.uiPort) {
    if (opts.onlyChanged)
      throw new Error(`--only-changed is not supported in UI mode. If you'd like that to change, see https://github.com/microsoft/playwright/issues/15075 for more details.`);
+    if (cliOverrides.debug === 'end')
+      throw new Error(`--debug=end is not supported in UI mode. If you'd like that to change, file an issue and let us know about your usecase for it.`);


Typo in error message: "usecase" should be two words ("use case") for correct spelling.

Suggested change

throw new Error(`--debug=end is not supported in UI mode. If you'd like that to change, file an issue and let us know about your usecase for it.`);

throw new Error(`--debug=end is not supported in UI mode. If you'd like that to change, file an issue and let us know about your use case for it.`);

docs/src/test-reporter-api/class-reporter.md

packages/playwright/src/common/config.ts

packages/playwright/src/program.ts

dgozman · 2025-06-02T08:51:19Z

packages/playwright/src/program.ts

-    process.env.PWDEBUG = '1';
+    if (!['begin', 'end'].includes(options.debug))
+      throw new Error(`Unsupported debug mode "${options.debug}", must be one of "begin" or "end"`);
+    overrides.debug = options.debug;


I guess I'm missing the place where --debug without a value is converted to begin.

dgozman · 2025-06-02T09:16:28Z

packages/playwright/src/worker/testInfo.ts

@@ -413,6 +424,54 @@ export class TestInfoImpl implements TestInfo {
    this._timeoutManager.setIgnoreTimeouts();
  }

+  async _pause() {


I'd rather move all this code to WorkerMain. It handles the test flow, while TestInfo has utility functions.

i'll try doing this as part of the onDidTestFunctionFinish change.

dgozman · 2025-06-02T09:17:55Z

packages/playwright/src/worker/workerMain.ts

@@ -473,7 +476,7 @@ export class WorkerMain extends ProcessRunner {
      await testInfo._tracing.stopIfNeeded();
    }).catch(() => {});  // Ignore the top-level error, it is already inside TestInfo.errors.

-    testInfo.duration = (testInfo._timeoutManager.defaultSlot().elapsed + afterHooksSlot.elapsed) | 0;
+    testInfo.duration = (testInfo._timeoutManager.defaultSlot().elapsed - testInfo._pausedSlot.elapsed + afterHooksSlot.elapsed) | 0;


This is fancy, but I'm not sure anyone would care about the duration of a paused test 😄

It reads almost like pure maths ^^ So should we take it into account or not?

dgozman · 2025-06-02T09:18:48Z

packages/playwright/src/worker/workerMain.ts

@@ -380,6 +381,8 @@ export class WorkerMain extends ProcessRunner {
    // No skips in after hooks.
    testInfo._allowSkips = true;

+    await testInfo._maybeDebugAtEnd(entry.isLastTest);


It seems like this is close to running testInfo._onDidFinishTestFunction? Perhaps combine the two, or at least run them in the same place.

dgozman · 2025-06-02T09:23:22Z

packages/playwright/src/index.ts

+
+    const disposables: Disposable[] = [];
+    if (testInfoImpl._configInternal.configCLIOverrides.debug === 'end') {
+      testInfoImpl._onPaused(location => {


Again, perhaps reusing "onTestFunctionFinished" callback will make things better here.

Agreed. onTestFunctionFinished is currently controlled by ArtifactRecorder though, I'll see how big of a change this is to untangle.

dgozman · 2025-06-02T09:24:36Z

packages/playwright/src/worker/testInfo.ts


 import { TimeoutManager, TimeoutManagerError, kMaxDeadline } from './timeoutManager';
 import { addSuffixToFilePath, filteredStackTrace, getContainedPath, normalizeAndSaveAttachment, sanitizeFilePathBeforeExtension, trimLongString, windowsFilesystemFriendlyLength } from '../util';
 import { TestTracing } from './testTracing';
 import { testInfoError } from './util';
+import { EventEmitter } from '../isomorphic/events';


We are in Node here, let's use Node's EventEmitter.

Co-authored-by: Dmitry Gozman <dgozman@gmail.com> Signed-off-by: Simon Knott <info@simonknott.de>

Skn0tt

Thanks for the good feedback! I've addressed most. Gonna look at _onDidFinishTestFunction now.

Skn0tt · 2025-06-02T11:44:47Z

docs/src/test-reporter-api/class-reporter.md

@@ -262,6 +262,22 @@ Test that has been started.

 Result of the test run, this object gets populated while the test runs.

+## optional method: Reporter.onTestPaused


Makes sense. I've added a naïve implementation.

Skn0tt · 2025-06-02T11:47:59Z

packages/playwright/src/index.ts

+
+    const disposables: Disposable[] = [];
+    if (testInfoImpl._configInternal.configCLIOverrides.debug === 'end') {
+      testInfoImpl._onPaused(location => {


Agreed. onTestFunctionFinished is currently controlled by ArtifactRecorder though, I'll see how big of a change this is to untangle.

Skn0tt · 2025-06-02T11:48:05Z

packages/playwright/src/program.ts

-    process.env.PWDEBUG = '1';
+    if (!['begin', 'end'].includes(options.debug))
+      throw new Error(`Unsupported debug mode "${options.debug}", must be one of "begin" or "end"`);
+    overrides.debug = options.debug;


Skn0tt · 2025-06-02T11:48:46Z

packages/playwright/src/worker/testInfo.ts

@@ -413,6 +424,54 @@ export class TestInfoImpl implements TestInfo {
    this._timeoutManager.setIgnoreTimeouts();
  }

+  async _pause() {


i'll try doing this as part of the onDidTestFunctionFinish change.

Skn0tt · 2025-06-02T11:49:14Z

packages/playwright/src/worker/workerMain.ts

@@ -473,7 +476,7 @@ export class WorkerMain extends ProcessRunner {
      await testInfo._tracing.stopIfNeeded();
    }).catch(() => {});  // Ignore the top-level error, it is already inside TestInfo.errors.

-    testInfo.duration = (testInfo._timeoutManager.defaultSlot().elapsed + afterHooksSlot.elapsed) | 0;
+    testInfo.duration = (testInfo._timeoutManager.defaultSlot().elapsed - testInfo._pausedSlot.elapsed + afterHooksSlot.elapsed) | 0;


It reads almost like pure maths ^^ So should we take it into account or not?

packages/playwright/src/runner/dispatcher.ts

Skn0tt · 2025-06-02T11:52:54Z

packages/playwright/src/runner/dispatcher.ts

-        return { testId: test.id, retry: test.results.length };
+      entries: this.job.tests.map((test, index) => {
+        let isLastTest: boolean | 'if-failure' = isEnd && index === this.job.tests.length - 1;
+        if (!isLastTest && this._failureTracker.hasReachedMaxFailures())


Let's remove interaction with maxFailures

done

It seems like when running with --debug=end, we should always stop at the first test failure if that ever happens?

I don't think so. In CLI usage, I expect users to focus on a single test, so this doesn't matter. But when used in VS Code, it's important we run the same tests as without the flag, and always pause the last test. So we shouldn't stop based on first failure. Maybe VS Code needs a slightly different mode?

Skn0tt · 2025-06-02T11:58:51Z

packages/playwright/src/worker/testInfo.ts

+    await this._pause();
+  }
+
+  _willBeRunAgain() {


I moved the retry logic into Dispatcher. I think we should take it into account so we don't change execution order for VS Code. CLI usage is probably different - starts feeling like two separate modes emerge.

Skn0tt

Alright, I migrated to _onDidTestFunctionEnd. That didn't excatly make the code cleaner because _onDidTestFunctionEnd is only used in ArtifactsRecorder. I added a second _onTestPaused function that's similar though - much easier to follow.

github-actions · 2025-06-02T13:53:46Z

Test results for "tests 1"

10 flaky

⚠️ [chromium-library] › library/browsercontext-reuse.spec.ts:256:1 › should work with clock emulation @chromium-ubuntu-22.04-node18
⚠️ [chromium-library] › library/browsercontext-reuse.spec.ts:256:1 › should work with clock emulation @chromium-ubuntu-22.04-node20
⚠️ [chromium-library] › library/browsercontext-reuse.spec.ts:256:1 › should work with clock emulation @chromium-ubuntu-22.04-node22
⚠️ [firefox-library] › library/inspector/cli-codegen-1.spec.ts:986:7 › cli codegen › should not throw csp directive violation errors @firefox-ubuntu-22.04-node18
⚠️ [firefox-page] › page/page-wait-for-function.spec.ts:104:3 › should work with strict CSP policy @firefox-ubuntu-22.04-node18
⚠️ [chromium-library] › library/browsercontext-reuse.spec.ts:256:1 › should work with clock emulation @ubuntu-22.04-chromium-tip-of-tree
⚠️ [webkit-library] › library/browsercontext-device.spec.ts:45:5 › device › should scroll to click @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › library/ignorehttpserrors.spec.ts:30:3 › should isolate contexts @webkit-ubuntu-22.04-node18
⚠️ [webkit-page] › page/page-screenshot.spec.ts:345:5 › page screenshot › should work while navigating @webkit-ubuntu-22.04-node18
⚠️ [playwright-test] › ui-mode-test-watch.spec.ts:145:5 › should watch all @windows-latest-node18-1

39274 passed, 818 skipped
✔️✔️✔️

Merge workflow run.

Skn0tt added 14 commits May 30, 2025 15:50

pause method

eda03e2

subscribe page.pause()

9492e10

debug location

a5d8867

dispatcher

b5a4d83

provide location

a75d5ab

pause

2230860

reporter

86859e5

pause at end

df319e5

put behind flag

092a09a

forbid usage with ui + watch mode

41c1799

revert page

58b8e98

condition on debug==end

933032c

safer skipping

ce32811

_isPaused()

f0b4250

Skn0tt requested review from dgozman and Copilot June 2, 2025 08:31

Skn0tt self-assigned this Jun 2, 2025

Copilot AI reviewed Jun 2, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

dgozman reviewed Jun 2, 2025

View reviewed changes

Skn0tt and others added 10 commits June 2, 2025 12:45

Update docs/src/test-reporter-api/class-reporter.md

aa8e9e5

Co-authored-by: Dmitry Gozman <dgozman@gmail.com> Signed-off-by: Simon Knott <info@simonknott.de>

move after "onDidFinishTestFunction"

2e14e1c

globalTimeout 0

a5d5cdc

rename

8f25863

move retry check into dispatcher

26cd2ca

consider phase

d14bd9f

default to begin

d3f74f2

throw

7f0e652

shorten cli help

8f4b6a1

pass data

afb053b

Skn0tt added 3 commits June 2, 2025 13:17

typo

c2e27ec

forbid max-failures

a1a78d3

move _pause() call into worker

8671246

This comment has been minimized.

Sign in to view

Skn0tt added 3 commits June 2, 2025 13:38

fix phases calc

d4626a4

implement teleemitter

bab8168

take retries into account

feaf67f

Skn0tt commented Jun 2, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

Skn0tt added 2 commits June 2, 2025 14:47

move into _onDidFinishTestFunction

ccba771

revert page

1065389

This comment has been minimized.

Sign in to view

Skn0tt added 2 commits June 2, 2025 14:58

second

026aa2f

remove remnants

f64f2f8

This comment has been minimized.

Sign in to view

Skn0tt commented Jun 2, 2025

View reviewed changes

Skn0tt requested a review from dgozman June 2, 2025 13:12

Skn0tt mentioned this pull request Jun 2, 2025

chore: page.pause(location) #36161

Closed

Skn0tt closed this Jun 5, 2025

		void this.onEnd(fakeEnd).then(() => {
		this._skipEpilogue = true;

	throw new Error(`--debug=end is not supported in UI mode. If you'd like that to change, file an issue and let us know about your usecase for it.`);
	throw new Error(`--debug=end is not supported in UI mode. If you'd like that to change, file an issue and let us know about your use case for it.`);

		@@ -262,6 +262,22 @@ Test that has been started.

		Result of the test run, this object gets populated while the test runs.

		## optional method: Reporter.onTestPaused

feat: --debug=end #36153

feat: --debug=end #36153

Uh oh!

Conversation

Skn0tt commented Jun 2, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

Skn0tt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Skn0tt Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Skn0tt left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jun 2, 2025

Test results for "tests 1"

Uh oh!

Uh oh!

feat: `--debug=end` #36153

feat: `--debug=end` #36153

Skn0tt Jun 2, 2025 •

edited

Loading