From 7cccd68de6b75842f2b42a69cc452c46bf3de1e7 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Tue, 4 Mar 2025 19:28:03 -0800 Subject: [PATCH] cherry-pick(#35032): chore: improve prompt to use code frame and inline error --- examples/todomvc/playwright.config.ts | 2 + .../src/utils/isomorphic/stackTrace.ts | 28 ++++ packages/playwright/src/DEPS.list | 4 + packages/playwright/src/index.ts | 71 +--------- packages/playwright/src/prompt.ts | 124 ++++++++++++++++++ packages/playwright/src/reporters/base.ts | 25 +--- tests/playwright-test/ui-mode-llm.spec.ts | 4 +- tests/playwright-test/ui-mode-trace.spec.ts | 10 +- 8 files changed, 172 insertions(+), 96 deletions(-) create mode 100644 packages/playwright/src/prompt.ts diff --git a/examples/todomvc/playwright.config.ts b/examples/todomvc/playwright.config.ts index 3d76370d4d..831aa90355 100644 --- a/examples/todomvc/playwright.config.ts +++ b/examples/todomvc/playwright.config.ts @@ -12,6 +12,8 @@ export default defineConfig({ /* Maximum time one test can run for. */ timeout: 15_000, + captureGitInfo: { commit: true, diff: true }, + expect: { /** diff --git a/packages/playwright-core/src/utils/isomorphic/stackTrace.ts b/packages/playwright-core/src/utils/isomorphic/stackTrace.ts index f3d5a54da0..dca83848e9 100644 --- a/packages/playwright-core/src/utils/isomorphic/stackTrace.ts +++ b/packages/playwright-core/src/utils/isomorphic/stackTrace.ts @@ -134,6 +134,34 @@ export function splitErrorMessage(message: string): { name: string, message: str }; } +export function parseErrorStack(stack: string, pathSeparator: string, showInternalStackFrames: boolean = false): { + message: string; + stackLines: string[]; + location?: StackFrame; +} { + const lines = stack.split('\n'); + let firstStackLine = lines.findIndex(line => line.startsWith(' at ')); + if (firstStackLine === -1) + firstStackLine = lines.length; + const message = lines.slice(0, firstStackLine).join('\n'); + const stackLines = lines.slice(firstStackLine); + let location: StackFrame | undefined; + for (const line of stackLines) { + const frame = parseStackFrame(line, pathSeparator, showInternalStackFrames); + if (!frame || !frame.file) + continue; + if (belongsToNodeModules(frame.file, pathSeparator)) + continue; + location = { file: frame.file, column: frame.column || 0, line: frame.line || 0 }; + break; + } + return { message, stackLines, location }; +} + +function belongsToNodeModules(file: string, pathSeparator: string) { + return file.includes(`${pathSeparator}node_modules${pathSeparator}`); +} + const re = new RegExp('^' + // Sometimes we strip out the ' at' because it's noisy '(?:\\s*at )?' + diff --git a/packages/playwright/src/DEPS.list b/packages/playwright/src/DEPS.list index 200d75de1a..25ca70ff18 100644 --- a/packages/playwright/src/DEPS.list +++ b/packages/playwright/src/DEPS.list @@ -8,7 +8,11 @@ common/ [index.ts] @testIsomorphic/** +./prompt.ts ./worker/testTracing.ts [internalsForTest.ts] ** + +[prompt.ts] +./transform/babelBundle.ts diff --git a/packages/playwright/src/index.ts b/packages/playwright/src/index.ts index cc5a906c4c..a8c128694e 100644 --- a/packages/playwright/src/index.ts +++ b/packages/playwright/src/index.ts @@ -22,15 +22,15 @@ import { setBoxedStackPrefixes, asLocator, createGuid, currentZone, debugMode, i import { currentTestInfo } from './common/globals'; import { rootTestType } from './common/testType'; -import { stripAnsiEscapes } from './util'; +import { attachErrorPrompts } from './prompt'; -import type { MetadataWithCommitInfo } from './isomorphic/types'; import type { Fixtures, PlaywrightTestArgs, PlaywrightTestOptions, PlaywrightWorkerArgs, PlaywrightWorkerOptions, ScreenshotMode, TestInfo, TestType, VideoMode } from '../types/test'; import type { ContextReuseMode } from './common/config'; import type { TestInfoImpl, TestStepInternal } from './worker/testInfo'; import type { ApiCallData, ClientInstrumentation, ClientInstrumentationListener } from '../../playwright-core/src/client/clientInstrumentation'; import type { Playwright as PlaywrightImpl } from '../../playwright-core/src/client/playwright'; import type { APIRequestContext, Browser, BrowserContext, BrowserContextOptions, LaunchOptions, Page, Tracing, Video } from 'playwright-core'; + export { expect } from './matchers/expect'; export const _baseTest: TestType<{}, {}> = rootTestType.test; @@ -619,6 +619,7 @@ class ArtifactsRecorder { private _screenshotRecorder: SnapshotRecorder; private _pageSnapshot: string | undefined; + private _sourceCache: Map = new Map(); constructor(playwright: PlaywrightImpl, artifactsDir: string, screenshot: ScreenshotOption) { this._playwright = playwright; @@ -701,71 +702,7 @@ class ArtifactsRecorder { }))); await this._screenshotRecorder.persistTemporary(); - await this._attachErrorPrompts(); - } - - private async _attachErrorPrompts() { - if (process.env.PLAYWRIGHT_NO_COPY_PROMPT) - return; - - if (this._testInfo.errors.length === 0) - return; - - const testSources = await fs.promises.readFile(this._testInfo.file, 'utf-8'); - for (const [index, error] of this._testInfo.errors.entries()) { - if (this._testInfo.attachments.find(a => a.name === `_prompt-${index}`)) - continue; - - const metadata = this._testInfo.config.metadata as MetadataWithCommitInfo; - - const promptParts = [ - `My Playwright test failed.`, - `Explain why, be concise, respect Playwright best practices.`, - '', - `Failed test: ${this._testInfo.titlePath.join(' >> ')}`, - '', - 'Error:', - '', - '```', - stripAnsiEscapes(error.stack || error.message || ''), - '```', - ]; - - if (this._pageSnapshot) { - promptParts.push( - '', - 'Page snapshot:', - '```yaml', - this._pageSnapshot, - '```', - ); - } - - if (metadata.gitDiff) { - promptParts.push( - '', - 'Local changes:', - '```diff', - metadata.gitDiff, - '```', - ); - } - - promptParts.push( - '', - 'Test file:', - '```ts', - `// ${this._testInfo.file}`, - testSources, - '```', - ); - - this._testInfo._attach({ - name: `_prompt-${index}`, - contentType: 'text/markdown', - body: Buffer.from(promptParts.join('\n')), - }, undefined); - } + await attachErrorPrompts(this._testInfo, this._sourceCache, this._pageSnapshot); } private async _startTraceChunkOnContextCreation(tracing: Tracing) { diff --git a/packages/playwright/src/prompt.ts b/packages/playwright/src/prompt.ts new file mode 100644 index 0000000000..d115f7187a --- /dev/null +++ b/packages/playwright/src/prompt.ts @@ -0,0 +1,124 @@ +/** + * Copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the 'License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as fs from 'fs'; +import * as path from 'path'; + +import { parseErrorStack } from 'playwright-core/lib/utils'; + +import { stripAnsiEscapes } from './util'; +import { codeFrameColumns } from './transform/babelBundle'; + +import type { TestInfo } from '../types/test'; +import type { MetadataWithCommitInfo } from './isomorphic/types'; +import type { TestInfoImpl } from './worker/testInfo'; + +export async function attachErrorPrompts(testInfo: TestInfo, sourceCache: Map, ariaSnapshot: string | undefined) { + if (process.env.PLAYWRIGHT_NO_COPY_PROMPT) + return; + + for (const [index, error] of testInfo.errors.entries()) { + if (testInfo.attachments.find(a => a.name === `_prompt-${index}`)) + continue; + + const metadata = testInfo.config.metadata as MetadataWithCommitInfo; + + const promptParts = [ + `# Instructions`, + '', + `- Following Playwright test failed.`, + `- Explain why, be concise, respect Playwright best practices.`, + `- Provide a snippet of code with the fix is possible.`, + '', + `# Test info`, + '', + `- Name: ${testInfo.titlePath.slice(1).join(' >> ')}`, + `- Location: ${testInfo.file}:${testInfo.line}:${testInfo.column}`, + '', + '# Error details', + '', + '```', + stripAnsiEscapes(error.stack || error.message || ''), + '```', + ]; + + if (ariaSnapshot) { + promptParts.push( + '', + '# Page snapshot', + '', + '```yaml', + ariaSnapshot, + '```', + ); + } + + const parsedError = error.stack ? parseErrorStack(error.stack, path.sep) : undefined; + const inlineMessage = stripAnsiEscapes(parsedError?.message || error.message || '').split('\n')[0]; + const location = parsedError?.location || { file: testInfo.file, line: testInfo.line, column: testInfo.column }; + const source = await loadSource(location.file, sourceCache); + const codeFrame = codeFrameColumns( + source, + { + start: { + line: location.line, + column: location.column + }, + }, + { + highlightCode: false, + linesAbove: 100, + linesBelow: 100, + message: inlineMessage || undefined, + } + ); + promptParts.push( + '', + '# Test source', + '', + '```ts', + codeFrame, + '```', + ); + + if (metadata.gitDiff) { + promptParts.push( + '', + '# Local changes', + '', + '```diff', + metadata.gitDiff, + '```', + ); + } + + (testInfo as TestInfoImpl)._attach({ + name: `_prompt-${index}`, + contentType: 'text/markdown', + body: Buffer.from(promptParts.join('\n')), + }, undefined); + } +} + +async function loadSource(file: string, sourceCache: Map) { + let source = sourceCache.get(file); + if (!source) { + // A mild race is Ok here. + source = await fs.promises.readFile(file, 'utf8'); + sourceCache.set(file, source); + } + return source; +} diff --git a/packages/playwright/src/reporters/base.ts b/packages/playwright/src/reporters/base.ts index b8a78105bc..30534d7e8f 100644 --- a/packages/playwright/src/reporters/base.ts +++ b/packages/playwright/src/reporters/base.ts @@ -16,8 +16,7 @@ import path from 'path'; -import { getPackageManagerExecCommand } from 'playwright-core/lib/utils'; -import { parseStackFrame } from 'playwright-core/lib/utils'; +import { getPackageManagerExecCommand, parseErrorStack } from 'playwright-core/lib/utils'; import { ms as milliseconds } from 'playwright-core/lib/utilsBundle'; import { colors as realColors, noColors } from 'playwright-core/lib/utils'; @@ -540,23 +539,7 @@ export function prepareErrorStack(stack: string): { stackLines: string[]; location?: Location; } { - const lines = stack.split('\n'); - let firstStackLine = lines.findIndex(line => line.startsWith(' at ')); - if (firstStackLine === -1) - firstStackLine = lines.length; - const message = lines.slice(0, firstStackLine).join('\n'); - const stackLines = lines.slice(firstStackLine); - let location: Location | undefined; - for (const line of stackLines) { - const frame = parseStackFrame(line, path.sep, !!process.env.PWDEBUGIMPL); - if (!frame || !frame.file) - continue; - if (belongsToNodeModules(frame.file)) - continue; - location = { file: frame.file, column: frame.column || 0, line: frame.line || 0 }; - break; - } - return { message, stackLines, location }; + return parseErrorStack(stack, path.sep, !!process.env.PWDEBUGIMPL); } function characterWidth(c: string) { @@ -611,10 +594,6 @@ export function fitToWidth(line: string, width: number, prefix?: string): string return taken.reverse().join(''); } -function belongsToNodeModules(file: string) { - return file.includes(`${path.sep}node_modules${path.sep}`); -} - function resolveFromEnv(name: string): string | undefined { const value = process.env[name]; if (value) diff --git a/tests/playwright-test/ui-mode-llm.spec.ts b/tests/playwright-test/ui-mode-llm.spec.ts index b639f52e37..27f5c91aeb 100644 --- a/tests/playwright-test/ui-mode-llm.spec.ts +++ b/tests/playwright-test/ui-mode-llm.spec.ts @@ -18,7 +18,7 @@ import { test, expect, retries } from './ui-mode-fixtures'; test.describe.configure({ mode: 'parallel', retries }); -test('openai', async ({ runUITest, server }) => { +test.fixme('openai', async ({ runUITest, server }) => { server.setRoute('/v1/chat/completions', async (req, res) => { res.setHeader('Access-Control-Allow-Origin', '*'); res.setHeader('Access-Control-Allow-Headers', '*'); @@ -59,7 +59,7 @@ test('openai', async ({ runUITest, server }) => { `); }); -test('anthropic', async ({ runUITest, server }) => { +test.fixme('anthropic', async ({ runUITest, server }) => { server.setRoute('/v1/messages', async (req, res) => { res.setHeader('Access-Control-Allow-Origin', '*'); res.setHeader('Access-Control-Allow-Headers', '*'); diff --git a/tests/playwright-test/ui-mode-trace.spec.ts b/tests/playwright-test/ui-mode-trace.spec.ts index 9cf0f8ac1e..47662adf97 100644 --- a/tests/playwright-test/ui-mode-trace.spec.ts +++ b/tests/playwright-test/ui-mode-trace.spec.ts @@ -519,9 +519,11 @@ test('fails', async ({ page }) => { const prompt = await page.evaluate(() => navigator.clipboard.readText()); expect(prompt, 'contains error').toContain('expect(received).toBe(expected)'); expect(prompt.replaceAll('\r\n', '\n'), 'contains test sources').toContain(` -test('fails', async ({ page }) => { - await page.setContent(''); - expect(1).toBe(2); -}); + 1 | import { test, expect } from '@playwright/test'; + 2 | test('fails', async ({ page }) => { + 3 | await page.setContent(''); +> 4 | expect(1).toBe(2); + | ^ Error: expect(received).toBe(expected) // Object.is equality + 5 | }); `.trim()); });