chore: move Copy Prompt rendering to UI #36089

Skn0tt · 2025-05-26T11:20:15Z

Trims down the error-context attachment to the actual context bit, which is only the page snapshot. All other parts are dropped.
The error context can be rendered into a "Copy Prompt" prompt with the copyPrompt function. This now lives in packages/web and is shared by Trace Viewer and HTML report.

Here's how we change behaviour:

"Error Context" file linked in terminal reporters doesn't contain error info anymore. This is a good thing - a consuming agent already knows all of that from the terminal report.
"Copy Prompt" in HTML report doesn't contain # Test source section when opened on remote server anymore, because we don't attach the sources and it can only get it via the local server
Trace Viewer shows slightly different formatting for # Test Info section

The VS Code extension can take the error-context attachment and blindly tack it onto the error message.

github-actions · 2025-05-27T13:30:52Z

Test results for "tests 1"

4 failed
❌ [default] › run-tests.spec.ts:1305:5 › should provide page snapshot to copilot @vscode-extension
❌ [default-reuse] › run-tests.spec.ts:1305:5 › should provide page snapshot to copilot @vscode-extension
❌ [default-trace] › run-tests.spec.ts:1305:5 › should provide page snapshot to copilot @vscode-extension
❌ [webkit-library] › library/video.spec.ts:475:5 › screencast › should scale frames down to the requested size @webkit-ubuntu-22.04-node18

