fix: allow changed to be passed in config (microsoft#34473)

Author: pavelfeldman

packages/playwright/src/common/configLoader.ts

@@ -240,8 +240,8 @@ function validateConfig(file: string, config: Config) {
   }
 
   if ('updateSnapshots' in config && config.updateSnapshots !== undefined) {
-    if (typeof config.updateSnapshots !== 'string' || !['all', 'none', 'missing'].includes(config.updateSnapshots))
-      throw errorWithFile(file, `config.updateSnapshots must be one of "all", "none" or "missing"`);
+    if (typeof config.updateSnapshots !== 'string' || !['all', 'changed', 'missing', 'none'].includes(config.updateSnapshots))
+      throw errorWithFile(file, `config.updateSnapshots must be one of "all", "changed", "missing" or "none"`);
   }
 
   if ('workers' in config && config.workers !== undefined) {

packages/playwright/src/matchers/toMatchAriaSnapshot.ts

@@ -30,12 +30,13 @@ import path from 'path';
 type ToMatchAriaSnapshotExpected = {
   name?: string;
   path?: string;
+  timeout?: number;
 } | string;
 
 export async function toMatchAriaSnapshot(
   this: ExpectMatcherState,
   receiver: LocatorEx,
-  expectedParam: ToMatchAriaSnapshotExpected,
+  expectedParam?: ToMatchAriaSnapshotExpected,
   options: { timeout?: number } = {},
 ): Promise<MatcherResult<string | RegExp, string>> {
   const matcherName = 'toMatchAriaSnapshot';
@@ -55,9 +56,11 @@ export async function toMatchAriaSnapshot(
   };
 
   let expected: string;
+  let timeout: number;
   let expectedPath: string | undefined;
   if (isString(expectedParam)) {
     expected = expectedParam;
+    timeout = options.timeout ?? this.timeout;
   } else {
     if (expectedParam?.name) {
       expectedPath = testInfo.snapshotPath(sanitizeFilePathBeforeExtension(expectedParam.name));
@@ -71,6 +74,7 @@ export async function toMatchAriaSnapshot(
       expectedPath = testInfo.snapshotPath(sanitizeForFilePath(trimLongString(fullTitleWithoutSpec)) + '.yml');
     }
     expected = await fs.promises.readFile(expectedPath, 'utf8').catch(() => '');
+    timeout = expectedParam?.timeout ?? this.timeout;
   }
 
   const generateMissingBaseline = updateSnapshots === 'missing' && !expected;
@@ -84,7 +88,6 @@ export async function toMatchAriaSnapshot(
     }
   }
 
-  const timeout = options.timeout ?? this.timeout;
   expected = unshift(expected);
   const { matches: pass, received, log, timedOut } = await receiver._expect('to.match.aria', { expectedValue: expected, isNot: this.isNot, timeout });
   const typedReceived = received as MatcherReceived | typeof kNoElementsFoundError;

packages/playwright/src/program.ts

@@ -286,7 +286,7 @@ function overridesFromOptions(options: { [key: string]: any }): ConfigCLIOverrid
   if (['all', 'changed', 'missing', 'none'].includes(options.updateSnapshots))
     updateSnapshots = options.updateSnapshots;
   else
-    updateSnapshots = 'updateSnapshots' in options ? 'changed' : 'missing';
+    updateSnapshots = 'updateSnapshots' in options ? 'changed' : undefined;
 
   const overrides: ConfigCLIOverrides = {
     forbidOnly: options.forbidOnly ? true : undefined,

tests/playwright-test/aria-snapshot-file.spec.ts

@@ -152,3 +152,60 @@ test('should generate snapshot name', async ({ runInlineTest }, testInfo) => {
   const snapshot2 = await fs.promises.readFile(testInfo.outputPath('__snapshots__/a.spec.ts/test-name-2.yml'), 'utf8');
   expect(snapshot2).toBe('- heading "hello world 2" [level=1]');
 });
+
+for (const updateSnapshots of ['all', 'changed', 'missing', 'none']) {
+  test(`should update snapshot with the update-snapshots=${updateSnapshots} (config)`, async ({ runInlineTest }, testInfo) => {
+    const result = await runInlineTest({
+      'playwright.config.ts': `
+        export default {
+          snapshotPathTemplate: '__snapshots__/{testFilePath}/{arg}{ext}',
+          updateSnapshots: '${updateSnapshots}',
+        };
+      `,
+      'a.spec.ts': `
+        import { test, expect } from '@playwright/test';
+        test('test', async ({ page }) => {
+          await page.setContent(\`<h1>New content</h1>\`);
+          await expect(page.locator('body')).toMatchAriaSnapshot({ timeout: 1 });
+        });
+      `,
+      '__snapshots__/a.spec.ts/test-1.yml': '- heading "Old content" [level=1]',
+    });
+
+    const rebase = updateSnapshots === 'all' || updateSnapshots === 'changed';
+    expect(result.exitCode).toBe(rebase ? 0 : 1);
+    if (rebase) {
+      const snapshotOutputPath = testInfo.outputPath('__snapshots__/a.spec.ts/test-1.yml');
+      expect(result.output).toContain(`A snapshot is generated at`);
+      const data = fs.readFileSync(snapshotOutputPath);
+      expect(data.toString()).toBe('- heading "New content" [level=1]');
+    } else {
+      expect(result.output).toContain(`expect.toMatchAriaSnapshot`);
+    }
+  });
+}
+
+test('should respect timeout', async ({ runInlineTest }, testInfo) => {
+  const result = await runInlineTest({
+    'playwright.config.ts': `
+      export default {
+        snapshotPathTemplate: '__snapshots__/{testFilePath}/{arg}{ext}',
+      };
+    `,
+    'test.yml': `
+      - heading "hello world"
+    `,
+    'a.spec.ts': `
+      import { test, expect } from '@playwright/test';
+      import path from 'path';
+      test('test', async ({ page }) => {
+        await page.setContent(\`<h1>hello world</h1>\`);
+        await expect(page.locator('body')).toMatchAriaSnapshot({ timeout: 1 });
+      });
+    `,
+    '__snapshots__/a.spec.ts/test-1.yml': '- heading "new world" [level=1]',
+  });
+
+  expect(result.exitCode).toBe(1);
+  expect(result.output).toContain(`Timed out 1ms waiting for`);
+});

tests/playwright-test/golden.spec.ts

@@ -341,6 +341,36 @@ test('should update snapshot with the update-snapshots flag', async ({ runInline
   expect(data.toString()).toBe(ACTUAL_SNAPSHOT);
 });
 
+for (const updateSnapshots of ['all', 'changed', 'missing', 'none']) {
+  test(`should update snapshot with the update-snapshots=${updateSnapshots} (config)`, async ({ runInlineTest }, testInfo) => {
+    const result = await runInlineTest({
+      'playwright.config.ts': `export default { updateSnapshots: '${updateSnapshots}' };`,
+      ...files,
+      'a.spec.js-snapshots/snapshot.txt': 'Hello world',
+      'a.spec.js': `
+        const { test, expect } = require('./helper');
+        test('is a test', ({}) => {
+          expect('Hello world updated').toMatchSnapshot('snapshot.txt');
+        });
+      `
+    });
+
+    const rebase = updateSnapshots === 'all' || updateSnapshots === 'changed';
+    expect(result.exitCode).toBe(rebase ? 0 : 1);
+    if (rebase) {
+      const snapshotOutputPath = testInfo.outputPath('a.spec.js-snapshots/snapshot.txt');
+      if (updateSnapshots === 'all')
+        expect(result.output).toContain(`${snapshotOutputPath} is not the same, writing actual.`);
+      if (updateSnapshots === 'changed')
+        expect(result.output).toContain(`${snapshotOutputPath} does not match, writing actual.`);
+      const data = fs.readFileSync(snapshotOutputPath);
+      expect(data.toString()).toBe('Hello world updated');
+    } else {
+      expect(result.output).toContain(`toMatchSnapshot`);
+    }
+  });
+}
+
 test('should ignore text snapshot with the ignore-snapshots flag', async ({ runInlineTest }, testInfo) => {
   const EXPECTED_SNAPSHOT = 'Hello world';
   const ACTUAL_SNAPSHOT = 'Hello world updated';
@@ -1140,3 +1170,25 @@ test('should throw if a Promise was passed to toMatchSnapshot', async ({ runInli
   expect(result.exitCode).toBe(0);
   expect(result.passed).toBe(1);
 });
+
+
+test('should respect update snapshot option from config', async ({ runInlineTest }, testInfo) => {
+  const EXPECTED_SNAPSHOT = 'Hello world';
+  const ACTUAL_SNAPSHOT = 'Hello world updated';
+  const result = await runInlineTest({
+    ...files,
+    'a.spec.js-snapshots/snapshot.txt': EXPECTED_SNAPSHOT,
+    'a.spec.js': `
+      const { test, expect } = require('./helper');
+      test('is a test', ({}) => {
+        expect('${ACTUAL_SNAPSHOT}').toMatchSnapshot('snapshot.txt');
+      });
+    `
+  }, { 'update-snapshots': true });
+
+  expect(result.exitCode).toBe(0);
+  const snapshotOutputPath = testInfo.outputPath('a.spec.js-snapshots/snapshot.txt');
+  expect(result.output).toContain(`${snapshotOutputPath} does not match, writing actual.`);
+  const data = fs.readFileSync(snapshotOutputPath);
+  expect(data.toString()).toBe(ACTUAL_SNAPSHOT);
+});