Skip to content

Commit

Permalink
[Snapshot testing] Improve test consistency (#1103)
Browse files Browse the repository at this point in the history
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 `headless` mode 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 in `headless` mode, my interpretation is
that UI failures are due to excessive system resource consumption by the
Playwright UI causing inconsistent page rendering results.

## Changes
- Reduced the suite-wide config's `maxDiffPixels` from `25` to `0`,
allowing us to catch even the smallest unexpected UI change
- Added `snapshotModifications.css` to apply snapshot-testing-specific
style modifications. This approach allows us to replace `dotted` borders
with `solid` borders on all links to ensure consistent rendering. Since
the `dotted` style 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).
- Added the page header's USA flag to the suite-wide `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.
- <img width="528" alt="Screenshot 2025-01-09 at 11 36 49 AM"
src="https://github.com/user-attachments/assets/818d0c96-699c-4aa7-a43d-ba5aba3031a0"
/>


## How to test this PR

1. Checkout this branch and `yarn start`
1. `yarn test:e2e:snapshot --headless`
2. Ensure all tests pass

## Notes - Alternate approaches not chosen
- I did experiment with masking the entire page footer, since that was
the most frequent trigger of failures due to dotted border issues, but
still encountered failures due to links within the page content, so that
didn't feel like the most helpful approach.
- I considered breaking full-page tests into smaller element tests, but
found that with the masking of `.u-usa-flag` and modifying the link
border style, we were able to avoid the effort needed to significantly
refactor how these tests are implemented.

## Screenshots

![screencapture-localhost-9323-2025-01-09-11_33_55](https://github.com/user-attachments/assets/be36de36-78b5-4bd4-a16d-ff2b2f60c14f)
  • Loading branch information
meissadia authored Feb 7, 2025
1 parent 89c5343 commit d0ad6f3
Show file tree
Hide file tree
Showing 68 changed files with 22 additions and 5 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
clickContinueNext,
} from '../../../utils/navigation.utils';
import { checkSnapshot } from '../../../utils/snapshotTesting';
import { TIMEOUT_XS } from '../../../utils/timeoutConstants';
import { ResultUploadMessage, uploadFile } from '../../../utils/uploadFile';
import { verifyDownloadableReport } from '../../../utils/verifyDownloadableReport';

Expand Down Expand Up @@ -70,6 +71,7 @@ test('Resolve Errors (Logic)', async ({ page, navigateToUploadFile }) => {
await expect(page3row1.getByRole('cell').nth(0)).toHaveText('62');
await expect(page3row1.getByRole('cell').nth(2)).toHaveText('999');
await expect(page3row1.getByRole('cell').nth(3)).toHaveText('1');
await page.waitForTimeout(2 * TIMEOUT_XS);
await checkSnapshot(page);
};

Expand Down
8 changes: 8 additions & 0 deletions e2e/utils/snapshotModifications.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
a {
/*
Dotted borders are inconsistently rendered, causing test flake.
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;
}
13 changes: 11 additions & 2 deletions e2e/utils/snapshotTesting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,17 @@ import { expect } from '@playwright/test';
export const checkSnapshot = async (page: Page, options = {}) => {
if (process.env.SBL_ENABLE_PLAYWRIGHT_SNAPSHOT_TESTING === 'true') {
await expect(page).toHaveScreenshot({
fullPage: true, // Verify the full page
mask: [page.locator('.snapshot-ignore')], // Ignore flagged areas
// Verify the full page
fullPage: true,
mask: [
// Ignore flagged areas
page.locator('.snapshot-ignore'),
// USA flag in header is flaky in very tall snapshots
page.locator('.u-usa-flag'),
],
// CSS modifications to reduce flake (see CSS file for details)
stylePath: 'e2e/utils/snapshotModifications.css',
// Any option overrides provided on a test-by-test basis
...options,
});
}
Expand Down
4 changes: 1 addition & 3 deletions playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,7 @@ const config: PlaywrightTestConfig = {
timeout: TIMEOUT_EXPECT,
/* Snapshot testing */
toHaveScreenshot: {
// High enough to ignore dotted border flakiness
// but low enough to catch meaningful diffs??
maxDiffPixels: 25,
maxDiffPixels: 0,
},
},
/* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */
Expand Down

0 comments on commit d0ad6f3

Please sign in to comment.