7 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
⚠️ [chromium-library] › library/browsercontext-reuse.spec.ts:256:1 › should work with clock emulation @ubuntu-22.04-chromium-tip-of-tree
⚠️ [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

39248 passed, 808 skipped
✔️✔️✔️

Merge workflow run.

dgozman

I feel like this PR is too large to be reviewed effectively. Could you please split it up?

dgozman · 2025-05-28T09:14:19Z

packages/web/package.json

@@ -4,8 +4,12 @@
  "version": "0.0.0",
  "scripts": {},
  "dependencies": {
+    "@babel/code-frame": "^7.27.1",


So, we are brining babel into our web properties. How large is this one? Does it run in the browser?

dgozman · 2025-05-28T09:14:54Z

package-lock.json

-      "version": "7.26.2",
-      "resolved": "https://registry.npmjs.org/@babel/code-frame/-/code-frame-7.26.2.tgz",
-      "integrity": "sha512-RJlIHRueQgwWitWgF8OdFYGZX328Ax5BCemNGlqHfplnRT9ESi8JkFlvaVYbS+UubVY6dpv87Fs2u5M29iNFVQ==",
+      "version": "7.27.1",


Can we stick with the old version we already had to avoid unnecessary deps churn?

dgozman · 2025-05-28T09:18:28Z

tests/playwright-test/playwright.connect.spec.ts

@@ -223,7 +223,6 @@ test('should record trace', async ({ runInlineTest }) => {
    'After Hooks',
    'Fixture "page"',
    'Fixture "context"',
-    'Attach "error-context-0"',


What happened here?

dgozman · 2025-05-28T09:19:01Z

tests/playwright-test/playwright.trace.spec.ts

@@ -992,6 +991,7 @@ test('should record nested steps, even after timeout', async ({ runInlineTest },
    '      Expect "barPage teardown"',
    '      Step "step in barPage teardown"',
    '        Close context',
+    'Attach "error-context"',


I am missing where all the changes to the steps are coming from.

dgozman · 2025-05-28T09:19:56Z

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

@@ -358,9 +358,6 @@ test('should report parallelIndex', async ({ runInlineTest }, testInfo) => {

 test('attaches error context', async ({ runInlineTest }) => {


Not sure this test brings any value. Why is it in reporter-json of all things?

dgozman · 2025-05-28T09:24:23Z

packages/html-reporter/src/testResultView.tsx

+            metadata,
+            errorContextContent,
+            async file => {
+              const response = await fetch('trace/file?' + new URLSearchParams({ path: `${rootDir}/${file}` }));


I'd prefer that html report does not know about rootDir or anything filesystem-related. Can we instead fetch sources while building the report, and include them explicitly? This way, you can also open the report from anywhere at any time, and things will work.

dgozman · 2025-05-28T09:26:04Z

packages/html-reporter/src/testErrorView.tsx

-    </CodeSnippet>
-  );
+}> = ({ error, testId }) => {
+  return <CodeSnippet code={error} testId={testId}/>;


Do we even need this component now? Even better, inline CodeSnippet here?

dgozman · 2025-05-28T09:27:56Z

packages/html-reporter/src/testResultView.tsx

    const otherAttachments = new Set<TestAttachment>(attachments);
    [...screenshots, ...videos, ...traces].forEach(a => otherAttachments.delete(a));
+    if (errorContext)
+      otherAttachments.delete(errorContext);


Is this to hide the error-context from the list of attachments I can download? Why do that?

dgozman · 2025-05-28T09:30:19Z

packages/html-reporter/src/testResultView.tsx

  }, [result]);

+  const prompt = useAsyncMemo(
+      async () => {
+        const errorContextContent = errorContext?.path ? await fetch(errorContext.path).then(r => r.text()) : errorContext?.body;


Why is this using fetch() instead of LoadedReport.entry()?

dgozman · 2025-05-28T09:33:47Z

packages/trace-viewer/src/ui/workbench.tsx

@@ -56,7 +57,8 @@ export const Workbench: React.FunctionComponent<{
  inert?: boolean,
  onOpenExternally?: (location: modelUtil.SourceLocation) => void,
  revealSource?: boolean,
-}> = ({ model, showSourcesFirst, rootDir, fallbackLocation, isLive, hideTimeline, status, annotations, inert, onOpenExternally, revealSource }) => {
+  metadata?: MetadataWithCommitInfo,


Let's call this testRunMetadata to differentiate from "Metadata" tab in the trace viewer.

Skn0tt · 2025-05-28T10:58:02Z

Splitting it up!

Skn0tt added 5 commits May 26, 2025 10:32

trim down error-context

c3f5dc0

adapt trace viewer

63340d1

adapt html report

494f439

fix files

073c7dc

delete old

f91ec3c

Skn0tt self-assigned this May 26, 2025

fix component test

e371bbb

This comment has been minimized.

Sign in to view

import type

36ace1c

This comment has been minimized.

Sign in to view

remove

518fe7a

This comment has been minimized.

Sign in to view

Skn0tt added 2 commits May 27, 2025 14:38

remove _sourceCache

60607ad

tests, remove testserver option

3f71d9d

Skn0tt requested a review from dgozman May 28, 2025 08:45

dgozman reviewed May 28, 2025

View reviewed changes

This was referenced May 28, 2025

chore: remove error context JSON format #36116

Merged

chore: trim error context and make it singular #36117

Merged

Skn0tt closed this May 28, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore: move Copy Prompt rendering to UI #36089

chore: move Copy Prompt rendering to UI #36089

Skn0tt commented May 26, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented May 27, 2025

Uh oh!

dgozman left a comment

Uh oh!

dgozman May 28, 2025

Uh oh!

dgozman May 28, 2025

Uh oh!

dgozman May 28, 2025

Uh oh!

dgozman May 28, 2025

Uh oh!

dgozman May 28, 2025

Uh oh!

dgozman May 28, 2025

Uh oh!

dgozman May 28, 2025

Uh oh!

dgozman May 28, 2025

Uh oh!

dgozman May 28, 2025

Uh oh!

dgozman May 28, 2025

Uh oh!

Skn0tt commented May 28, 2025

Uh oh!

Uh oh!

		@@ -358,9 +358,6 @@ test('should report parallelIndex', async ({ runInlineTest }, testInfo) => {

		test('attaches error context', async ({ runInlineTest }) => {

chore: move Copy Prompt rendering to UI #36089

chore: move Copy Prompt rendering to UI #36089

Conversation

Skn0tt commented May 26, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented May 27, 2025

Test results for "tests 1"

Uh oh!

dgozman 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!

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!

Skn0tt commented May 28, 2025

Uh oh!

Uh oh!