From 411f93829603d5c8fedfb0a053634357b59bceef Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Tue, 25 Feb 2025 09:21:17 -0800 Subject: [PATCH] chore: clean up git commit metadata props and UI (#34867) --- docs/src/test-api/class-testconfig.md | 22 +----- packages/html-reporter/src/metadataView.css | 7 +- packages/html-reporter/src/metadataView.tsx | 67 +++++++---------- packages/html-reporter/src/testErrorView.tsx | 2 +- packages/playwright/src/common/config.ts | 3 - packages/playwright/src/isomorphic/types.d.ts | 30 +++++--- .../src/plugins/gitCommitInfoPlugin.ts | 75 +++++++++++-------- packages/playwright/types/test.d.ts | 28 +------ packages/trace-viewer/src/ui/errorsTab.tsx | 2 +- tests/playwright-test/reporter-html.spec.ts | 51 ++++++++----- .../playwright-test/ui-mode-metadata.spec.ts | 4 +- 11 files changed, 129 insertions(+), 162 deletions(-) diff --git a/docs/src/test-api/class-testconfig.md b/docs/src/test-api/class-testconfig.md index 0d30f612ac..33419387c7 100644 --- a/docs/src/test-api/class-testconfig.md +++ b/docs/src/test-api/class-testconfig.md @@ -239,7 +239,7 @@ export default defineConfig({ Metadata contains key-value pairs to be included in the report. For example, HTML report will display it as key-value pairs, and JSON report will include metadata serialized as json. -See also [`property: TestConfig.populateGitInfo`] that populates metadata. +Providing `'git.commit.info': {}` property will populate it with the git commit details. This is useful for CI/CD environments. **Usage** @@ -326,26 +326,6 @@ This path will serve as the base directory for each test file snapshot directory ## property: TestConfig.snapshotPathTemplate = %%-test-config-snapshot-path-template-%% * since: v1.28 -## property: TestConfig.populateGitInfo -* since: v1.51 -- type: ?<[boolean]> - -Whether to populate `'git.commit.info'` field of the [`property: TestConfig.metadata`] with Git commit info and CI/CD information. - -This information will appear in the HTML and JSON reports and is available in the Reporter API. - -On Github Actions, this feature is enabled by default. - -**Usage** - -```js title="playwright.config.ts" -import { defineConfig } from '@playwright/test'; - -export default defineConfig({ - populateGitInfo: !!process.env.CI, -}); -``` - ## property: TestConfig.preserveOutput * since: v1.10 - type: ?<[PreserveOutput]<"always"|"never"|"failures-only">> diff --git a/packages/html-reporter/src/metadataView.css b/packages/html-reporter/src/metadataView.css index 0cbced5250..c383d6776a 100644 --- a/packages/html-reporter/src/metadataView.css +++ b/packages/html-reporter/src/metadataView.css @@ -37,10 +37,6 @@ line-height: 24px; } -.metadata-section { - align-items: center; -} - .metadata-properties { display: flex; flex-direction: column; @@ -57,9 +53,8 @@ border-bottom: 1px solid var(--color-border-default); } -.git-commit-info a { +.metadata-view a { color: var(--color-fg-default); - font-weight: 600; } .copyable-property { diff --git a/packages/html-reporter/src/metadataView.tsx b/packages/html-reporter/src/metadataView.tsx index 0f54bb4249..a7db657b4d 100644 --- a/packages/html-reporter/src/metadataView.tsx +++ b/packages/html-reporter/src/metadataView.tsx @@ -87,12 +87,12 @@ const InnerMetadataView = () => { {entries.length > 0 &&
} } -
+
{entries.map(([propertyName, value]) => { const valueString = typeof value !== 'object' || value === null || value === undefined ? String(value) : JSON.stringify(value); const trimmedValue = valueString.length > 1000 ? valueString.slice(0, 1000) + '\u2026' : valueString; return ( -
+
{propertyName} : {linkifyText(trimmedValue)} @@ -105,47 +105,38 @@ const InnerMetadataView = () => { }; const GitCommitInfoView: React.FC<{ info: GitCommitInfo }> = ({ info }) => { - const email = info['revision.email'] ? ` <${info['revision.email']}>` : ''; - const author = `${info['revision.author'] || ''}${email}`; + const email = info.revision?.email ? ` <${info.revision?.email}>` : ''; + const author = `${info.revision?.author || ''}${email}`; - let subject = info['revision.subject'] || ''; - let link = info['revision.link']; - let shortSubject = info['revision.id']?.slice(0, 7) || 'unknown'; + let subject = info.revision?.subject || ''; + let link = info.revision?.link; - if (info['pull.link'] && info['pull.title']) { - subject = info['pull.title']; - link = info['pull.link']; - shortSubject = link ? 'Pull Request' : ''; + if (info.pull_request?.link && info.pull_request?.title) { + subject = info.pull_request?.title; + link = info.pull_request?.link; } - const shortTimestamp = Intl.DateTimeFormat(undefined, { dateStyle: 'medium' }).format(info['revision.timestamp']); - const longTimestamp = Intl.DateTimeFormat(undefined, { dateStyle: 'full', timeStyle: 'long' }).format(info['revision.timestamp']); - return
-
-
- {link ? ( - - {subject} - - ) : + const shortTimestamp = Intl.DateTimeFormat(undefined, { dateStyle: 'medium' }).format(info.revision?.timestamp); + const longTimestamp = Intl.DateTimeFormat(undefined, { dateStyle: 'full', timeStyle: 'long' }).format(info.revision?.timestamp); + return +
+ {author} + on {shortTimestamp} + {info.ci?.link && ( + <> + · + Logs + + )}
- {link ? ( - - {shortSubject} - - ) : !!shortSubject && {shortSubject}}
; }; diff --git a/packages/html-reporter/src/testErrorView.tsx b/packages/html-reporter/src/testErrorView.tsx index 42ec8d9ff6..9134f082d0 100644 --- a/packages/html-reporter/src/testErrorView.tsx +++ b/packages/html-reporter/src/testErrorView.tsx @@ -50,7 +50,7 @@ const PromptButton: React.FC<{ const gitCommitInfo = useGitCommitInfo(); const prompt = React.useMemo(() => fixTestPrompt( error, - gitCommitInfo?.['pull.diff'] ?? gitCommitInfo?.['revision.diff'], + gitCommitInfo?.pull_request?.diff ?? gitCommitInfo?.revision?.diff, result?.attachments.find(a => a.name === 'pageSnapshot')?.body ), [gitCommitInfo, result, error]); diff --git a/packages/playwright/src/common/config.ts b/packages/playwright/src/common/config.ts index 4578c3771a..4e266110fd 100644 --- a/packages/playwright/src/common/config.ts +++ b/packages/playwright/src/common/config.ts @@ -48,7 +48,6 @@ export class FullConfigInternal { readonly plugins: TestRunnerPluginRegistration[]; readonly projects: FullProjectInternal[] = []; readonly singleTSConfigPath?: string; - readonly populateGitInfo: boolean; cliArgs: string[] = []; cliGrep: string | undefined; cliGrepInvert: string | undefined; @@ -78,7 +77,6 @@ export class FullConfigInternal { const privateConfiguration = (userConfig as any)['@playwright/test']; this.plugins = (privateConfiguration?.plugins || []).map((p: any) => ({ factory: p })); this.singleTSConfigPath = pathResolve(configDir, userConfig.tsconfig); - this.populateGitInfo = takeFirst(userConfig.populateGitInfo, defaultPopulateGitInfo); this.globalSetups = (Array.isArray(userConfig.globalSetup) ? userConfig.globalSetup : [userConfig.globalSetup]).map(s => resolveScript(s, configDir)).filter(script => script !== undefined); this.globalTeardowns = (Array.isArray(userConfig.globalTeardown) ? userConfig.globalTeardown : [userConfig.globalTeardown]).map(s => resolveScript(s, configDir)).filter(script => script !== undefined); @@ -301,7 +299,6 @@ function resolveScript(id: string | undefined, rootDir: string): string | undefi export const defaultGrep = /.*/; export const defaultReporter = process.env.CI ? 'dot' : 'list'; -const defaultPopulateGitInfo = process.env.GITHUB_ACTIONS === 'true'; const configInternalSymbol = Symbol('configInternalSymbol'); diff --git a/packages/playwright/src/isomorphic/types.d.ts b/packages/playwright/src/isomorphic/types.d.ts index 2619c0df33..2d54911c6e 100644 --- a/packages/playwright/src/isomorphic/types.d.ts +++ b/packages/playwright/src/isomorphic/types.d.ts @@ -15,16 +15,22 @@ */ export interface GitCommitInfo { - 'revision.id'?: string; - 'revision.author'?: string; - 'revision.email'?: string; - 'revision.subject'?: string; - 'revision.timestamp'?: number | Date; - 'revision.link'?: string; - 'revision.diff'?: string; - 'pull.link'?: string; - 'pull.diff'?: string; - 'pull.base'?: string; - 'pull.title'?: string; - 'ci.link'?: string; + revision?: { + id?: string; + author?: string; + email?: string; + subject?: string; + timestamp?: number; + link?: string; + diff?: string; + }, + pull_request?: { + link?: string; + diff?: string; + base?: string; + title?: string; + }, + ci?: { + link?: string; + } } diff --git a/packages/playwright/src/plugins/gitCommitInfoPlugin.ts b/packages/playwright/src/plugins/gitCommitInfoPlugin.ts index 7ade4a005c..20945f6332 100644 --- a/packages/playwright/src/plugins/gitCommitInfoPlugin.ts +++ b/packages/playwright/src/plugins/gitCommitInfoPlugin.ts @@ -26,7 +26,8 @@ import type { GitCommitInfo } from '../isomorphic/types'; const GIT_OPERATIONS_TIMEOUT_MS = 1500; export const addGitCommitInfoPlugin = (fullConfig: FullConfigInternal) => { - if (fullConfig.populateGitInfo) + const commitProperty = fullConfig.config.metadata['git.commit.info']; + if (commitProperty && typeof commitProperty === 'object' && Object.keys(commitProperty).length === 0) fullConfig.plugins.push({ factory: gitCommitInfo }); }; @@ -35,10 +36,10 @@ export const gitCommitInfo = (options?: GitCommitInfoPluginOptions): TestRunnerP name: 'playwright:git-commit-info', setup: async (config: FullConfig, configDir: string) => { - const fromEnv = await linksFromEnv(); - const fromCLI = await gitStatusFromCLI(options?.directory || configDir, fromEnv); + const commitInfo = await linksFromEnv(); + await enrichStatusFromCLI(options?.directory || configDir, commitInfo); config.metadata = config.metadata || {}; - config.metadata['git.commit.info'] = { ...fromEnv, ...fromCLI }; + config.metadata['git.commit.info'] = commitInfo; }, }; }; @@ -47,28 +48,39 @@ interface GitCommitInfoPluginOptions { directory?: string; } -async function linksFromEnv() { - const out: Partial = {}; +async function linksFromEnv(): Promise { + const out: GitCommitInfo = {}; // Jenkins: https://www.jenkins.io/doc/book/pipeline/jenkinsfile/#using-environment-variables - if (process.env.BUILD_URL) - out['ci.link'] = process.env.BUILD_URL; + if (process.env.BUILD_URL) { + out.ci = out.ci || {}; + out.ci.link = process.env.BUILD_URL; + } // GitLab: https://docs.gitlab.com/ee/ci/variables/predefined_variables.html - if (process.env.CI_PROJECT_URL && process.env.CI_COMMIT_SHA) - out['revision.link'] = `${process.env.CI_PROJECT_URL}/-/commit/${process.env.CI_COMMIT_SHA}`; - if (process.env.CI_JOB_URL) - out['ci.link'] = process.env.CI_JOB_URL; + if (process.env.CI_PROJECT_URL && process.env.CI_COMMIT_SHA) { + out.revision = out.revision || {}; + out.revision.link = `${process.env.CI_PROJECT_URL}/-/commit/${process.env.CI_COMMIT_SHA}`; + } + if (process.env.CI_JOB_URL) { + out.ci = out.ci || {}; + out.ci.link = process.env.CI_JOB_URL; + } // GitHub: https://docs.github.com/en/actions/learn-github-actions/environment-variables#default-environment-variables - if (process.env.GITHUB_SERVER_URL && process.env.GITHUB_REPOSITORY && process.env.GITHUB_SHA) - out['revision.link'] = `${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/commit/${process.env.GITHUB_SHA}`; - if (process.env.GITHUB_SERVER_URL && process.env.GITHUB_REPOSITORY && process.env.GITHUB_RUN_ID) - out['ci.link'] = `${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/actions/runs/${process.env.GITHUB_RUN_ID}`; + if (process.env.GITHUB_SERVER_URL && process.env.GITHUB_REPOSITORY && process.env.GITHUB_SHA) { + out.revision = out.revision || {}; + out.revision.link = `${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/commit/${process.env.GITHUB_SHA}`; + } + if (process.env.GITHUB_SERVER_URL && process.env.GITHUB_REPOSITORY && process.env.GITHUB_RUN_ID) { + out.ci = out.ci || {}; + out.ci.link = `${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/actions/runs/${process.env.GITHUB_RUN_ID}`; + } if (process.env.GITHUB_EVENT_PATH) { try { const json = JSON.parse(await fs.promises.readFile(process.env.GITHUB_EVENT_PATH, 'utf8')); if (json.pull_request) { - out['pull.title'] = json.pull_request.title; - out['pull.link'] = `${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/pull/${json.pull_request.number}`; - out['pull.base'] = json.pull_request.base.ref; + out.pull_request = out.pull_request || {}; + out.pull_request.title = json.pull_request.title; + out.pull_request.link = `${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/pull/${json.pull_request.number}`; + out.pull_request.base = json.pull_request.base.ref; } } catch { } @@ -76,7 +88,7 @@ async function linksFromEnv() { return out; } -async function gitStatusFromCLI(gitDir: string, envInfo: Pick): Promise { +async function enrichStatusFromCLI(gitDir: string, commitInfo: GitCommitInfo) { const separator = `:${createGuid().slice(0, 4)}:`; const commitInfoResult = await spawnAsync( 'git', @@ -90,23 +102,24 @@ async function gitStatusFromCLI(gitDir: string, envInfo: Pick { * Metadata contains key-value pairs to be included in the report. For example, HTML report will display it as * key-value pairs, and JSON report will include metadata serialized as json. * - * See also - * [testConfig.populateGitInfo](https://playwright.dev/docs/api/class-testconfig#test-config-populate-git-info) that - * populates metadata. + * Providing `'git.commit.info': {}` property will populate it with the git commit details. This is useful for CI/CD + * environments. * * **Usage** * @@ -1360,29 +1359,6 @@ interface TestConfig { */ outputDir?: string; - /** - * Whether to populate `'git.commit.info'` field of the - * [testConfig.metadata](https://playwright.dev/docs/api/class-testconfig#test-config-metadata) with Git commit info - * and CI/CD information. - * - * This information will appear in the HTML and JSON reports and is available in the Reporter API. - * - * On Github Actions, this feature is enabled by default. - * - * **Usage** - * - * ```js - * // playwright.config.ts - * import { defineConfig } from '@playwright/test'; - * - * export default defineConfig({ - * populateGitInfo: !!process.env.CI, - * }); - * ``` - * - */ - populateGitInfo?: boolean; - /** * Whether to preserve test output in the * [testConfig.outputDir](https://playwright.dev/docs/api/class-testconfig#test-config-output-dir). Defaults to diff --git a/packages/trace-viewer/src/ui/errorsTab.tsx b/packages/trace-viewer/src/ui/errorsTab.tsx index a5d9a17aa6..5cb28299df 100644 --- a/packages/trace-viewer/src/ui/errorsTab.tsx +++ b/packages/trace-viewer/src/ui/errorsTab.tsx @@ -101,7 +101,7 @@ function Error({ message, error, errorId, sdkLanguage, pageSnapshot, revealInSou const [showLLM, setShowLLM] = React.useState(false); const llmAvailable = useIsLLMAvailable(); const gitCommitInfo = useGitCommitInfo(); - const diff = gitCommitInfo?.['pull.diff'] ?? gitCommitInfo?.['revision.diff']; + const diff = gitCommitInfo?.pull_request?.diff ?? gitCommitInfo?.revision?.diff; let location: string | undefined; let longLocation: string | undefined; diff --git a/tests/playwright-test/reporter-html.spec.ts b/tests/playwright-test/reporter-html.spec.ts index 9783194073..7e9c550619 100644 --- a/tests/playwright-test/reporter-html.spec.ts +++ b/tests/playwright-test/reporter-html.spec.ts @@ -1187,13 +1187,12 @@ for (const useIntermediateMergeReport of [true, false] as const) { ]); }); - test('should include metadata with populateGitInfo = true', async ({ runInlineTest, writeFiles, showReport, page }) => { + test('should include metadata with git.commit.info', async ({ runInlineTest, writeFiles, showReport, page }) => { const files = { 'uncommitted.txt': `uncommitted file`, 'playwright.config.ts': ` export default { - populateGitInfo: true, - metadata: { foo: 'value1', bar: { prop: 'value2' }, baz: ['value3', 123] } + metadata: { 'git.commit.info': {}, foo: 'value1', bar: { prop: 'value2' }, baz: ['value3', 123] } }; `, 'example.spec.ts': ` @@ -1230,20 +1229,23 @@ for (const useIntermediateMergeReport of [true, false] as const) { expect(result.exitCode).toBe(0); await page.getByRole('button', { name: 'Metadata' }).click(); await expect(page.locator('.metadata-view')).toMatchAriaSnapshot(` - - 'link "chore(html): make this test look nice"' - - text: /^William on/ - - link /^[a-f0-9]{7}$/ - - text: 'foo : value1 bar : {"prop":"value2"} baz : ["value3",123]' + - list: + - listitem: + - 'link "chore(html): make this test look nice"' + - listitem: /William / + - list: + - listitem: "foo : value1" + - listitem: "bar : {\\"prop\\":\\"value2\\"}" + - listitem: "baz : [\\"value3\\",123]" `); }); - test('should include metadata with populateGitInfo on GHA', async ({ runInlineTest, writeFiles, showReport, page }) => { + test('should include metadata with git.commit.info on GHA', async ({ runInlineTest, writeFiles, showReport, page }) => { const files = { 'uncommitted.txt': `uncommitted file`, 'playwright.config.ts': ` export default { - populateGitInfo: true, - metadata: { foo: 'value1', bar: { prop: 'value2' }, baz: ['value3', 123] } + metadata: { 'git.commit.info': {}, foo: 'value1', bar: { prop: 'value2' }, baz: ['value3', 123] } }; `, 'example.spec.ts': ` @@ -1291,18 +1293,23 @@ for (const useIntermediateMergeReport of [true, false] as const) { expect(result.exitCode).toBe(0); await page.getByRole('button', { name: 'Metadata' }).click(); await expect(page.locator('.metadata-view')).toMatchAriaSnapshot(` - - 'link "My PR"' - - text: /^William on/ - - link "Logs" - - link "Pull Request" - - text: 'foo : value1 bar : {"prop":"value2"} baz : ["value3",123]' + - list: + - listitem: + - link "My PR" + - listitem: + - text: /William / + - link "Logs" + - list: + - listitem: "foo : value1" + - listitem: "bar : {\\"prop\\":\\"value2\\"}" + - listitem: "baz : [\\"value3\\",123]" `); }); - test('should not include git metadata with populateGitInfo = false', async ({ runInlineTest, showReport, page }) => { + test('should not include git metadata w/o git.commit.info', async ({ runInlineTest, showReport, page }) => { const result = await runInlineTest({ 'playwright.config.ts': ` - export default { populateGitInfo: false }; + export default {}; `, 'example.spec.ts': ` import { test, expect } from '@playwright/test'; @@ -1323,7 +1330,7 @@ for (const useIntermediateMergeReport of [true, false] as const) { 'playwright.config.ts': ` export default { metadata: { - 'git.commit.info': { 'revision.timestamp': 'hi' } + 'git.commit.info': { revision: { timestamp: 'hi' } } }, }; `, @@ -2757,8 +2764,12 @@ for (const useIntermediateMergeReport of [true, false] as const) { 'uncommitted.txt': `uncommitted file`, 'playwright.config.ts': ` export default { - populateGitInfo: true, - metadata: { foo: 'value1', bar: { prop: 'value2' }, baz: ['value3', 123] } + metadata: { + 'git.commit.info': {}, + foo: 'value1', + bar: { prop: 'value2' }, + baz: ['value3', 123] + } }; `, 'example.spec.ts': ` diff --git a/tests/playwright-test/ui-mode-metadata.spec.ts b/tests/playwright-test/ui-mode-metadata.spec.ts index 8f032ea987..c6127cb7cb 100644 --- a/tests/playwright-test/ui-mode-metadata.spec.ts +++ b/tests/playwright-test/ui-mode-metadata.spec.ts @@ -21,14 +21,14 @@ test('should render html report git info metadata', async ({ runUITest }) => { 'reporter.ts': ` module.exports = class Reporter { onBegin(config, suite) { - console.log('ci.link:', config.metadata['git.commit.info']['ci.link']); + console.log('ci.link:', config.metadata['git.commit.info'].ci.link); } } `, 'playwright.config.ts': ` import { defineConfig } from '@playwright/test'; export default defineConfig({ - populateGitInfo: true, + metadata: { 'git.commit.info': {} }, reporter: './reporter.ts', }); `,