[Snapshot testing] Improve test consistency #1103
Merged
+22
−5
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
meissadia
force-pushed
the
1099-e2e-mask-footer
branch
from
e871363 to
ecb8779
Compare
meissadia
requested review from
billhimmelsbach and
tanner-ricks
as code owners
billhimmelsbach
approved these changes
Feb 7, 2025
| Since this is a border style defined in the CFPB Design System, | ||
| we should be safe not validating this styling as part of our app testing. | ||
| */ | ||
| border-style: solid; |
There was a problem hiding this comment.
Good idea @meissadia!
| // High enough to ignore dotted border flakiness | ||
| // but low enough to catch meaningful diffs?? | ||
| maxDiffPixels: 25, | ||
| maxDiffPixels: 0, |
There was a problem hiding this comment.
This is incredible!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #1099
I explored a few options to reduce the flakiness of our snapshot tests and the approach implemented in this PR seems to provide the most reliable result with sensible tradeoffs.
Local test runs in
headlessmode have passed 100%, 6x in a row, with 4 workers. Snapshot testing via the UI can still be rather flaky but, since tests pass consistently inheadlessmode, my interpretation is that UI failures are due to excessive system resource consumption by the Playwright UI causing inconsistent page rendering results.Changes
maxDiffPixelsfrom25to0, allowing us to catch even the smallest unexpected UI changesnapshotModifications.cssto apply snapshot-testing-specific style modifications. This approach allows us to replacedottedborders withsolidborders on all links to ensure consistent rendering. Since thedottedstyle comes from the CFPB Design System, it does not seem necessary to verify it's implementation as part of our app (though it would certainly be preferred that these tests validate exactly what the user is expected to see).mask (ignore)list. This element tends to trigger false-positives on tests with very tall screenshots and, since it's implementation comes from the Design System, does not seem to be something we need to validate as part of our app. As a next step, we could break these taller pages down into smaller tests of subsections of the page, but for now this workaround seems acceptable.How to test this PR
yarn startyarn test:e2e:snapshot --headlessNotes - Alternate approaches not chosen
.u-usa-flagand modifying the link border style, we were able to avoid the effort needed to significantly refactor how these tests are implemented.Screenshots