chore: address API review comments for the snapshotPathTemplate (#18716)

This patch:
- updates documentation to lead users from `TestConfig.snapshotDir` and
  `testInfo.snapshotSuffix` to `TestConfig.snapshotPathTemplate` as a
  better and more flexible alternative.
- drops `{snapshotSuffix}` from documentation
- stops using `snapshotSuffix = ''` in our own tests and switches us
  to the `snapshotPathTemplate`.
- adds `{testName}` token.

docs/src/api/params.md

@@ -1386,31 +1386,34 @@ And the following `page-click.spec.ts` that uses `toHaveScreenshot()` call:
 // page-click.spec.ts
 import { test, expect } from '@playwright/test';
 
-test('should work', async ({ page }) => {
-  await expect(page).toHaveScreenshot(['foo', 'bar', 'baz.png']);
+test.describe('suite', () => {
+  test('test should work', async ({ page }) => {
+    await expect(page).toHaveScreenshot(['foo', 'bar', 'baz.png']);
+  });
 });
 ```
 
 The list of supported tokens:
 
 * `{testDir}` - Project's [`property: TestConfig.testDir`].
-  * Example: `tests/`
+  * Value: `/home/playwright/tests` (absolute path is since `testDir` is resolved relative to directory with config)
 * `{snapshotDir}` - Project's [`property: TestConfig.snapshotDir`].
-  * Example: `tests/` (since `snapshotDir` is not provided in config, it defaults to `testDir`)
+  * Value: `/home/playwright/tests` (since `snapshotDir` is not provided in config, it defaults to `testDir`)
 * `{platform}` - The value of `process.platform`.
-* `{snapshotSuffix}` - The value of [`property: TestInfo.snapshotSuffix`].
-* `{projectName}` - Project's sanitized name, if any.
-  * Example: `''` (empty string).
+* `{projectName}` - Project's file-system-sanitized name, if any.
+  * Value: `''` (empty string).
 * `{testFileDir}` - Directories in relative path from `testDir` to **test file**.
-  * Example: `page/`
+  * Value: `page`
 * `{testFileName}` - Test file name with extension.
-  * Example: `page-click.spec.ts`
+  * Value: `page-click.spec.ts`
 * `{testFilePath}` - Relative path from `testDir` to **test file**
-  * Example: `page/page-click.spec.ts`
+  * Value: `page/page-click.spec.ts`
+* `{testName}` - File-system-sanitized test title, including parent describes but excluding file name.
+  * Value: `suite-test-should-work`
 * `{arg}` - Relative snapshot path **without extension**. These come from the arguments passed to the `toHaveScreenshot()` and `toMatchSnapshot()` calls; if called without arguments, this will be an auto-generated snapshot name.
-  * Example: `foo/bar/baz`
+  * Value: `foo/bar/baz`
 * `{ext}` - snapshot extension (with dots)
-  * Example: `.png`
+  * Value: `.png`
 
 Each token can be preceded with a single character that will be used **only if** this token has non-empty value.
 

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

@@ -331,6 +331,11 @@ test('example test', async ({}, testInfo) => {
 * since: v1.10
 - type: ?<[string]>
 
+:::note
+Use of [`property: TestConfig.snapshotDir`] is discouraged. Please use [`property: TestConfig.snapshotPathTemplate`] to configure
+snapshot paths.
+:::
+
 The base directory, relative to the config file, for snapshot files created with `toMatchSnapshot`. Defaults to [`property: TestConfig.testDir`].
 
 The directory for each test can be accessed by [`property: TestInfo.snapshotDir`] and [`method: TestInfo.snapshotPath`].

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

@@ -460,6 +460,11 @@ The name of the snapshot or the path segments to define the snapshot file path.
 * since: v1.10
 - type: <[string]>
 
+:::note
+Use of [`property: TestInfo.snapshotSuffix`] is discouraged. Please use [`property: TestConfig.snapshotPathTemplate`] to configure
+snapshot paths.
+:::
+
 Suffix used to differentiate snapshots between multiple test configurations. For example, if snapshots depend on the platform, you can set `testInfo.snapshotSuffix` equal to `process.platform`. In this case `expect(value).toMatchSnapshot(snapshotName)` will use different snapshots depending on the platform. Learn more about [snapshots](../test-snapshots.md).
 
 ## property: TestInfo.status

packages/playwright-test/src/testInfo.ts

@@ -234,18 +234,25 @@ export class TestInfoImpl implements TestInfo {
     throw new Error(`The outputPath is not allowed outside of the parent directory. Please fix the defined path.\n\n\toutputPath: ${joinedPath}`);
   }
 
+  _fsSanitizedTestName() {
+    const fullTitleWithoutSpec = this.titlePath.slice(1).join(' ');
+    return sanitizeForFilePath(trimLongString(fullTitleWithoutSpec));
+  }
+
   snapshotPath(...pathSegments: string[]) {
     const subPath = path.join(...pathSegments);
     const parsedSubPath = path.parse(subPath);
     const relativeTestFilePath = path.relative(this.project.testDir, this._test._requireFile);
     const parsedRelativeTestFilePath = path.parse(relativeTestFilePath);
     const projectNamePathSegment = sanitizeForFilePath(this.project.name);
+
     const snapshotPath = path.resolve(this.config._configDir, this.project.snapshotPathTemplate
         .replace(/\{(.)?testDir\}/g, '$1' + this.project.testDir)
         .replace(/\{(.)?snapshotDir\}/g, '$1' + this.project.snapshotDir)
         .replace(/\{(.)?snapshotSuffix\}/g, this.snapshotSuffix ? '$1' + this.snapshotSuffix : ''))
         .replace(/\{(.)?platform\}/g, '$1' + process.platform)
         .replace(/\{(.)?projectName\}/g, projectNamePathSegment ? '$1' + projectNamePathSegment : '')
+        .replace(/\{(.)?testName\}/g, '$1' + this._fsSanitizedTestName())
         .replace(/\{(.)?testFileDir\}/g, '$1' + parsedRelativeTestFilePath.dir)
         .replace(/\{(.)?testFileName\}/g, '$1' + parsedRelativeTestFilePath.base)
         .replace(/\{(.)?testFilePath\}/g, '$1' + relativeTestFilePath)

packages/playwright-test/types/test.d.ts

@@ -756,6 +756,11 @@ interface TestConfig {
   outputDir?: string;
 
   /**
+   * > NOTE: Use of [testConfig.snapshotDir](https://playwright.dev/docs/api/class-testconfig#test-config-snapshot-dir) is
+   * discouraged. Please use
+   * [testConfig.snapshotPathTemplate](https://playwright.dev/docs/api/class-testconfig#test-config-snapshot-path-template)
+   * to configure snapshot paths.
+   *
    * The base directory, relative to the config file, for snapshot files created with `toMatchSnapshot`. Defaults to
    * [testConfig.testDir](https://playwright.dev/docs/api/class-testconfig#test-config-test-dir).
    *
@@ -802,27 +807,27 @@ interface TestConfig {
    *
    * The list of supported tokens:
    * - `{testDir}` - Project's [testConfig.testDir](https://playwright.dev/docs/api/class-testconfig#test-config-test-dir).
-   *   - Example: `tests/`
+   *   - Value: `/home/playwright/tests` (absolute path is since `testDir` is resolved relative to directory with config)
    * - `{snapshotDir}` - Project's
    *   [testConfig.snapshotDir](https://playwright.dev/docs/api/class-testconfig#test-config-snapshot-dir).
-   *   - Example: `tests/` (since `snapshotDir` is not provided in config, it defaults to `testDir`)
+   *   - Value: `/home/playwright/tests` (since `snapshotDir` is not provided in config, it defaults to `testDir`)
    * - `{platform}` - The value of `process.platform`.
-   * - `{snapshotSuffix}` - The value of
-   *   [testInfo.snapshotSuffix](https://playwright.dev/docs/api/class-testinfo#test-info-snapshot-suffix).
-   * - `{projectName}` - Project's sanitized name, if any.
-   *   - Example: `''` (empty string).
+   * - `{projectName}` - Project's file-system-sanitized name, if any.
+   *   - Value: `''` (empty string).
    * - `{testFileDir}` - Directories in relative path from `testDir` to **test file**.
-   *   - Example: `page/`
+   *   - Value: `page`
    * - `{testFileName}` - Test file name with extension.
-   *   - Example: `page-click.spec.ts`
+   *   - Value: `page-click.spec.ts`
    * - `{testFilePath}` - Relative path from `testDir` to **test file**
-   *   - Example: `page/page-click.spec.ts`
+   *   - Value: `page/page-click.spec.ts`
+   * - `{testName}` - File-system-sanitized test title, including parent describes but excluding file name.
+   *   - Value: `suite-test-should-work`
    * - `{arg}` - Relative snapshot path **without extension**. These come from the arguments passed to the
    *   `toHaveScreenshot()` and `toMatchSnapshot()` calls; if called without arguments, this will be an auto-generated
    *   snapshot name.
-   *   - Example: `foo/bar/baz`
+   *   - Value: `foo/bar/baz`
    * - `{ext}` - snapshot extension (with dots)
-   *   - Example: `.png`
+   *   - Value: `.png`
    *
    * Each token can be preceded with a single character that will be used **only if** this token has non-empty value.
    *
@@ -1792,6 +1797,11 @@ export interface TestInfo {
   snapshotPath(...pathSegments: Array<string>): string;
 
   /**
+   * > NOTE: Use of [testInfo.snapshotSuffix](https://playwright.dev/docs/api/class-testinfo#test-info-snapshot-suffix) is
+   * discouraged. Please use
+   * [testConfig.snapshotPathTemplate](https://playwright.dev/docs/api/class-testconfig#test-config-snapshot-path-template)
+   * to configure snapshot paths.
+   *
    * Suffix used to differentiate snapshots between multiple test configurations. For example, if snapshots depend on the
    * platform, you can set `testInfo.snapshotSuffix` equal to `process.platform`. In this case
    * `expect(value).toMatchSnapshot(snapshotName)` will use different snapshots depending on the platform. Learn more about
@@ -4587,27 +4597,27 @@ interface TestProject {
    *
    * The list of supported tokens:
    * - `{testDir}` - Project's [testConfig.testDir](https://playwright.dev/docs/api/class-testconfig#test-config-test-dir).
-   *   - Example: `tests/`
+   *   - Value: `/home/playwright/tests` (absolute path is since `testDir` is resolved relative to directory with config)
    * - `{snapshotDir}` - Project's
    *   [testConfig.snapshotDir](https://playwright.dev/docs/api/class-testconfig#test-config-snapshot-dir).
-   *   - Example: `tests/` (since `snapshotDir` is not provided in config, it defaults to `testDir`)
+   *   - Value: `/home/playwright/tests` (since `snapshotDir` is not provided in config, it defaults to `testDir`)
    * - `{platform}` - The value of `process.platform`.
-   * - `{snapshotSuffix}` - The value of
-   *   [testInfo.snapshotSuffix](https://playwright.dev/docs/api/class-testinfo#test-info-snapshot-suffix).
-   * - `{projectName}` - Project's sanitized name, if any.
-   *   - Example: `''` (empty string).
+   * - `{projectName}` - Project's file-system-sanitized name, if any.
+   *   - Value: `''` (empty string).
    * - `{testFileDir}` - Directories in relative path from `testDir` to **test file**.
-   *   - Example: `page/`
+   *   - Value: `page`
    * - `{testFileName}` - Test file name with extension.
-   *   - Example: `page-click.spec.ts`
+   *   - Value: `page-click.spec.ts`
    * - `{testFilePath}` - Relative path from `testDir` to **test file**
-   *   - Example: `page/page-click.spec.ts`
+   *   - Value: `page/page-click.spec.ts`
+   * - `{testName}` - File-system-sanitized test title, including parent describes but excluding file name.
+   *   - Value: `suite-test-should-work`
    * - `{arg}` - Relative snapshot path **without extension**. These come from the arguments passed to the
    *   `toHaveScreenshot()` and `toMatchSnapshot()` calls; if called without arguments, this will be an auto-generated
    *   snapshot name.
-   *   - Example: `foo/bar/baz`
+   *   - Value: `foo/bar/baz`
    * - `{ext}` - snapshot extension (with dots)
-   *   - Example: `.png`
+   *   - Value: `.png`
    *
    * Each token can be preceded with a single character that will be used **only if** this token has non-empty value.
    *

tests/config/baseTest.ts

@@ -37,7 +37,4 @@ export const baseTest = base
     ._extendTest(platformTest)
     ._extendTest(testModeTest)
     .extend<CommonFixtures, CommonWorkerFixtures>(commonFixtures)
-    .extend<ServerFixtures, ServerWorkerOptions>(serverFixtures)
-    .extend<{}, { _snapshotSuffix: string }>({
-      _snapshotSuffix: ['', { scope: 'worker' }],
-    });
+    .extend<ServerFixtures, ServerWorkerOptions>(serverFixtures);

tests/electron/playwright.config.ts

@@ -51,7 +51,7 @@ const metadata = {
 };
 
 config.projects.push({
-  name: 'chromium',  // We use 'chromium' here to share screenshots with chromium.
+  name: 'electron',
   use: {
     browserName: 'chromium',
     coverageName: 'electron',
@@ -61,7 +61,9 @@ config.projects.push({
 });
 
 config.projects.push({
-  name: 'chromium',  // We use 'chromium' here to share screenshots with chromium.
+  name: 'electron',
+  // Share screenshots with chromium.
+  snapshotPathTemplate: '{testDir}/{testFileDir}/{testFileName}-snapshots/{arg}-chromium{ext}',
   use: {
     browserName: 'chromium',
     coverageName: 'electron',

tests/library/playwright.config.ts

@@ -118,6 +118,7 @@ for (const browserName of browserNames) {
       name: browserName,
       testDir: path.join(testDir, folder),
       testIgnore,
+      snapshotPathTemplate: '{testDir}/{testFileDir}/{testFileName}-snapshots/{arg}{-projectName}{ext}',
       use: {
         mode,
         browserName,

tests/playwright-test/snapshot-path-template.spec.ts

@@ -25,14 +25,16 @@ async function getSnapshotPaths(runInlineTest, testInfo, playwrightConfig, pathA
       module.exports = ${JSON.stringify(playwrightConfig, null, 2)}
     `,
     'a/b/c/d.spec.js': `
-      pwt.test('test', async ({ page }, testInfo) => {
-        console.log([
-          ${JSON.stringify(SEPARATOR)},
-          testInfo.project.name,
-          ${JSON.stringify(SEPARATOR)},
-          testInfo.snapshotPath(...${JSON.stringify(pathArgs)}),
-          ${JSON.stringify(SEPARATOR)},
-        ].join(''));
+      pwt.test.describe('suite', () => {
+        pwt.test('test should work', async ({ page }, testInfo) => {
+          console.log([
+            ${JSON.stringify(SEPARATOR)},
+            testInfo.project.name,
+            ${JSON.stringify(SEPARATOR)},
+            testInfo.snapshotPath(...${JSON.stringify(pathArgs)}),
+            ${JSON.stringify(SEPARATOR)},
+          ].join(''));
+        });
       });
     `
   }, { workers: 1 });
@@ -83,6 +85,9 @@ test('tokens should expand property', async ({ runInlineTest }, testInfo) => {
     }, {
       name: 'snapshotSuffix',
       snapshotPathTemplate: '{-snapshotSuffix}',
+    }, {
+      name: 'testName',
+      snapshotPathTemplate: '{testName}',
     }],
   }, ['foo.png']);
   expect.soft(snapshotPath['proj1']).toBe('proj1');
@@ -97,6 +102,7 @@ test('tokens should expand property', async ({ runInlineTest }, testInfo) => {
   expect.soft(snapshotPath['testFileName']).toBe('d.spec.js');
   expect.soft(snapshotPath['snapshotDir']).toBe('a-snapshot-dir.png');
   expect.soft(snapshotPath['snapshotSuffix']).toBe('-' + process.platform);
+  expect.soft(snapshotPath['testName']).toBe('suite-test-should-work');
 });
 
 test('args array should work', async ({ runInlineTest }, testInfo) => {

tests/webview2/playwright.config.ts

@@ -52,7 +52,9 @@ const metadata = {
 };
 
 config.projects.push({
-  name: 'chromium',  // We use 'chromium' here to share screenshots with chromium.
+  name: 'webview2',
+  // Share screenshots with chromium.
+  snapshotPathTemplate: '{testDir}/{testFileDir}/{testFileName}-snapshots/{arg}-chromium{ext}',
   use: {
     browserName: 'chromium',
     coverageName: 'webview2',