feat: pause on `--debug` #38345

Skn0tt · 2025-11-27T10:29:14Z

No description provided.

Skn0tt · 2025-11-27T10:53:14Z

gotta figure out what to do about result.errors 🤔

tests/playwright-test/max-failures.spec.ts

packages/playwright/src/worker/workerMain.ts

packages/playwright/src/worker/testInfo.ts

dgozman · 2025-11-27T13:13:20Z

packages/playwright/src/worker/testInfo.ts


+  _emitErrors() {
+    const errors = this.errors.slice(this._reportedError);
+    this._reportedError = this.errors.length;


Suggested change

this._reportedError = this.errors.length;

this._reportedError = Math.max(this._reportedError, this.errors.length);

is this just being defensive, or is there a race here that I don't see?

packages/playwright/src/worker/testInfo.ts

dgozman · 2025-11-27T13:22:55Z

packages/playwright/src/runner/dispatcher.ts

+      return;
+    const { result } = data;
+    for (const error of params.errors)
+      addLocationAndSnippetToError(this._config.config, error);


Shouldn't internal reporter do this already, upon reporter.onTestEnd? I am afraid we'll get double snippets. Perhaps this should be handled specifically in onTestPaused?

I moved all result/step snippet adding into dispatcher, that's the cleanest way of doing things I think.

dgozman · 2025-11-27T13:24:16Z

packages/playwright/src/reporters/internalReporter.ts

  }

  onStepBegin(test: TestCase, result: TestResult, step: TestStep) {
+    this._addSnippetToTestErrors(test, result);


Oh, this will definitely produce double snippets. We should figure out this part.

When you say "produce double snippets", you mean that we wastefully compute them twice, right? Because I don't see how we store them multiple times

packages/playwright/src/reporters/teleEmitter.ts

tests/playwright-test/timeout.spec.ts

tests/playwright-test/pause-at-end.spec.ts

dgozman · 2025-12-01T11:33:47Z

packages/playwright/src/reporters/internalReporter.ts

    const codeFrame = codeFrameColumns(source, { start: location }, { highlightCode: true });
    // Convert /var/folders to /private/var/folders on Mac.
-    if (!file || fs.realpathSync(file) !== location.file) {
+    if (!file || fs.realpathSync(file) !== location.file && false) {


Surprising change here!

dgozman · 2025-12-01T11:40:27Z

packages/playwright/src/transform/babelHighlightUtils.ts

+import { traverse, babelParse, T, types as t } from './babelBundle';
+import type { Location } from '../../types/testReporter';
+
+function containsPosition(location: T.SourceLocation, position: Location): boolean {


I don't like that we mix both position and location terms in this file. Let's stick to one.

dgozman · 2025-12-01T11:40:46Z

packages/playwright/src/worker/testInfo.ts

  private _onStepBegin: (payload: StepBeginPayload) => void;
  private _onStepEnd: (payload: StepEndPayload) => void;
  private _onAttach: (payload: AttachmentPayload) => void;
+  private _onErrors: (errors: TestErrorPayload) => void;


TestErrorPayload -> TestErrorsPayload?

dgozman · 2025-12-01T11:41:47Z

packages/playwright/src/worker/testInfo.ts

+      return filteredStackTrace(this.error.stack.split('\n'))[0];
+  }
+
+  async _testEndLocation(): Promise<Location | undefined> {


packages/playwright/src/reporters/internalReporter.ts

github-actions · 2025-12-01T12:22:17Z

Test results for "MCP"

3 failed
❌ [webkit] › mcp/launch.spec.ts:21 › test reopen browser @mcp-ubuntu-latest
❌ [webkit] › mcp/launch.spec.ts:80 › persistent context @mcp-ubuntu-latest
❌ [webkit] › mcp/video.spec.ts:63 › should work with recordVideo (isolated) @mcp-ubuntu-latest

794 passed, 42 skipped

Merge workflow run.

dgozman · 2025-12-01T16:05:01Z

packages/playwright/src/reporters/teleEmitter.ts

  onStepBegin(test: reporterTypes.TestCase, result: reporterTypes.TestResult, step: reporterTypes.TestStep): void {
    (step as any)[this._idSymbol] = createGuid();
+    // This is here to support "test paused" step, where we want to see all errors
+    // at the time of the pause. If we were to have `onTestError()` reporter api, this


Hmm.... We do now have onTestError!

dgozman · 2025-12-01T16:06:17Z

packages/playwright/src/runner/dispatcher.ts

+    result.errors.push(...params.errors);
+    result.error = result.errors[0];
+    for (const error of result.errors)
+      this._reporter.onTestError?.(test, result, error);


We should not notify about the same error from result.errors multiple times.

It is better for consistency to maintain that result.errors[result.errors.length - 1] === newError inside onTestError().

Suggested change

result.errors.push(...params.errors);

result.error = result.errors[0];

for (const error of result.errors)

this._reporter.onTestError?.(test, result, error);

for (const error of params.errors) {

result.errors.push(error);

result.error = result.errors[0];

this._reporter.onTestError?.(test, result, error);

}

dgozman · 2025-12-01T16:06:58Z

packages/playwright/src/runner/dispatcher.ts

+    result.errors.push(...errors);
    result.error = result.errors[0];
+    for (const error of result.errors)
+      this._reporter.onTestError?.(test, result, error);


Suggested change

result.errors.push(...errors);

result.error = result.errors[0];

for (const error of result.errors)

this._reporter.onTestError?.(test, result, error);

for (const error of errors) {

result.errors.push(error);

result.error = result.errors[0];

this._reporter.onTestError?.(test, result, error);

}

packages/playwright/src/transform/babelHighlightUtils.ts

github-actions · 2025-12-02T13:42:36Z

Test results for "tests 1"

6 flaky

⚠️ [firefox-library] › library/inspector/cli-codegen-1.spec.ts:1079 › cli codegen › should not throw csp directive violation errors `@firefox-ubuntu-22.04-node18`
⚠️ [firefox-page] › page/page-event-request.spec.ts:182 › should return response body when Cross-Origin-Opener-Policy is set `@firefox-ubuntu-22.04-node18`
⚠️ [firefox-page] › page/page-wait-for-function.spec.ts:104 › should work with strict CSP policy `@firefox-ubuntu-22.04-node18`
⚠️ [webkit-library] › library/proxy.spec.ts:93 › should proxy local network requests › with other bypasses › loopback address `@webkit-ubuntu-22.04-node18`
⚠️ [webkit-library] › library/proxy.spec.ts:146 › should work with authenticate followed by redirect `@webkit-ubuntu-22.04-node18`
⚠️ [playwright-test] › ui-mode-test-watch.spec.ts:354 › should watch test defined outside of .spec.ts file `@windows-latest-node18-2`

40350 passed, 790 skipped

Merge workflow run.

Skn0tt added 3 commits November 27, 2025 11:09

feat: --debug pauses test

892e2ce

location

bcf2fec

proper step location

c40f102

Skn0tt requested a review from dgozman November 27, 2025 10:29

Skn0tt self-assigned this Nov 27, 2025

Skn0tt marked this pull request as draft November 27, 2025 10:53

This comment has been minimized.

Sign in to view

progressively report errors

4b87e41

This comment has been minimized.

Sign in to view

tele

b26031f

Skn0tt marked this pull request as ready for review November 27, 2025 11:23

Skn0tt added 3 commits November 27, 2025 12:24

skip on windows

c1c9867

reporting more errors

b515837

errors are optional

0a611bb

This comment has been minimized.

Sign in to view

mark optional for backwards compat

db2419d

This comment has been minimized.

Sign in to view

extract into TestErrorPayload ipc

0996534

This comment has been minimized.

Sign in to view

Skn0tt added 2 commits November 27, 2025 13:52

mirror attachments

8381924

typescript

d075147

dgozman reviewed Nov 27, 2025

View reviewed changes

Skn0tt added 5 commits November 27, 2025 14:32

unflake

7f90a0f

style

2242b4c

more feedback

6061a29

feedback

bda2662

add snippets in dispatcher

3999df2

Skn0tt requested a review from dgozman November 27, 2025 13:54

Skn0tt added 2 commits November 27, 2025 15:00

merge

807eafe

pass file when possible

c420d97

Skn0tt mentioned this pull request Nov 28, 2025

feat: --pause #38338

Closed

add comment

c8bd245

dgozman reviewed Dec 1, 2025

View reviewed changes

address

dea4b7d

This comment has been minimized.

Sign in to view

rename

22d0744

Skn0tt requested a review from dgozman December 1, 2025 15:43

dgozman reviewed Dec 1, 2025

View reviewed changes

Skn0tt added 5 commits December 2, 2025 08:20

rename

dab17dd

maintain last error index

7bf8763

use onTestError

5817b37

remove unused map

f768a69

revert linechange

7020ccd

Skn0tt requested a review from dgozman December 2, 2025 07:36

dgozman approved these changes Dec 2, 2025

View reviewed changes

the multiplexer!

fa8fa14

Skn0tt merged commit 7199022 into microsoft:main Dec 2, 2025
33 of 34 checks passed

	this._reportedError = this.errors.length;
	this._reportedError = Math.max(this._reportedError, this.errors.length);

feat: pause on --debug #38345

feat: pause on --debug #38345

Conversation

Skn0tt commented Nov 27, 2025

Uh oh!

Skn0tt commented Nov 27, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

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!

Uh oh!

github-actions bot commented Dec 1, 2025

Test results for "MCP"

Uh oh!

This comment has been minimized.

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!

Uh oh!

github-actions bot commented Dec 2, 2025

Test results for "tests 1"

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

feat: pause on `--debug` #38345

feat: pause on `--debug` #38345