diff --git a/docs/src/test-snapshots.md b/docs/src/test-snapshots.md index 4f57f29c10..c7570d0aae 100644 --- a/docs/src/test-snapshots.md +++ b/docs/src/test-snapshots.md @@ -47,6 +47,8 @@ Sometimes you need to update the reference screenshot, for example when the page npx playwright test --update-snapshots ``` +Note that `snapshotName` is *not a path* relative to the test file, so don't try to use it like `expect(value).toMatchSnapshot('../../test-snapshots/snapshot.png')`. + Playwright Test uses the [pixelmatch](https://github.com/mapbox/pixelmatch) library. You can pass comparison `threshold` as an option. ```js js-flavor=js diff --git a/src/test/workerRunner.ts b/src/test/workerRunner.ts index 9672fab677..8b86f8670b 100644 --- a/src/test/workerRunner.ts +++ b/src/test/workerRunner.ts @@ -243,13 +243,11 @@ export class WorkerRunner extends EventEmitter { suffix += '-' + this._projectNamePathSegment; if (testInfo.snapshotSuffix) suffix += '-' + testInfo.snapshotSuffix; - if (suffix) { - const ext = path.extname(snapshotName); - if (ext) - snapshotName = snapshotName.substring(0, snapshotName.length - ext.length) + suffix + ext; - else - snapshotName += suffix; - } + const ext = path.extname(snapshotName); + if (ext) + snapshotName = sanitizeForFilePath(snapshotName.substring(0, snapshotName.length - ext.length)) + suffix + ext; + else + snapshotName = sanitizeForFilePath(snapshotName) + suffix; return path.join(spec._requireFile + '-snapshots', snapshotName); }, skip: (...args: [arg?: any, description?: string]) => modifier(testInfo, 'skip', args), diff --git a/tests/playwright-test/golden.spec.ts b/tests/playwright-test/golden.spec.ts index 78f3d53d75..34103842dd 100644 --- a/tests/playwright-test/golden.spec.ts +++ b/tests/playwright-test/golden.spec.ts @@ -422,3 +422,33 @@ test('should respect project threshold', async ({runInlineTest}) => { }); expect(result.exitCode).toBe(0); }); + +test('should sanitize snapshot name', async ({runInlineTest}) => { + const result = await runInlineTest({ + 'a.spec.js-snapshots/-snapshot-.txt': `Hello world`, + 'a.spec.js': ` + const { test } = pwt; + test('is a test', ({}) => { + expect('Hello world').toMatchSnapshot('../../snapshot!.txt'); + }); + ` + }); + expect(result.exitCode).toBe(0); +}); + +test('should write missing expectations with sanitized snapshot name', async ({runInlineTest}, testInfo) => { + const result = await runInlineTest({ + 'a.spec.js': ` + const { test } = pwt; + test('is a test', ({}) => { + expect('Hello world').toMatchSnapshot('../../snapshot!.txt'); + }); + ` + }, {}, { CI: '' }); + + expect(result.exitCode).toBe(1); + const snapshotOutputPath = testInfo.outputPath('a.spec.js-snapshots/-snapshot-.txt'); + expect(result.output).toContain(`${snapshotOutputPath} is missing in snapshots, writing actual`); + const data = fs.readFileSync(snapshotOutputPath); + expect(data.toString()).toBe('Hello world'); +});