Add SnapshotFormat: audio file vs MD5 checksum#12
Merged
Conversation
70f8b43 to
262b79a
Compare
For repositories with many audio snapshot tests, .caf files can bloat the repo. This adds a SnapshotFormat enum (.audio / .checksum) so users can opt into lightweight 32-byte .md5 checksum files instead of full ALAC audio snapshots. Default is .audio, preserving full backward compatibility.
262b79a to
f46422e
Compare
There was a problem hiding this comment.
Pull request overview
Adds an opt-in snapshot artifact format that stores lightweight MD5 checksum files instead of full ALAC .caf audio snapshots, while keeping .audio as the default for backward compatibility.
Changes:
- Introduce
SnapshotFormat(.audio/.checksum) and thread it throughAudioSnapshotTrait. - Implement checksum-mode recording/verifying by hashing a temporary
.cafand storing/reading.md5artifacts. - Add checksum-focused tests and commit expected
.md5snapshot fixtures (including multi-buffer indexed naming).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/AudioSnapshotTestingTests/AudioSnapshots/AudioSnapshotTestingTests/checksumRoundTrip.440hz.md5 | Adds expected checksum snapshot fixture for round-trip test. |
| Tests/AudioSnapshotTestingTests/AudioSnapshots/AudioSnapshotTestingTests/checksumMultiBuffer.1.md5 | Adds expected checksum snapshot fixture for multi-buffer test (buffer 1). |
| Tests/AudioSnapshotTestingTests/AudioSnapshots/AudioSnapshotTestingTests/checksumMultiBuffer.2.md5 | Adds expected checksum snapshot fixture for multi-buffer test (buffer 2). |
| Tests/AudioSnapshotTestingTests/AudioSnapshotTestingTests.swift | Adds new tests that run snapshotting in .checksum format. |
| Sources/AudioSnapshotTesting/Internal/AudioChecksumWriter.swift | New helper for computing/writing/reading MD5 checksum artifacts. |
| Sources/AudioSnapshotTesting/Core/SnapshotFormat.swift | New public enum that defines supported snapshot artifact formats. |
| Sources/AudioSnapshotTesting/Core/AudioSnapshotTrait.swift | Adds format option to the trait and trait factory API. |
| Sources/AudioSnapshotTesting/Core/AudioSnapshotTesting.swift | Implements .checksum behavior for snapshot pathing, recording, and verification. |
Comments suppressed due to low confidence (3)
Sources/AudioSnapshotTesting/Internal/AudioChecksumWriter.swift:12
computeChecksum(of:)loads the entire audio file into memory viaData(contentsOf:). Snapshot audio can be non-trivial in size, so this can cause unnecessary memory spikes (and slower hashing). Consider streaming the file withFileHandleand updating anInsecure.MD5digest incrementally (or at least using mapped reads) to keep memory usage bounded.
let data = try Data(contentsOf: url)
let digest = Insecure.MD5.hash(data: data)
return digest.map { String(format: "%02x", $0) }.joined()
Sources/AudioSnapshotTesting/Core/AudioSnapshotTesting.swift:155
- In
.checksummode, this writes a temporary.caffile to compute the hash but never removes it. Over time (especially across many tests/runs), the temp directory will accumulate snapshot audio artifacts. Consider deletingtempPathafter the checksum is computed/written (e.g.,defer { try? FileManager.default.removeItem(at: tempPath) }).
case .checksum:
let tempPath = context.temporaryAudioPath(index: index, count: bufferCount)
try AudioFileWriter.write(buffer: buffers[index], to: tempPath, bitDepth: context.trait.bitDepth)
let checksum = try AudioChecksumWriter.computeChecksum(of: tempPath)
try AudioChecksumWriter.writeChecksum(checksum, to: path)
Sources/AudioSnapshotTesting/Core/AudioSnapshotTesting.swift:205
- In
.checksummode, verification writes a temporary.caffor each buffer and leaves it behind after hashing. Consider cleaning uptempPathafter computing the checksum to avoid leaving large temp files on disk (and to reduce the chance of interference between runs).
let tempPath = context.temporaryAudioPath(index: index, count: buffers.count)
try AudioFileWriter.write(buffer: buffer, to: tempPath, bitDepth: context.trait.bitDepth)
let actualChecksum = try AudioChecksumWriter.computeChecksum(of: tempPath)
let expectedChecksum = try AudioChecksumWriter.readChecksum(from: path)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or 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
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.
Summary
SnapshotFormatenum (.audio/.checksum) so users can choose between full ALAC.cafsnapshots (current behavior) or lightweight.md5checksum files (~32 bytes each).audio, preserving full backward compatibility — no changes needed for existing testsTest plan
checksumRoundTriptest: record + verify round-trip with.checksumformatchecksumMultiBuffertest: multi-buffer checksum with indexed naming (.1.md5,.2.md5).md5files contain 32-character hex checksums