Refactor Snapshot and Generated Image Handling #892
Conversation
There was a problem hiding this comment.
I think that this file is now unnecessary
vello_tests/src/snapshot.rs
Outdated
| SnapshotDirectory::Smoke => "smoke_snapshots", | ||
| SnapshotDirectory::Lfs => "snapshots", | ||
| SnapshotDirectory::Smoke => "smoke", | ||
| SnapshotDirectory::Lfs => "lfs", |
There was a problem hiding this comment.
I think I would like to have vello_tests/snapshots be LFS files by default, and then have vello_tests/snapshots/smoke for the smoke tests. This matches our existing LFS setup, so no config changes are needed.
945fc2b to
32da0ba
Compare
|
I'll review on Monday. Just a passing thought that the main we thing we need to be sure of is that a |
There was a problem hiding this comment.
This looks good, except for the one thing about oversized images. However, two things are in our favour there:
- The test will never succeed if it generates an oversized file.
- The snapshot sizes are still checked on CI
I think the quickest course of action to resolve is to just re-add *.oversized.png to the snapshots/gitignore.
Edit: Sorry my timeline for reviewing was slightly off
This should have been changes in #892. I'm not overly happy with the difference between `vello_tests/current` and `vello_sparse_tests/diffs`, but that isn't for this PR to solve.
Refactor Snapshot and Generated Image Handling
This pull request changes how snapshots and currently generated images are handled in tests.
It moves
smoke_snapshotstosnapshots/smokeand originalsnapshotstosnapshots/lfs.Instead of generating images into the same directory as snapshots, it generates images into a separate directory
current.In comparison tests, it generates CPU/GPU tests in separate directories.
Example (Snapshot Tests)
Original layout:
smoke_snapshots/xxx.png(reference)smoke_snapshots/xxx.cpu.png(CPU generated)smoke_snapshots/xxx.gpu.png(GPU generated)New layout:
snapshots/smoke/xxx.png(reference)current/cpu/smoke/xxx.png(CPU generated)current/gpu/smoke/xxx.png(GPU generated)Example (Comparison Tests)
Original layout:
comparison/xxx.cpu.pngcomparison/xxx.gpu.pngNew layout:
comparison/cpu/xxx.pngcomparison/gpu/xxx.pngWhy?
The important property of this change is that files to be compared have the same filenames in the same nested directory structure. This allows for simple directory-to-directory comparisons for all tests at once. For example,
current/cpuvssnapshots, orcomparison/cpuvscomparison/gpu.This is a preparatory step for full Kompari integration. However, this layout allows immediate use of
kompari-clito generate diff reports.Note: This PR lefts
debug_outputsas it is.