chore: do not compute git diff on PRs

Author: pavelfeldman

docs/src/test-api/class-testconfig.md

@@ -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]>

packages/playwright/src/isomorphic/types.d.ts

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

packages/playwright/src/plugins/gitCommitInfoPlugin.ts

@@ -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}`,
+      commitHash: process.env.GITHUB_SHA,
       prHref: pr ? `${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/pull/${pr.number}` : undefined,
-      prTitle: pr ? pr.title : undefined,
+      prTitle: pr?.title,
+      prBaseHash: pr?.baseHash,
       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,
     };
   }
 
   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) {
+  if (ci?.prBaseHash) {
     // First try the diff against the base branch.
-    const diff = await runGit(`git fetch origin ${ci.baseHash} && git diff ${ci.baseHash} HEAD`, gitDir);
+    const diff = await runGit(`git fetch origin ${ci.prBaseHash} && 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)

packages/playwright/src/prompt.ts

@@ -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 = [

packages/playwright/types/test.d.ts

@@ -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;
 

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

@@ -2786,6 +2786,40 @@ 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, writeFiles, showReport, page }) => {
+      const files = {
+        '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);
+          });
+        `,
+      };
+      const baseDir = await writeFiles(files);
+      await initGitRepo(baseDir);
+
+      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_REF_NAME: '42/merge',
+        GITHUB_BASE_REF: 'HEAD~1',
+        PLAYWRIGHT_COPY_PROMPT: '1',
+      });
+
+      expect(result.exitCode).toBe(1);
+      await showReport();
+
+      await page.getByRole('link', { name: 'sample' }).click();
+      await expect(page.getByRole('button', { name: 'Copy prompt' })).toHaveCount(1);
+    });
   });
 }