chore: do not compute git diff on PRs (#35062)

This commit is contained in:
Pavel Feldman 2025-03-05 20:43:27 -08:00 committed by GitHub
parent f35a7bce72
commit 319f4630de
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 132 additions and 71 deletions

View File

@ -39,12 +39,10 @@ export default defineConfig({
## property: TestConfig.captureGitInfo
* since: v1.51
- type: ?<[Object]>
- `commit` ?<boolean> Whether to capture commit information such as hash, author, timestamp.
- `commit` ?<boolean> Whether to capture commit and pull request information such as hash, author, timestamp.
- `diff` ?<boolean> Whether to capture commit diff.
* These settings control whether git information is captured and stored in the config [`property: TestConfig.metadata`].
* The structure of the git commit metadata is subject to change.
* Default values for these settings depend on the environment. When tests run as a part of CI where it is safe to obtain git information, the default value is true, false otherwise.
These settings control whether git information is captured and stored in the config [`property: TestConfig.metadata`].
**Usage**
@ -56,6 +54,20 @@ export default defineConfig({
});
```
**Details**
* Capturing `commit` information is useful when you'd like to see it in your HTML (or a third party) report.
* Capturing `diff` information is useful to enrich the report with the actual source diff. This information can be used to provide intelligent advice on how to fix the test.
:::note
Default values for these settings depend on the environment. When tests run as a part of CI where it is safe to obtain git information, the default value is `true`, `false` otherwise.
:::
:::note
The structure of the git commit metadata is subject to change.
:::
## property: TestConfig.expect
* since: v1.10
- type: ?<[Object]>

View File

@ -36,9 +36,9 @@ export type CIInfo = {
commitHref: string;
prHref?: string;
prTitle?: string;
prBaseHash?: string;
buildHref?: string;
commitHash?: string;
baseHash?: string;
branch?: string;
};

View File

@ -71,22 +71,19 @@ async function ciInfo(): Promise<CIInfo | undefined> {
return {
commitHref: `${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/commit/${process.env.GITHUB_SHA}`,
prHref: pr ? `${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/pull/${pr.number}` : undefined,
prTitle: pr ? pr.title : undefined,
buildHref: `${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/actions/runs/${process.env.GITHUB_RUN_ID}`,
commitHash: process.env.GITHUB_SHA,
baseHash: pr ? pr.baseHash : process.env.GITHUB_BASE_REF,
branch: process.env.GITHUB_REF_NAME,
prHref: pr ? `${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/pull/${pr.number}` : undefined,
prTitle: pr?.title,
prBaseHash: pr?.baseHash,
buildHref: `${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/actions/runs/${process.env.GITHUB_RUN_ID}`,
};
}
if (process.env.GITLAB_CI) {
return {
commitHref: `${process.env.CI_PROJECT_URL}/-/commit/${process.env.CI_COMMIT_SHA}`,
prHref: process.env.CI_MERGE_REQUEST_IID ? `${process.env.CI_PROJECT_URL}/-/merge_requests/${process.env.CI_MERGE_REQUEST_IID}` : undefined,
buildHref: process.env.CI_JOB_URL,
commitHash: process.env.CI_COMMIT_SHA,
baseHash: process.env.CI_COMMIT_BEFORE_SHA,
buildHref: process.env.CI_JOB_URL,
branch: process.env.CI_COMMIT_REF_NAME,
};
}
@ -95,7 +92,6 @@ async function ciInfo(): Promise<CIInfo | undefined> {
return {
commitHref: process.env.BUILD_URL,
commitHash: process.env.GIT_COMMIT,
baseHash: process.env.GIT_PREVIOUS_COMMIT,
branch: process.env.GIT_BRANCH,
};
}
@ -144,13 +140,17 @@ async function gitCommitInfo(gitDir: string): Promise<GitCommitInfo | undefined>
async function gitDiff(gitDir: string, ci?: CIInfo): Promise<string | undefined> {
const diffLimit = 100_000;
if (ci?.baseHash) {
// First try the diff against the base branch.
const diff = await runGit(`git fetch origin ${ci.baseHash} && git diff ${ci.baseHash} HEAD`, gitDir);
if (ci?.prBaseHash) {
await runGit(`git fetch origin ${ci.prBaseHash}`, gitDir);
const diff = await runGit(`git diff ${ci.prBaseHash} HEAD`, gitDir);
if (diff)
return diff.substring(0, diffLimit);
}
// Do not attempt to diff on CI commit.
if (ci)
return;
// Check dirty state first.
const uncommitted = await runGit('git diff', gitDir);
if (uncommitted)

View File

@ -30,10 +30,24 @@ export async function attachErrorPrompts(testInfo: TestInfo, sourceCache: Map<st
if (process.env.PLAYWRIGHT_NO_COPY_PROMPT)
return;
const meaningfulSingleLineErrors = new Set(testInfo.errors.filter(e => e.message && !e.message.includes('\n')).map(e => e.message!));
for (const error of testInfo.errors) {
for (const singleLineError of meaningfulSingleLineErrors.keys()) {
if (error.message?.includes(singleLineError))
meaningfulSingleLineErrors.delete(singleLineError);
}
}
for (const [index, error] of testInfo.errors.entries()) {
if (!error.message)
return;
if (testInfo.attachments.find(a => a.name === `_prompt-${index}`))
continue;
// Skip errors that are just a single line - they are likely to already be the error message.
if (!error.message.includes('\n') && !meaningfulSingleLineErrors.has(error.message))
continue;
const metadata = testInfo.config.metadata as MetadataWithCommitInfo;
const promptParts = [

View File

@ -960,11 +960,8 @@ interface TestConfig<TestArgs = {}, WorkerArgs = {}> {
};
/**
* - These settings control whether git information is captured and stored in the config
* [testConfig.metadata](https://playwright.dev/docs/api/class-testconfig#test-config-metadata).
* - The structure of the git commit metadata is subject to change.
* - Default values for these settings depend on the environment. When tests run as a part of CI where it is safe to
* obtain git information, the default value is true, false otherwise.
* These settings control whether git information is captured and stored in the config
* [testConfig.metadata](https://playwright.dev/docs/api/class-testconfig#test-config-metadata).
*
* **Usage**
*
@ -977,10 +974,20 @@ interface TestConfig<TestArgs = {}, WorkerArgs = {}> {
* });
* ```
*
* **Details**
* - Capturing `commit` information is useful when you'd like to see it in your HTML (or a third party) report.
* - Capturing `diff` information is useful to enrich the report with the actual source diff. This information can
* be used to provide intelligent advice on how to fix the test.
*
* **NOTE** Default values for these settings depend on the environment. When tests run as a part of CI where it is
* safe to obtain git information, the default value is `true`, `false` otherwise.
*
* **NOTE** The structure of the git commit metadata is subject to change.
*
*/
captureGitInfo?: {
/**
* Whether to capture commit information such as hash, author, timestamp.
* Whether to capture commit and pull request information such as hash, author, timestamp.
*/
commit?: boolean;

View File

@ -1232,10 +1232,7 @@ for (const useIntermediateMergeReport of [true, false] as const) {
const result = await runInlineTest(files, { reporter: 'dot,html' }, {
PLAYWRIGHT_HTML_OPEN: 'never',
GITHUB_ACTIONS: '1',
GITHUB_REPOSITORY: 'microsoft/playwright-example-for-test',
GITHUB_SERVER_URL: 'https://playwright.dev',
GITHUB_SHA: 'example-sha',
...ghaCommitEnv(),
});
await showReport();
@ -1262,23 +1259,9 @@ for (const useIntermediateMergeReport of [true, false] as const) {
const baseDir = await writeFiles(files);
await initGitRepo(baseDir);
const eventPath = path.join(baseDir, 'event.json');
await fs.promises.writeFile(eventPath, JSON.stringify({
pull_request: {
title: 'My PR',
number: 42,
base: { ref: 'main' },
},
}));
const result = await runInlineTest(files, { reporter: 'dot,html' }, {
PLAYWRIGHT_HTML_OPEN: 'never',
GITHUB_ACTIONS: '1',
GITHUB_REPOSITORY: 'microsoft/playwright-example-for-test',
GITHUB_RUN_ID: 'example-run-id',
GITHUB_SERVER_URL: 'https://playwright.dev',
GITHUB_SHA: 'example-sha',
GITHUB_EVENT_PATH: eventPath,
...(await ghaPullRequestEnv(baseDir))
});
await showReport();
@ -2746,7 +2729,7 @@ for (const useIntermediateMergeReport of [true, false] as const) {
await expect(page.locator('.tree-item', { hasText: 'stdout' })).toHaveCount(1);
});
test('should show AI prompt', async ({ runInlineTest, writeFiles, showReport, page }) => {
test('should include diff in AI prompt', async ({ runInlineTest, writeFiles, showReport, page }) => {
const files = {
'uncommitted.txt': `uncommitted file`,
'playwright.config.ts': `export default {}`,
@ -2754,23 +2737,26 @@ for (const useIntermediateMergeReport of [true, false] as const) {
import { test, expect } from '@playwright/test';
test('sample', async ({ page }) => {
await page.setContent('<button>Click me</button>');
expect(2).toBe(3);
expect(2).toBe(2);
});
`,
};
const baseDir = await writeFiles(files);
await initGitRepo(baseDir);
await writeFiles({
'example.spec.ts': `
import { test, expect } from '@playwright/test';
test('sample', async ({ page }) => {
await page.setContent('<button>Click me</button>');
expect(2).toBe(3);
});`
});
await execGit(baseDir, ['checkout', '-b', 'pr_branch']);
await execGit(baseDir, ['commit', '-am', 'changes']);
const result = await runInlineTest(files, { reporter: 'dot,html' }, {
const result = await runInlineTest({}, { reporter: 'dot,html' }, {
PLAYWRIGHT_HTML_OPEN: 'never',
GITHUB_ACTIONS: '1',
GITHUB_REPOSITORY: 'microsoft/playwright-example-for-test',
GITHUB_RUN_ID: 'example-run-id',
GITHUB_SERVER_URL: 'https://playwright.dev',
GITHUB_SHA: 'example-sha',
GITHUB_REF_NAME: '42/merge',
GITHUB_BASE_REF: 'HEAD~1',
PLAYWRIGHT_COPY_PROMPT: '1',
...(await ghaPullRequestEnv(baseDir)),
});
expect(result.exitCode).toBe(1);
@ -2786,6 +2772,24 @@ for (const useIntermediateMergeReport of [true, false] as const) {
expect(prompt, 'contains snapshot').toContain('- button "Click me"');
expect(prompt, 'contains diff').toContain(`+ expect(2).toBe(3);`);
});
test('should not show prompt for empty timeout error', async ({ runInlineTest, showReport, page }) => {
const result = await runInlineTest({
'uncommitted.txt': `uncommitted file`,
'playwright.config.ts': `export default {}`,
'example.spec.ts': `
import { test, expect } from '@playwright/test';
test('sample', async ({ page }) => {
test.setTimeout(2000);
await page.setChecked('input', true);
});
`,
}, { reporter: 'dot,html' }, { PLAYWRIGHT_HTML_OPEN: 'never' });
expect(result.exitCode).toBe(1);
await showReport();
await page.getByRole('link', { name: 'sample' }).click();
await expect(page.getByRole('button', { name: 'Copy prompt' })).toHaveCount(1);
});
});
}
@ -2797,19 +2801,45 @@ function readAllFromStream(stream: NodeJS.ReadableStream): Promise<Buffer> {
});
}
async function initGitRepo(baseDir) {
const execGit = async (args: string[]) => {
const { code, stdout, stderr } = await spawnAsync('git', args, { stdio: 'pipe', cwd: baseDir });
if (!!code)
throw new Error(`Non-zero exit of:\n$ git ${args.join(' ')}\nConsole:\nstdout:\n${stdout}\n\nstderr:\n${stderr}\n\n`);
return;
};
await execGit(['init']);
await execGit(['config', '--local', 'user.email', 'shakespeare@example.local']);
await execGit(['config', '--local', 'user.name', 'William']);
await execGit(['add', 'playwright.config.ts']);
await execGit(['commit', '-m', 'init']);
await execGit(['add', '*.ts']);
await execGit(['commit', '-m', 'chore(html): make this test look nice']);
async function execGit(baseDir: string, args: string[]) {
const { code, stdout, stderr } = await spawnAsync('git', args, { stdio: 'pipe', cwd: baseDir });
if (!!code)
throw new Error(`Non-zero exit of:\n$ git ${args.join(' ')}\nConsole:\nstdout:\n${stdout}\n\nstderr:\n${stderr}\n\n`);
return;
}
async function initGitRepo(baseDir: string) {
await execGit(baseDir, ['init']);
await execGit(baseDir, ['config', '--local', 'user.email', 'shakespeare@example.local']);
await execGit(baseDir, ['config', '--local', 'user.name', 'William']);
await execGit(baseDir, ['checkout', '-b', 'main']);
await execGit(baseDir, ['add', 'playwright.config.ts']);
await execGit(baseDir, ['commit', '-m', 'init']);
await execGit(baseDir, ['add', '*.ts']);
await execGit(baseDir, ['commit', '-m', 'chore(html): make this test look nice']);
}
function ghaCommitEnv() {
return {
GITHUB_ACTIONS: '1',
GITHUB_REPOSITORY: 'microsoft/playwright-example-for-test',
GITHUB_SERVER_URL: 'https://playwright.dev',
GITHUB_SHA: 'example-sha',
};
}
async function ghaPullRequestEnv(baseDir: string) {
const eventPath = path.join(baseDir, 'event.json');
await fs.promises.writeFile(eventPath, JSON.stringify({
pull_request: {
title: 'My PR',
number: 42,
base: { sha: 'main' },
},
}));
return {
...ghaCommitEnv(),
GITHUB_RUN_ID: 'example-run-id',
GITHUB_EVENT_PATH: eventPath,
};
}

View File

@ -46,7 +46,6 @@ test.fixme('openai', async ({ runUITest, server }) => {
}, {
EXPERIMENTAL_OPENAI_API_KEY: 'fake-key',
OPENAI_BASE_URL: server.PREFIX,
PLAYWRIGHT_COPY_PROMPT: '1',
});
await page.getByTitle('Run all').click();
@ -87,7 +86,6 @@ test.fixme('anthropic', async ({ runUITest, server }) => {
}, {
EXPERIMENTAL_ANTHROPIC_API_KEY: 'fake-key',
ANTHROPIC_BASE_URL: server.PREFIX,
PLAYWRIGHT_COPY_PROMPT: '1',
});
await page.getByTitle('Run all').click();

View File

@ -508,7 +508,7 @@ test('fails', async ({ page }) => {
expect(1).toBe(2);
});
`.trim(),
}, { PLAYWRIGHT_COPY_PROMPT: '1' });
});
await page.getByText('fails').dblclick();