From 971b5da741670733b1b0a961d8016faf7f01e596 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Fri, 22 Nov 2024 18:30:35 -0800 Subject: [PATCH] chore: introduce update-source-method (#33738) --- docs/src/test-api/class-fullconfig.md | 6 ++ docs/src/test-api/class-testconfig.md | 9 +++ packages/playwright/src/common/config.ts | 1 + packages/playwright/src/common/ipc.ts | 1 + .../playwright/src/isomorphic/teleReceiver.ts | 1 + packages/playwright/src/program.ts | 2 + packages/playwright/src/runner/rebase.ts | 29 ++++--- packages/playwright/types/test.d.ts | 14 ++++ .../update-aria-snapshot.spec.ts | 79 +++++++++++++++++++ 9 files changed, 130 insertions(+), 12 deletions(-) diff --git a/docs/src/test-api/class-fullconfig.md b/docs/src/test-api/class-fullconfig.md index 81e102436c..923c9fa858 100644 --- a/docs/src/test-api/class-fullconfig.md +++ b/docs/src/test-api/class-fullconfig.md @@ -118,6 +118,12 @@ See [`property: TestConfig.shard`]. See [`property: TestConfig.updateSnapshots`]. +## property: FullConfig.updateSourceMethod +* since: v1.50 +- type: <[UpdateSourceMethod]<"overwrite"|"3way"|"patch">> + +See [`property: TestConfig.updateSourceMethod`]. + ## property: FullConfig.version * since: v1.20 - type: <[string]> diff --git a/docs/src/test-api/class-testconfig.md b/docs/src/test-api/class-testconfig.md index d9641128bc..37b9ca5f27 100644 --- a/docs/src/test-api/class-testconfig.md +++ b/docs/src/test-api/class-testconfig.md @@ -590,6 +590,15 @@ export default defineConfig({ }); ``` +## property: TestConfig.updateSourceMethod +* since: v1.50 +- type: ?<[UpdateSourceMethod]<"overwrite"|"3way"|"patch">> + +Defines how to update the source code snapshots. +* `'overwrite'` - Overwrite the source code snapshot with the actual result. +* `'3way'` - Use a three-way merge to update the source code snapshot. +* `'patch'` - Use a patch to update the source code snapshot. This is the default. + ## property: TestConfig.use * since: v1.10 - type: ?<[TestOptions]> diff --git a/packages/playwright/src/common/config.ts b/packages/playwright/src/common/config.ts index 031a5215f2..0e8babce10 100644 --- a/packages/playwright/src/common/config.ts +++ b/packages/playwright/src/common/config.ts @@ -97,6 +97,7 @@ export class FullConfigInternal { projects: [], shard: takeFirst(configCLIOverrides.shard, userConfig.shard, null), updateSnapshots: takeFirst(configCLIOverrides.updateSnapshots, userConfig.updateSnapshots, 'missing'), + updateSourceMethod: takeFirst(configCLIOverrides.updateSourceMethod, userConfig.updateSourceMethod, 'patch'), version: require('../../package.json').version, workers: 0, webServer: null, diff --git a/packages/playwright/src/common/ipc.ts b/packages/playwright/src/common/ipc.ts index 2f08a87650..ad0e91f5c3 100644 --- a/packages/playwright/src/common/ipc.ts +++ b/packages/playwright/src/common/ipc.ts @@ -39,6 +39,7 @@ export type ConfigCLIOverrides = { tsconfig?: string; ignoreSnapshots?: boolean; updateSnapshots?: 'all'|'changed'|'missing'|'none'; + updateSourceMethod?: 'overwrite'|'patch'|'3way'; workers?: number | string; projects?: { name: string, use?: any }[], use?: any; diff --git a/packages/playwright/src/isomorphic/teleReceiver.ts b/packages/playwright/src/isomorphic/teleReceiver.ts index 0c4408096d..f96547d427 100644 --- a/packages/playwright/src/isomorphic/teleReceiver.ts +++ b/packages/playwright/src/isomorphic/teleReceiver.ts @@ -594,6 +594,7 @@ export const baseFullConfig: reporterTypes.FullConfig = { quiet: false, shard: null, updateSnapshots: 'missing', + updateSourceMethod: 'patch', version: '', workers: 0, webServer: null, diff --git a/packages/playwright/src/program.ts b/packages/playwright/src/program.ts index a827483bbd..ea0a48fe6a 100644 --- a/packages/playwright/src/program.ts +++ b/packages/playwright/src/program.ts @@ -303,6 +303,7 @@ function overridesFromOptions(options: { [key: string]: any }): ConfigCLIOverrid tsconfig: options.tsconfig ? path.resolve(process.cwd(), options.tsconfig) : undefined, ignoreSnapshots: options.ignoreSnapshots ? !!options.ignoreSnapshots : undefined, updateSnapshots, + updateSourceMethod: options.updateSourceMethod || 'patch', workers: options.workers, }; @@ -385,6 +386,7 @@ const testOptions: [string, string][] = [ ['--ui-host ', 'Host to serve UI on; specifying this option opens UI in a browser tab'], ['--ui-port ', 'Port to serve UI on, 0 for any free port; specifying this option opens UI in a browser tab'], ['-u, --update-snapshots [mode]', `Update snapshots with actual results. Possible values are 'all', 'changed', 'missing' and 'none'. Not passing defaults to 'missing', passing without value defaults to 'changed'`], + ['--update-source-method ', `Chooses the way source is updated. Possible values are 'overwrite', '3way' and 'patch'. Defaults to 'patch'`], ['-j, --workers ', `Number of concurrent workers or percentage of logical CPU cores, use 1 to run in a single worker (default: 50%)`], ['-x', `Stop after the first failure`], ]; diff --git a/packages/playwright/src/runner/rebase.ts b/packages/playwright/src/runner/rebase.ts index e088b82742..de18df465b 100644 --- a/packages/playwright/src/runner/rebase.ts +++ b/packages/playwright/src/runner/rebase.ts @@ -56,6 +56,8 @@ export async function applySuggestedRebaselines(config: FullConfigInternal, repo const files: string[] = []; const gitCache = new Map(); + const patchFile = path.join(project.project.outputDir, 'rebaselines.patch'); + for (const fileName of [...suggestedRebaselines.keys()].sort()) { const source = await fs.promises.readFile(fileName, 'utf8'); const lines = source.split('\n'); @@ -97,24 +99,27 @@ export async function applySuggestedRebaselines(config: FullConfigInternal, repo for (const range of ranges) result = result.substring(0, range.start) + range.newText + result.substring(range.end); - if (process.env.PWTEST_UPDATE_SNAPSHOTS === 'overwrite') { + const relativeName = path.relative(process.cwd(), fileName); + files.push(relativeName); + + if (config.config.updateSourceMethod === 'overwrite') { await fs.promises.writeFile(fileName, result); - } else if (process.env.PWTEST_UPDATE_SNAPSHOTS === 'manual') { + } else if (config.config.updateSourceMethod === '3way') { await fs.promises.writeFile(fileName, applyPatchWithConflictMarkers(source, result)); } else { const gitFolder = findGitRoot(path.dirname(fileName), gitCache); - const relativeName = path.relative(gitFolder || process.cwd(), fileName); - files.push(relativeName); - patches.push(createPatch(relativeName, source, result)); - - const patchFile = path.join(project.project.outputDir, 'rebaselines.patch'); - await fs.promises.mkdir(path.dirname(patchFile), { recursive: true }); - await fs.promises.writeFile(patchFile, patches.join('\n')); - - const fileList = files.map(file => ' ' + colors.dim(file)).join('\n'); - reporter.onStdErr(`\nNew baselines created for:\n\n${fileList}\n\n ` + colors.cyan('git apply ' + path.relative(process.cwd(), patchFile)) + '\n'); + const relativeToGit = path.relative(gitFolder || process.cwd(), fileName); + patches.push(createPatch(relativeToGit, source, result)); } } + + const fileList = files.map(file => ' ' + colors.dim(file)).join('\n'); + reporter.onStdErr(`\nNew baselines created for:\n\n${fileList}\n`); + if (config.config.updateSourceMethod === 'patch') { + await fs.promises.mkdir(path.dirname(patchFile), { recursive: true }); + await fs.promises.writeFile(patchFile, patches.join('\n')); + reporter.onStdErr(`\n ` + colors.cyan('git apply ' + path.relative(process.cwd(), patchFile)) + '\n'); + } } function createPatch(fileName: string, before: string, after: string) { diff --git a/packages/playwright/types/test.d.ts b/packages/playwright/types/test.d.ts index c80329ca8a..8d05bdafef 100644 --- a/packages/playwright/types/test.d.ts +++ b/packages/playwright/types/test.d.ts @@ -1688,6 +1688,14 @@ interface TestConfig { */ updateSnapshots?: "all"|"changed"|"missing"|"none"; + /** + * Defines how to update the source code snapshots. + * - `'overwrite'` - Overwrite the source code snapshot with the actual result. + * - `'3way'` - Use a three-way merge to update the source code snapshot. + * - `'patch'` - Use a patch to update the source code snapshot. This is the default. + */ + updateSourceMethod?: "overwrite"|"3way"|"patch"; + /** * The maximum number of concurrent worker processes to use for parallelizing tests. Can also be set as percentage of * logical CPU cores, e.g. `'50%'.` @@ -1837,6 +1845,12 @@ export interface FullConfig { */ updateSnapshots: "all"|"changed"|"missing"|"none"; + /** + * See + * [testConfig.updateSourceMethod](https://playwright.dev/docs/api/class-testconfig#test-config-update-source-method). + */ + updateSourceMethod: "overwrite"|"3way"|"patch"; + /** * Playwright version. */ diff --git a/tests/playwright-test/update-aria-snapshot.spec.ts b/tests/playwright-test/update-aria-snapshot.spec.ts index 3b07e8b93e..6a3d10eb43 100644 --- a/tests/playwright-test/update-aria-snapshot.spec.ts +++ b/tests/playwright-test/update-aria-snapshot.spec.ts @@ -490,3 +490,82 @@ test.describe('update-snapshots all', () => { expect(result2.exitCode).toBe(0); }); }); + +test.describe('update-source-method', () => { + test('should overwrite source', async ({ runInlineTest }, testInfo) => { + const result = await runInlineTest({ + '.git/marker': '', + 'a.spec.ts': ` + import { test, expect } from '@playwright/test'; + test('test', async ({ page }) => { + await page.setContent(\`

hello

\`); + await expect(page.locator('body')).toMatchAriaSnapshot(\` + - heading "world" + \`); + }); + ` + }, { 'update-snapshots': 'all', 'update-source-method': 'overwrite' }); + + expect(result.exitCode).toBe(0); + const patchPath = testInfo.outputPath('test-results/rebaselines.patch'); + expect(fs.existsSync(patchPath)).toBeFalsy(); + + const data = fs.readFileSync(testInfo.outputPath('a.spec.ts'), 'utf-8'); + expect(data).toBe(` + import { test, expect } from '@playwright/test'; + test('test', async ({ page }) => { + await page.setContent(\`

hello

\`); + await expect(page.locator('body')).toMatchAriaSnapshot(\` + - heading "hello" [level=1] + \`); + }); + `); + + expect(stripAnsi(result.output).replace(/\\/g, '/')).toContain(`New baselines created for: + + a.spec.ts +`); + + const result2 = await runInlineTest({}); + expect(result2.exitCode).toBe(0); + }); + + test('should 3way source', async ({ runInlineTest }, testInfo) => { + const result = await runInlineTest({ + '.git/marker': '', + 'a.spec.ts': ` + import { test, expect } from '@playwright/test'; + test('test', async ({ page }) => { + await page.setContent(\`

hello

\`); + await expect(page.locator('body')).toMatchAriaSnapshot(\` + - heading "world" + \`); + }); + ` + }, { 'update-snapshots': 'all', 'update-source-method': '3way' }); + + expect(result.exitCode).toBe(0); + const patchPath = testInfo.outputPath('test-results/rebaselines.patch'); + expect(fs.existsSync(patchPath)).toBeFalsy(); + + const data = fs.readFileSync(testInfo.outputPath('a.spec.ts'), 'utf-8'); + expect(data).toBe(` + import { test, expect } from '@playwright/test'; + test('test', async ({ page }) => { + await page.setContent(\`

hello

\`); + await expect(page.locator('body')).toMatchAriaSnapshot(\` +\<<<<<<< HEAD + - heading "world" +======= + - heading "hello" [level=1] +>>>>>>> SNAPSHOT + \`); + }); + `); + + expect(stripAnsi(result.output).replace(/\\/g, '/')).toContain(`New baselines created for: + + a.spec.ts +`); + }); +});