Repository navigation
Keep one set of golden images - #80
Merged
Merged
Conversation
- Drop the platform suffix from golden file names. - Remove the environment lookup and its subprocess. - Ignore failure output written next to the golden images.
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.
Golden images were keyed by platform, as
grid.TestCrop-linux-alpine_amd64.golden.png. They are now justgrid.TestCrop.golden.png.The suffix was not earning its place
Comparing every pair that existed on both architectures:
The three that differ are
TestRotate90,TestRotate180, andTestRotate270, and they differ because the arm64 files are stale rather than because the architecture matters. #66 replacedSimilaritywithvips.Rotatefor quarter turns and only the amd64 baselines were regenerated.TestRotate180on arm64 is 6887 bytes, which is exactly the resampled size that change removed.So the split never recorded a real platform difference. What it did was let one side go stale without anything noticing, which is the opposite of what a golden image is for.
What the suffix cost
getEnvironmentranlsb_release -cs, or ash -csubshell reading/etc/os-releaseif that was missing, to build the key. That is gone along with theos/exec,runtime, andsyncimports it needed.It also made new tests awkward. Several were added on amd64 with no arm64 counterpart, since this machine has no arm64 emulation, and every one of those PRs carried a note asking for
-updateto be run elsewhere. Five tests are in that state right now:TestAutolevel,TestBrightness,TestResizeIgnoreAspectRatio,TestResizeIgnoreAspectRatioWidthOnly, andTestThumbnailIgnoreAspectRatio. Collapsing to one set resolves all of them.Which set was kept
The amd64 set, because it is complete at 21 files against 16, and because it is the one that reflects current behaviour for the rotate tests. The 16 arm64 files are removed.
If a platform difference ever does appear
CI runs these tests, so a regeneration that changes the bytes shows up as a failing check rather than as a silently divergent second set. That is a better signal than two sets where either may be stale.
.gitignoreinresources/goldennow covers*.failed.*, the diagnostic output written when a comparison fails.Verification
In
ghcr.io/beetlebugorg/go-dims:builder:gofmt,go vet, andgo test -race ./...all pass. Removing a golden file still fails loudly: