Skip to content

Commit 71a616e

Browse files
macariemacarie
committed
chore: apply review suggestions
1 parent cffe8cf commit 71a616e

File tree

2 files changed

+83
-63
lines changed

2 files changed

+83
-63
lines changed

‎test/browser/fixtures/expect-dom/toMatchScreenshot.test.ts‎

Lines changed: 81 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,16 @@ const renderTestCase = (colors: readonly [string, string, string]) =>
1616
</div>
1717
`)
1818

19+
/**
20+
* ## Screenshot Testing Strategy
21+
*
22+
* Tests create reference screenshots on-the-fly on demand, then compare
23+
* against them. References are cleaned up after each test.
24+
*
25+
* Screenshot references are unstable across environments (headless vs UI mode,
26+
* different operating systems, different browsers). Storing references for
27+
* every environment combination would create a maintenance burden.
28+
*/
1929
describe('.toMatchScreenshot', () => {
2030
test('compares screenshots correctly', async ({ onTestFinished }) => {
2131
const filename = globalThis.crypto.randomUUID()
@@ -37,93 +47,103 @@ describe('.toMatchScreenshot', () => {
3747

3848
const locator = page.getByTestId(dataTestId)
3949

50+
// Create a reference screenshot by explicitly saving one
4051
await locator.screenshot({
4152
save: true,
4253
path,
4354
})
4455

56+
// Test that `toMatchScreenshot()` correctly finds and compares against
57+
// this reference; since the element hasn't changed, it should match
4558
await expect(locator).toMatchScreenshot(filename)
4659
})
4760

48-
test("throws when screenshots don't match", async ({ onTestFinished }) => {
49-
const filename = globalThis.crypto.randomUUID()
50-
const path = join(
51-
'__screenshots__',
52-
'toMatchScreenshot.test.ts',
53-
`${filename}-${server.browser}-${server.platform}.png`,
54-
)
61+
// Only run this test if snapshots aren't being updated
62+
test.runIf(server.config.snapshotOptions.updateSnapshot !== 'all')(
63+
"throws when screenshots don't match",
64+
async ({ onTestFinished }) => {
65+
const filename = globalThis.crypto.randomUUID()
66+
const path = join(
67+
'__screenshots__',
68+
'toMatchScreenshot.test.ts',
69+
`${filename}-${server.browser}-${server.platform}.png`,
70+
)
5571

56-
onTestFinished(async () => {
57-
await server.commands.removeFile(path)
58-
})
72+
onTestFinished(async () => {
73+
await server.commands.removeFile(path)
74+
})
5975

60-
renderTestCase([
61-
'oklch(39.6% 0.141 25.723)',
62-
'oklch(40.5% 0.101 131.063)',
63-
'oklch(37.9% 0.146 265.522)',
64-
])
76+
// Create reference with first color set
77+
renderTestCase([
78+
'oklch(39.6% 0.141 25.723)',
79+
'oklch(40.5% 0.101 131.063)',
80+
'oklch(37.9% 0.146 265.522)',
81+
])
6582

66-
const locator = page.getByTestId(dataTestId)
83+
const locator = page.getByTestId(dataTestId)
6784

68-
await locator.screenshot({
69-
save: true,
70-
path,
71-
})
85+
await locator.screenshot({
86+
save: true,
87+
path,
88+
})
7289

73-
renderTestCase([
74-
'oklch(84.1% 0.238 128.85)',
75-
'oklch(84.1% 0.238 128.85)',
76-
'oklch(84.1% 0.238 128.85)',
77-
])
90+
// Change to different colors - this should cause comparison to fail
91+
renderTestCase([
92+
'oklch(84.1% 0.238 128.85)',
93+
'oklch(84.1% 0.238 128.85)',
94+
'oklch(84.1% 0.238 128.85)',
95+
])
7896

79-
let errorMessage: string
97+
let errorMessage: string
8098

81-
try {
82-
await expect(locator).toMatchScreenshot(filename)
83-
} catch (error) {
84-
errorMessage = error.message
85-
}
99+
try {
100+
await expect(locator).toMatchScreenshot(filename)
101+
} catch (error) {
102+
errorMessage = error.message
103+
}
86104

87-
const [referencePath, actualPath, diffPath] = extractToMatchScreenshotPaths(
88-
errorMessage,
89-
filename,
90-
)
105+
const [referencePath, actualPath, diffPath] = extractToMatchScreenshotPaths(
106+
errorMessage,
107+
filename,
108+
)
91109

92-
expect(referencePath).toMatch(new RegExp(`${path}$`))
93-
expect(typeof actualPath).toBe('string')
94-
expect(typeof diffPath).toBe('string')
110+
expect(referencePath).toMatch(new RegExp(`${path}$`))
111+
expect(typeof actualPath).toBe('string')
112+
expect(typeof diffPath).toBe('string')
95113

96-
onTestFinished(async () => {
97-
await Promise.all([
98-
server.commands.removeFile(actualPath),
99-
server.commands.removeFile(diffPath),
100-
])
101-
})
114+
onTestFinished(async () => {
115+
await Promise.all([
116+
server.commands.removeFile(actualPath),
117+
server.commands.removeFile(diffPath),
118+
])
119+
})
102120

103-
const { pixels, ratio } =
104-
/(?<pixels>\d+).*?ratio (?<ratio>[01]\.\d{2})/.exec(errorMessage)
105-
?.groups ?? {}
121+
const { pixels, ratio } =
122+
/(?<pixels>\d+).*?ratio (?<ratio>[01]\.\d{2})/.exec(errorMessage)
123+
?.groups ?? {}
106124

107-
expect(pixels).toMatch(/\d+/)
108-
expect(ratio).toMatch(/[01]\.\d{2}/)
125+
expect(pixels).toMatch(/\d+/)
126+
expect(ratio).toMatch(/[01]\.\d{2}/)
109127

110-
expect(errorMessage).toMatchInlineSnapshot(`
111-
expect(element).toMatchScreenshot()
128+
expect(errorMessage).toMatchInlineSnapshot(`
129+
expect(element).toMatchScreenshot()
112130
113-
Screenshot does not match the stored reference.
114-
${pixels} pixels (ratio ${ratio}) differ.
131+
Screenshot does not match the stored reference.
132+
${pixels} pixels (ratio ${ratio}) differ.
115133
116-
Reference screenshot:
117-
${referencePath}
134+
Reference screenshot:
135+
${referencePath}
118136
119-
Actual screenshot:
120-
${actualPath}
137+
Actual screenshot:
138+
${actualPath}
121139
122-
Diff image:
123-
${diffPath}
124-
`)
125-
})
140+
Diff image:
141+
${diffPath}
142+
`)
143+
},
144+
)
126145

146+
// Only run this test if snapshots aren't being updated
127147
test.runIf(server.config.snapshotOptions.updateSnapshot !== 'all')(
128148
'throws when creating a screenshot for the first time',
129149
async ({

‎test/test-utils/index.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,10 +208,10 @@ async function runCli(command: 'vitest' | 'vite-node', _options?: CliOptions | s
208208

209209
if (args[0] !== 'list' && (args.includes('--watch') || args[0] === 'watch')) {
210210
if (command === 'vitest') {
211-
// Wait for initial test run to complete
211+
// Waiting for either success or failure
212212
await Promise.race([
213213
cli.waitForStdout('Waiting for file changes'),
214-
cli.waitForStdout('Watching for file changes'),
214+
cli.waitForStdout('Tests failed. Watching for file changes'),
215215
])
216216
}
217217
// make sure watcher is ready

0 commit comments

Comments
 (0)