chromaui / chromatic-e2e

Archive end-to-end tests to be replayed in Storybook and Chromatic
MIT License
21 stars 4 forks source link

Playwright: Snapshots stored outside of `testInfo.attachments` #156

Closed skitterm closed 3 months ago

skitterm commented 3 months ago

Issue: #AP-4453

What Changed

We now use our custom object (instead of Playwright's testInfo.attachments) to store the snapshotted DOM. This is so this DOM won't be captured in reporters, which has caused out-of-memory issues in the JSON reporter, for example.

Before

When takeSnapshot() was called (either automatically at the end of the test, or manually by the user in their spec file), we'd call Playwright's testInfo.attach() to store the DOM snapshot as an attachment. This way we were able to access the snapshots later when writing them to disk.

However, this meant that the DOM snapshots (typically very large in size) were now being included in Playwright's reporters. For the JSON reporter, this ballooned the size of the reporter's output file to the point where Playwright was unable to generated the report file (Playwright still finished running the tests).

Now

Instead of putting the DOM snapshots on Playwright's testInfo object as attachments, we write them to our own in-memory object (chromaticSnapshots), with each test's snapshot(s) being written based on the test's ID. We then read from chromaticSnapshots to get the DOM snapshots to write to disk.

We also delete the snapshots off the chromaticSnapshots object after the test has completed to avoid that object getting massive.

This avoids the DOM snapshots getting written to the Playwright reporters.

How to test

  1. Ensure all Chromatic playwright snapshots (UI Tests: playwright-e2e PR status check) are unchanged
  2. Clone this repo and follow the verification steps there (ensures that reporter file is created with this fix, even for large numbers of large tests)
codacy-production[bot] commented 3 months ago

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
:white_check_mark: -0.94% (target: -1.00%) :white_check_mark: 92.00% (target: 80.00%)
Coverage variation details | | Coverable lines | Covered lines | Coverage | | ------------- | ------------- | ------------- | ------------- | | Common ancestor commit (c107353d7abd53037faca212ecc0e9f64c95fa83) | 224 | 214 | 95.54% | | | Head commit (04d307953e409f65e969c42f337566225c7d9741) | 259 (+35) | 245 (+31) | 94.59% (**-0.94%**) | **Coverage variation** is the difference between the coverage for the head and common ancestor commits of the pull request branch: ` - `
Diff coverage details | | Coverable lines | Covered lines | Diff coverage | | ------------- | ------------- | ------------- | ------------- | | Pull request (#156) | 25 | 23 | **92.00%** | **Diff coverage** is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: `/ * 100%`

See your quality gate settings    Change summary preferences


:rocket: Don’t miss a bit, follow what’s new on Codacy.

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

skitterm commented 3 months ago

@tevanoff this is ready for your QA now! I'm just writing up a few more tests in the meantime, then it'll be ready for your re-review as well.

skitterm commented 3 months ago

@tevanoff I've simplified the takeSnapshot tests to take in a mocked page, and it's much smaller now. I may try to put in a couple more tests tomorrow morning in makeTest (just making sure we're still deleting chromaticSnapshots entries if the test fails or throws errors, for example), but those should just be following the pattern of the tests already here!