[BUG] incorrect @playwright/test typings for toMatchSnapshot/toMatchInlineSnapshot/etc

[dbjorge]

Context:

• Playwright Version: 1.12.2
• @playwright/test version: 1.12.2
• Operating System: Windows 10 10.0.19043
• Node.js version: 14.17.0
• Browser: n/a
• Extra: n/a

Code Snippet

// repro.spec.ts
import { test, expect } from '@playwright/test'

test.describe('cases that typecheck successfully but fail at runtime', () => {
    test('toMatchInlineSnapshot()', () => {
        expect('some string').toMatchInlineSnapshot();
    });

    test('nullary toMatchSnapshot()', () => {
        expect('some string').toMatchSnapshot();
    });
});

> yarn init
> yarn add --dev playwright @playwright/test typescript
> npx tsc ./repro.spec.ts --esModuleInterop --noEmit
> $LASTEXITCODE
0
> npx playwright test

Running 2 tests using 1 worker

  x 1) repro.spec.ts:5:5 › [chromium] cases that typecheck successfully but fail at runtime toMatchInlineSnapshot() (6ms)
  x 2) repro.spec.ts:9:5 › [chromium] cases that typecheck successfully but fail at runtime nullary toMatchSnapshot() (8ms)


  1) repro.spec.ts:5:5 › [chromium] cases that typecheck successfully but fail at runtime toMatchInlineSnapshot()

    TypeError: (0 , _test.expect)(...).toMatchInlineSnapshot is not a function

      4 | test.describe('cases that typecheck successfully but fail at runtime', () => {
      5 |     test('toMatchInlineSnapshot()', () => {
    > 6 |         expect('some string').toMatchInlineSnapshot();
        |                               ^
      7 |     });
      8 |
      9 |     test('nullary toMatchSnapshot()', () => {

        at C:\repos\playwright-typing-snapshot-repro\repro.spec.ts:6:31
        at FixtureRunner.resolveParametersAndRunHookOrTest (C:\repos\playwright-typing-snapshot-repro\node_modules\@playwright\test\lib\test\fixtures.js:241:16)
        at WorkerRunner._runTestWithBeforeHooks (C:\repos\playwright-typing-snapshot-repro\node_modules\@playwright\test\lib\test\workerRunner.js:290:39)
        at processTicksAndRejections (internal/process/task_queues.js:95:5)

  2) repro.spec.ts:9:5 › [chromium] cases that typecheck successfully but fail at runtime nullary toMatchSnapshot()

    Error: toMatchSnapshot() requires a "name" parameter

       8 |
       9 |     test('nullary toMatchSnapshot()', () => {
    > 10 |         expect('some string').toMatchSnapshot();
         |                               ^
      11 |     });
      12 | });
      13 |

        at Object.toMatchSnapshot (C:\repos\playwright-typing-snapshot-repro\node_modules\@playwright\test\lib\test\expect.js:36:15)
        at __EXTERNAL_MATCHER_TRAP__ (C:\repos\playwright-typing-snapshot-repro\node_modules\expect\build\index.js:342:30)
        at Object.throwingMatcher (C:\repos\playwright-typing-snapshot-repro\node_modules\expect\build\index.js:343:15)
        at C:\repos\playwright-typing-snapshot-repro\repro.spec.ts:10:31
        at FixtureRunner.resolveParametersAndRunHookOrTest (C:\repos\playwright-typing-snapshot-repro\node_modules\@playwright\test\lib\test\fixtures.js:241:16)
        at WorkerRunner._runTestWithBeforeHooks (C:\repos\playwright-typing-snapshot-repro\node_modules\@playwright\test\lib\test\workerRunner.js:290:39)
        at processTicksAndRejections (internal/process/task_queues.js:95:5)


  2 failed
    repro.spec.ts:5:5 › [chromium] cases that typecheck successfully but fail at runtime toMatchInlineSnapshot()
    repro.spec.ts:9:5 › [chromium] cases that typecheck successfully but fail at runtime nullary toMatchSnapshot()

Describe the bug

The typings mismatch the actual implementation here - at minimum, the typings Playwright exports should match what's actually available.

But ideally, we'd prefer for the normal jest/expect behavior of toMatchSnapshot and toMatchInlineSnapshot to work per the original expect typings, in addition to Playwright exposing a way to do screenshot snapshots (rather than instead of it). If we were to use @playwright/test in most of our projects, it would involve migrating an existing corpus of Jest tests that make use of value snapshots, not just screenshot snapshots, so it'd be inconvenient to have to set up an alternative for them.

context: found while working on microsoft/axe-pipelines-samples#579

[mxschmitt]

Closing since this is fixed now on the master branch. You can try it out today via npm install @playwright/test@next.

[dbjorge]

Thanks for the quick resolution! @JoelEinbinder, I wanted to double check whether it was intended that the new typings of toMatchSnapshot as of #7292 don't allow for the "first param as a string" form that the implementation allows:

playwright/src/test/expect.ts

Lines 24 to 34 in 4c6deaa

	function toMatchSnapshot(received: Buffer | string, nameOrOptions: string | { name: string, threshold?: number }, optOptions: { threshold?: number } = {}) {
	let options: { name: string, threshold?: number };
	const testInfo = currentTestInfo();
	if (!testInfo)
	throw new Error(`toMatchSnapshot() must be called during the test`);
	if (typeof nameOrOptions === 'string')
	options = { name: nameOrOptions, ...optOptions };
	else
	options = { ...nameOrOptions };
	if (!options.name)
	throw new Error(`toMatchSnapshot() requires a "name" parameter`);

edit: it looks like the "Visual comparisons" docs are mostly using the string-param form, so probably not intentional

[JoelEinbinder]

@dbjorge did you try the patch? I wasn't able to reproduce this locally. The toMatchSnapshot(string) signature was not removed, you can expand the diff to see it there.

[dbjorge]

@JoelEinbinder, I'm sorry, my mistake! I missed that there was a separate overload underneath the modified typing for the string-arg form. I agree, it seems fine as is.