microsoft / playwright

Playwright is a framework for Web Testing and Automation. It allows testing Chromium, Firefox and WebKit with a single API.
https://playwright.dev
Apache License 2.0
63.8k stars 3.46k forks source link

[Bug]: Missing expected reference snapshot file name (PW does not pass the info to a reporter). #30693

Open chekrd opened 1 month ago

chekrd commented 1 month ago

Version

Started in 1.43.0

Steps to reproduce

  1. Clone a minimal repro repo https://github.com/chekrd/pw-snapshot-name
  2. Run npm ci.
  3. Be sure the ./snapshots directory is empty or deleted.
  4. Run npm run playwright (runs the test and fails).

Expected behavior

I have two versions of what to expect.

A. (preferred) Reference snapshots are generated with the same name as for test result attachments (without the -reference suffix, as it is now).

B. Reporters can access the expected reference snapshot file name through testResult or testCase. Right now, we can only parse the value from testCase.errors which is inconvenient and unstable as the error message can change and is not meant for further processing.

Actual behavior

After the step 4:

✅ The test failed because the reference snapshot is missing.

See the console output, it contains the testResult and testCase objects from a custom reporter ./reporters/screenComparisonReporter.ts.

What PW gives to our custom reporter through testResult.attachments:

[{
  name: 'Agenda-view-displays-empty-dac89-t-when-inventory-is-empty-1-actual.png',
  path: '/<your path>/pw-snapshot-name/test-results/src-AgendaView.uitest.tsx--2ce5e-ent-when-inventory-is-empty-chromium/Agenda-view-displays-empty-dac89-t-when-inventory-is-empty-1-actual.png',
  contentType: 'image/png',
  body: undefined
}],

✅ Test result attachment name is shortened ("dac89" part), which is correct because it exceeded 60 characters name length limit (covered in https://github.com/microsoft/playwright/issues/29719, released in v1.43).

❌ PW creates different snapshot reference name (Agenda-view-displays-empty-state-picture-instead-of-the-content-when-inventory-is-empty-1-chromium.png in this example, run npm run playwright-update-snapshots and see the ./snapshots content - the name is not shortened). Because of this, we can't know what reference snapshot is missing.

The only mention of the expected reference image is in testCase.errors:

message: "Error: A snapshot doesn't exist at /<your path>/pw-snapshot-name/snapshots/Agenda-view-displays-empty-state-picture-instead-of-the-content-when-inventory-is-empty-1-chromium.png, writing actual.",
      stack: "Error: A snapshot doesn't exist at /<your path>/pw-snapshot-name/snapshots/Agenda-view-displays-empty-state-picture-instead-of-the-content-when-inventory-is-empty-1-chromium.png, writing actual.\n" +
        '    at /<your path>/pw-snapshot-name/src/AgendaView.uitest.tsx:9:7',

But in previous PW versions...

For this example, changing the PW version back to 1.42 results in a testResult.attachment names we can work with. We know, that we can take "Agenda-view-displays-empty-state-picture-instead-of-the-content-when-inventory-is-empty-1" part, add a platform name and we have recreated the reference snapshot name PW expects in ./snapshots directory (Agenda-view-displays-empty-state-picture-instead-of-the-content-when-inventory-is-empty-1-chromium.png):

[{
  name: 'Agenda-view-displays-empty-state-picture-instead-of-the-content-when-inventory-is-empty-1-actual.png',
  path: '/<your path>/pw-snapshot-name/test-results/src-AgendaView.uitest.tsx-Agenda-view-displays-2ce5e--instead-of-the-content-when-inventory-is-empty-chromium/Agenda-view-displays-empty-state-picture-instead-of-the-content-when-inventory-is-empty-1-actual.png',
  contentType: 'image/png',
  body: undefined
}]

There is a chance that our approach is not right. But the test result attachments from PW@1.43+ are barely usable. Are there other ways to achieve it?

Additional context

PW snapshots taken in macOS and Windows do not match. We have mixed developer environments, so we decided to have a central point for generating PW snapshots that run only on Windows. Then we have a custom PW reporter for processing snapshot errors. It maps the PW test result output to our screen comparison service, in which developers can see the difference in a browser and resolve all issues (apply the -actual snapshot as a new source of truth or fix it in the code). But because PW test result attachment names do not match the PW snapshot generator, we cannot apply the new snapshot version if the reference is not yet created because we don't know what name PW expects in the ./snapshot directory. We need the information for renaming the -actual attachment to the snapshot reference and committing it to the ./snapshots in the repository.

Environment

System:
    OS: macOS 14.4.1
    CPU: (10) arm64 Apple M1 Pro
    Memory: 43.33 MB / 16.00 GB
  Binaries:
    Node: 22.1.0 - ~/.nvm/versions/node/v22.1.0/bin/node
    npm: 10.7.0 - ~/.nvm/versions/node/v22.1.0/bin/npm
  Languages:
    Bash: 3.2.57 - /bin/bash
  npmPackages:
    @playwright/experimental-ct-react: 1.44.0 => 1.44.0
    @playwright/test: 1.44.0 => 1.44.0
chekrd commented 1 month ago

💡 I just thought of traversing through the testCase object and re-creating our snapshotPathTemplate, maybe it can be a way out.

mxschmitt commented 1 month ago

Looks like you want something like https://github.com/microsoft/playwright/pull/24446#discussion_r1285606473.

Relates https://github.com/microsoft/playwright/issues/24310

chekrd commented 1 month ago

It is close, but not really. I don't want any change in UI mode. I want PW to provide the reporter with the name/filepath of the expected reference snapshot when PW informs about an error of missing reference snapshot. PW knows how it should be named, he is looking for it, that's why PW can say it is missing. You already have the name and everything, but it is not sent with the test result.

chekrd commented 2 weeks ago

The issue still blocks us from updating the PW.

I have tried to recreate the snapshot file name like this:

The test name part ✅:

function getFullTestName(test: TestCase | Suite): string {
  return test.parent?.type === 'describe'
    ? `${getFullTestName(test.parent)} ${test.title}`
    : test.title;
}

// Returns: "Agenda view displays empty state picture instead of the content when inventory is empty"

The platform part ✅: testCase.parent.project()?.name

❌ But I have no direct way to get the screenshot index number.

Could you please add a new property to result.attachments objects, containing the expected original snapshot filename? You already have it computed, just send it to the reporter pretty please.