Skip to content

Commit ca18554

Browse files
authored
Fix flaky snapshot-testing (#4367)
* Fix flaky snapshot-testing Signed-off-by: Matteo Collina <hello@matteocollina.com> * moar logging Signed-off-by: Matteo Collina <hello@matteocollina.com> * fixup Signed-off-by: Matteo Collina <hello@matteocollina.com> * fixup Signed-off-by: Matteo Collina <hello@matteocollina.com> * fixup Signed-off-by: Matteo Collina <hello@matteocollina.com> --------- Signed-off-by: Matteo Collina <hello@matteocollina.com>
1 parent 8bbf77c commit ca18554

2 files changed

Lines changed: 60 additions & 31 deletions

File tree

lib/mock/snapshot-recorder.js

Lines changed: 47 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,13 @@ const { writeFile, readFile, mkdir } = require('node:fs/promises')
44
const { dirname, resolve } = require('node:path')
55
const { InvalidArgumentError, UndiciError } = require('../core/errors')
66

7+
let crypto
8+
try {
9+
crypto = require('node:crypto')
10+
} catch {
11+
// Fallback if crypto is not available
12+
}
13+
714
/**
815
* Formats a request for consistent snapshot storage
916
* Caches normalized headers to avoid repeated processing
@@ -137,15 +144,45 @@ function normalizeHeaders (headers) {
137144

138145
/**
139146
* Creates a hash key for request matching
147+
* Properly orders headers to avoid conflicts and uses crypto hashing when available
140148
*/
141149
function createRequestHash (request) {
142150
const parts = [
143151
request.method,
144-
request.url,
145-
JSON.stringify(request.headers, Object.keys(request.headers).sort()),
146-
request.body || ''
152+
request.url
147153
]
148-
return Buffer.from(parts.join('|')).toString('base64url')
154+
155+
// Process headers in a deterministic way to avoid conflicts
156+
if (request.headers && typeof request.headers === 'object') {
157+
const headerKeys = Object.keys(request.headers).sort()
158+
for (const key of headerKeys) {
159+
const lowerKey = key.toLowerCase()
160+
const values = Array.isArray(request.headers[key])
161+
? request.headers[key]
162+
: [request.headers[key]]
163+
164+
// Add header name
165+
parts.push(lowerKey)
166+
167+
// Add all values for this header, sorted for consistency
168+
for (const value of values.sort()) {
169+
parts.push(String(value))
170+
}
171+
}
172+
}
173+
174+
// Add body
175+
parts.push(request.body || '')
176+
177+
const content = parts.join('|')
178+
179+
// Use crypto hash if available for better collision resistance
180+
if (crypto && crypto.createHash) {
181+
return crypto.createHash('sha256').update(content, 'utf8').digest('base64url')
182+
}
183+
184+
// Fallback to base64 encoding
185+
return Buffer.from(content).toString('base64url')
149186
}
150187

151188
/**
@@ -291,30 +328,14 @@ class SnapshotRecorder {
291328
if (!snapshot) return undefined
292329

293330
// Handle sequential responses
294-
if (snapshot.responses && Array.isArray(snapshot.responses)) {
295-
const currentCallCount = snapshot.callCount || 0
296-
const responseIndex = Math.min(currentCallCount, snapshot.responses.length - 1)
297-
snapshot.callCount = currentCallCount + 1
298-
299-
return {
300-
...snapshot,
301-
response: snapshot.responses[responseIndex]
302-
}
303-
}
331+
const currentCallCount = snapshot.callCount || 0
332+
const responseIndex = Math.min(currentCallCount, snapshot.responses.length - 1)
333+
snapshot.callCount = currentCallCount + 1
304334

305-
// Legacy format compatibility - convert single response to array format
306-
if (snapshot.response && !snapshot.responses) {
307-
snapshot.responses = [snapshot.response]
308-
snapshot.callCount = 1
309-
delete snapshot.response
310-
311-
return {
312-
...snapshot,
313-
response: snapshot.responses[0]
314-
}
335+
return {
336+
...snapshot,
337+
response: snapshot.responses[responseIndex]
315338
}
316-
317-
return snapshot
318339
}
319340

320341
/**

test/snapshot-testing.js

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -347,9 +347,18 @@ describe('SnapshotAgent - Request Handling', () => {
347347
setGlobalDispatcher(recordingAgent)
348348

349349
// Make multiple requests to record sequential responses
350-
await request(`${origin}/api/test`)
351-
await request(`${origin}/api/test`)
352-
await request(`${origin}/api/test`)
350+
{
351+
const res = await request(`${origin}/api/test`)
352+
await res.body.text()
353+
}
354+
{
355+
const res = await request(`${origin}/api/test`)
356+
await res.body.text()
357+
}
358+
{
359+
const res = await request(`${origin}/api/test`)
360+
await res.body.text()
361+
}
353362

354363
// Ensure all recordings are saved and verify the recording state
355364
await recordingAgent.saveSnapshots()
@@ -370,6 +379,7 @@ describe('SnapshotAgent - Request Handling', () => {
370379
})
371380

372381
setupCleanup(t, { agent: playbackAgent })
382+
setGlobalDispatcher(playbackAgent)
373383

374384
// Ensure snapshots are loaded and call counts are reset before setting dispatcher
375385
await playbackAgent.loadSnapshots()
@@ -385,8 +395,6 @@ describe('SnapshotAgent - Request Handling', () => {
385395
assert.strictEqual(snapshots.length, 1, 'Should have exactly one snapshot')
386396
assert.strictEqual(snapshots[0].responses.length, 3, 'Should have three sequential responses')
387397

388-
setGlobalDispatcher(playbackAgent)
389-
390398
// Test sequential responses
391399
const response1 = await request(`${origin}/api/test`)
392400
const body1 = await response1.body.text()

0 commit comments

Comments
 (0)