Skip to content

Commit 78fa911

Browse files
authored
chore: improve prompt to use code frame and inline error (#35032)
1 parent d6a4c1c commit 78fa911

File tree

8 files changed

+172
-96
lines changed

8 files changed

+172
-96
lines changed

examples/todomvc/playwright.config.ts

+2
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ export default defineConfig({
1212
/* Maximum time one test can run for. */
1313
timeout: 15_000,
1414

15+
captureGitInfo: { commit: true, diff: true },
16+
1517
expect: {
1618

1719
/**

packages/playwright-core/src/utils/isomorphic/stackTrace.ts

+28
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,34 @@ export function splitErrorMessage(message: string): { name: string, message: str
134134
};
135135
}
136136

137+
export function parseErrorStack(stack: string, pathSeparator: string, showInternalStackFrames: boolean = false): {
138+
message: string;
139+
stackLines: string[];
140+
location?: StackFrame;
141+
} {
142+
const lines = stack.split('\n');
143+
let firstStackLine = lines.findIndex(line => line.startsWith(' at '));
144+
if (firstStackLine === -1)
145+
firstStackLine = lines.length;
146+
const message = lines.slice(0, firstStackLine).join('\n');
147+
const stackLines = lines.slice(firstStackLine);
148+
let location: StackFrame | undefined;
149+
for (const line of stackLines) {
150+
const frame = parseStackFrame(line, pathSeparator, showInternalStackFrames);
151+
if (!frame || !frame.file)
152+
continue;
153+
if (belongsToNodeModules(frame.file, pathSeparator))
154+
continue;
155+
location = { file: frame.file, column: frame.column || 0, line: frame.line || 0 };
156+
break;
157+
}
158+
return { message, stackLines, location };
159+
}
160+
161+
function belongsToNodeModules(file: string, pathSeparator: string) {
162+
return file.includes(`${pathSeparator}node_modules${pathSeparator}`);
163+
}
164+
137165
const re = new RegExp('^' +
138166
// Sometimes we strip out the ' at' because it's noisy
139167
'(?:\\s*at )?' +

packages/playwright/src/DEPS.list

+4
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,11 @@ common/
88

99
[index.ts]
1010
@testIsomorphic/**
11+
./prompt.ts
1112
./worker/testTracing.ts
1213

1314
[internalsForTest.ts]
1415
**
16+
17+
[prompt.ts]
18+
./transform/babelBundle.ts

packages/playwright/src/index.ts

+4-67
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,15 @@ import { setBoxedStackPrefixes, asLocator, createGuid, currentZone, debugMode, i
2222

2323
import { currentTestInfo } from './common/globals';
2424
import { rootTestType } from './common/testType';
25-
import { stripAnsiEscapes } from './util';
25+
import { attachErrorPrompts } from './prompt';
2626

27-
import type { MetadataWithCommitInfo } from './isomorphic/types';
2827
import type { Fixtures, PlaywrightTestArgs, PlaywrightTestOptions, PlaywrightWorkerArgs, PlaywrightWorkerOptions, ScreenshotMode, TestInfo, TestType, VideoMode } from '../types/test';
2928
import type { ContextReuseMode } from './common/config';
3029
import type { TestInfoImpl, TestStepInternal } from './worker/testInfo';
3130
import type { ApiCallData, ClientInstrumentation, ClientInstrumentationListener } from '../../playwright-core/src/client/clientInstrumentation';
3231
import type { Playwright as PlaywrightImpl } from '../../playwright-core/src/client/playwright';
3332
import type { APIRequestContext, Browser, BrowserContext, BrowserContextOptions, LaunchOptions, Page, Tracing, Video } from 'playwright-core';
33+
3434
export { expect } from './matchers/expect';
3535
export const _baseTest: TestType<{}, {}> = rootTestType.test;
3636

@@ -619,6 +619,7 @@ class ArtifactsRecorder {
619619

620620
private _screenshotRecorder: SnapshotRecorder;
621621
private _pageSnapshot: string | undefined;
622+
private _sourceCache: Map<string, string> = new Map();
622623

623624
constructor(playwright: PlaywrightImpl, artifactsDir: string, screenshot: ScreenshotOption) {
624625
this._playwright = playwright;
@@ -701,71 +702,7 @@ class ArtifactsRecorder {
701702
})));
702703

703704
await this._screenshotRecorder.persistTemporary();
704-
await this._attachErrorPrompts();
705-
}
706-
707-
private async _attachErrorPrompts() {
708-
if (process.env.PLAYWRIGHT_NO_COPY_PROMPT)
709-
return;
710-
711-
if (this._testInfo.errors.length === 0)
712-
return;
713-
714-
const testSources = await fs.promises.readFile(this._testInfo.file, 'utf-8');
715-
for (const [index, error] of this._testInfo.errors.entries()) {
716-
if (this._testInfo.attachments.find(a => a.name === `_prompt-${index}`))
717-
continue;
718-
719-
const metadata = this._testInfo.config.metadata as MetadataWithCommitInfo;
720-
721-
const promptParts = [
722-
`My Playwright test failed.`,
723-
`Explain why, be concise, respect Playwright best practices.`,
724-
'',
725-
`Failed test: ${this._testInfo.titlePath.join(' >> ')}`,
726-
'',
727-
'Error:',
728-
'',
729-
'```',
730-
stripAnsiEscapes(error.stack || error.message || ''),
731-
'```',
732-
];
733-
734-
if (this._pageSnapshot) {
735-
promptParts.push(
736-
'',
737-
'Page snapshot:',
738-
'```yaml',
739-
this._pageSnapshot,
740-
'```',
741-
);
742-
}
743-
744-
if (metadata.gitDiff) {
745-
promptParts.push(
746-
'',
747-
'Local changes:',
748-
'```diff',
749-
metadata.gitDiff,
750-
'```',
751-
);
752-
}
753-
754-
promptParts.push(
755-
'',
756-
'Test file:',
757-
'```ts',
758-
`// ${this._testInfo.file}`,
759-
testSources,
760-
'```',
761-
);
762-
763-
this._testInfo._attach({
764-
name: `_prompt-${index}`,
765-
contentType: 'text/markdown',
766-
body: Buffer.from(promptParts.join('\n')),
767-
}, undefined);
768-
}
705+
await attachErrorPrompts(this._testInfo, this._sourceCache, this._pageSnapshot);
769706
}
770707

771708
private async _startTraceChunkOnContextCreation(tracing: Tracing) {

packages/playwright/src/prompt.ts

+124
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
/**
2+
* Copyright (c) Microsoft Corporation.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the 'License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
import * as fs from 'fs';
18+
import * as path from 'path';
19+
20+
import { parseErrorStack } from 'playwright-core/lib/utils';
21+
22+
import { stripAnsiEscapes } from './util';
23+
import { codeFrameColumns } from './transform/babelBundle';
24+
25+
import type { TestInfo } from '../types/test';
26+
import type { MetadataWithCommitInfo } from './isomorphic/types';
27+
import type { TestInfoImpl } from './worker/testInfo';
28+
29+
export async function attachErrorPrompts(testInfo: TestInfo, sourceCache: Map<string, string>, ariaSnapshot: string | undefined) {
30+
if (process.env.PLAYWRIGHT_NO_COPY_PROMPT)
31+
return;
32+
33+
for (const [index, error] of testInfo.errors.entries()) {
34+
if (testInfo.attachments.find(a => a.name === `_prompt-${index}`))
35+
continue;
36+
37+
const metadata = testInfo.config.metadata as MetadataWithCommitInfo;
38+
39+
const promptParts = [
40+
`# Instructions`,
41+
'',
42+
`- Following Playwright test failed.`,
43+
`- Explain why, be concise, respect Playwright best practices.`,
44+
`- Provide a snippet of code with the fix is possible.`,
45+
'',
46+
`# Test info`,
47+
'',
48+
`- Name: ${testInfo.titlePath.slice(1).join(' >> ')}`,
49+
`- Location: ${testInfo.file}:${testInfo.line}:${testInfo.column}`,
50+
'',
51+
'# Error details',
52+
'',
53+
'```',
54+
stripAnsiEscapes(error.stack || error.message || ''),
55+
'```',
56+
];
57+
58+
if (ariaSnapshot) {
59+
promptParts.push(
60+
'',
61+
'# Page snapshot',
62+
'',
63+
'```yaml',
64+
ariaSnapshot,
65+
'```',
66+
);
67+
}
68+
69+
const parsedError = error.stack ? parseErrorStack(error.stack, path.sep) : undefined;
70+
const inlineMessage = stripAnsiEscapes(parsedError?.message || error.message || '').split('\n')[0];
71+
const location = parsedError?.location || { file: testInfo.file, line: testInfo.line, column: testInfo.column };
72+
const source = await loadSource(location.file, sourceCache);
73+
const codeFrame = codeFrameColumns(
74+
source,
75+
{
76+
start: {
77+
line: location.line,
78+
column: location.column
79+
},
80+
},
81+
{
82+
highlightCode: false,
83+
linesAbove: 100,
84+
linesBelow: 100,
85+
message: inlineMessage || undefined,
86+
}
87+
);
88+
promptParts.push(
89+
'',
90+
'# Test source',
91+
'',
92+
'```ts',
93+
codeFrame,
94+
'```',
95+
);
96+
97+
if (metadata.gitDiff) {
98+
promptParts.push(
99+
'',
100+
'# Local changes',
101+
'',
102+
'```diff',
103+
metadata.gitDiff,
104+
'```',
105+
);
106+
}
107+
108+
(testInfo as TestInfoImpl)._attach({
109+
name: `_prompt-${index}`,
110+
contentType: 'text/markdown',
111+
body: Buffer.from(promptParts.join('\n')),
112+
}, undefined);
113+
}
114+
}
115+
116+
async function loadSource(file: string, sourceCache: Map<string, string>) {
117+
let source = sourceCache.get(file);
118+
if (!source) {
119+
// A mild race is Ok here.
120+
source = await fs.promises.readFile(file, 'utf8');
121+
sourceCache.set(file, source);
122+
}
123+
return source;
124+
}

packages/playwright/src/reporters/base.ts

+2-23
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@
1616

1717
import path from 'path';
1818

19-
import { getPackageManagerExecCommand } from 'playwright-core/lib/utils';
20-
import { parseStackFrame } from 'playwright-core/lib/utils';
19+
import { getPackageManagerExecCommand, parseErrorStack } from 'playwright-core/lib/utils';
2120
import { ms as milliseconds } from 'playwright-core/lib/utilsBundle';
2221
import { colors as realColors, noColors } from 'playwright-core/lib/utils';
2322

@@ -540,23 +539,7 @@ export function prepareErrorStack(stack: string): {
540539
stackLines: string[];
541540
location?: Location;
542541
} {
543-
const lines = stack.split('\n');
544-
let firstStackLine = lines.findIndex(line => line.startsWith(' at '));
545-
if (firstStackLine === -1)
546-
firstStackLine = lines.length;
547-
const message = lines.slice(0, firstStackLine).join('\n');
548-
const stackLines = lines.slice(firstStackLine);
549-
let location: Location | undefined;
550-
for (const line of stackLines) {
551-
const frame = parseStackFrame(line, path.sep, !!process.env.PWDEBUGIMPL);
552-
if (!frame || !frame.file)
553-
continue;
554-
if (belongsToNodeModules(frame.file))
555-
continue;
556-
location = { file: frame.file, column: frame.column || 0, line: frame.line || 0 };
557-
break;
558-
}
559-
return { message, stackLines, location };
542+
return parseErrorStack(stack, path.sep, !!process.env.PWDEBUGIMPL);
560543
}
561544

562545
function characterWidth(c: string) {
@@ -611,10 +594,6 @@ export function fitToWidth(line: string, width: number, prefix?: string): string
611594
return taken.reverse().join('');
612595
}
613596

614-
function belongsToNodeModules(file: string) {
615-
return file.includes(`${path.sep}node_modules${path.sep}`);
616-
}
617-
618597
function resolveFromEnv(name: string): string | undefined {
619598
const value = process.env[name];
620599
if (value)

tests/playwright-test/ui-mode-llm.spec.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import { test, expect, retries } from './ui-mode-fixtures';
1818

1919
test.describe.configure({ mode: 'parallel', retries });
2020

21-
test('openai', async ({ runUITest, server }) => {
21+
test.fixme('openai', async ({ runUITest, server }) => {
2222
server.setRoute('/v1/chat/completions', async (req, res) => {
2323
res.setHeader('Access-Control-Allow-Origin', '*');
2424
res.setHeader('Access-Control-Allow-Headers', '*');
@@ -59,7 +59,7 @@ test('openai', async ({ runUITest, server }) => {
5959
`);
6060
});
6161

62-
test('anthropic', async ({ runUITest, server }) => {
62+
test.fixme('anthropic', async ({ runUITest, server }) => {
6363
server.setRoute('/v1/messages', async (req, res) => {
6464
res.setHeader('Access-Control-Allow-Origin', '*');
6565
res.setHeader('Access-Control-Allow-Headers', '*');

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

+6-4
Original file line numberDiff line numberDiff line change
@@ -519,9 +519,11 @@ test('fails', async ({ page }) => {
519519
const prompt = await page.evaluate(() => navigator.clipboard.readText());
520520
expect(prompt, 'contains error').toContain('expect(received).toBe(expected)');
521521
expect(prompt.replaceAll('\r\n', '\n'), 'contains test sources').toContain(`
522-
test('fails', async ({ page }) => {
523-
await page.setContent('<button>Submit</button>');
524-
expect(1).toBe(2);
525-
});
522+
1 | import { test, expect } from '@playwright/test';
523+
2 | test('fails', async ({ page }) => {
524+
3 | await page.setContent('<button>Submit</button>');
525+
> 4 | expect(1).toBe(2);
526+
| ^ Error: expect(received).toBe(expected) // Object.is equality
527+
5 | });
526528
`.trim());
527529
});

0 commit comments

Comments
 (0)