fix(test-runner): sanitize snapshot name before constructing a path (#7620)

This avoids problems with `toMatchSnapshot('../../dir/file.png')`
where we append this path to `snapshotDir` and end up in some random
place.

Also added a note to documentation.

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

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);
-          const ext = path.extname(snapshotName);
+        if (ext)
-          if (ext)
+          snapshotName = sanitizeForFilePath(snapshotName.substring(0, snapshotName.length - ext.length)) + suffix + ext;
-            snapshotName = snapshotName.substring(0, snapshotName.length - ext.length) + suffix + ext;
+        else
-          else
+          snapshotName = sanitizeForFilePath(snapshotName) + suffix;
-            snapshotName += suffix;
-        }
         return path.join(spec._requireFile + '-snapshots', snapshotName);
       },
       skip: (...args: [arg?: any, description?: string]) => modifier(testInfo, 'skip', args),

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');
+});