Repository navigation
test: screenshot baselines, so a fix in one screen cannot move another - #41
Merged
Merged
Conversation
…e moves it Every screen on both ends was already rendered off screen and written to a PNG, and nothing compared the result with anything. That is how a fix in the transfer screen became a new bug in the jobs screen, found later by whoever opened the window: the only reviewer was a person. Both screenshot tests now compare their rendering against a baseline that lives in the repository, and fail with the share of pixels that moved, the region they moved in, and three images — baseline, this run, diff — under build/screenshots-diff. A deliberate change is accepted with CUBSYNC_SCREENSHOTS=update. For the comparison to mean anything the picture has to be the same picture twice, so the clock, the file timestamps and the scratch directory are now pinned rather than read from the machine: a run's timestamps through the engine's existing UtcClock, a log line's through AppLog's, "3 分钟前" through Format.clock, and the two preview fixtures use a fixed path because a random one was drawn in the breadcrumb. 30 baselines: 13 for the desktop, filed per operating system because text rasterisation differs; 17 for the phone, one set because Robolectric brings its own fonts.
The phone's screenshots matched between two runs on Windows and moved by up to 27% under CI on Linux: Robolectric draws text through the host's rasteriser, so the picture is not portable any more than the desktop's is. Both ends now file their baselines per operating system, and a machine that has none for its own records one instead of failing — which is honest about what CI did and did not verify.
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.
Every screen on both ends was already rendered off screen. Nothing compared the result with anything, so the only reviewer was whoever opened the window next — which is how a round of fixes becomes a round of new bugs.
What this adds
ScreenshotBaseline(testing/src/jvmMain): compares a rendered PNG with the baseline in the repository. Fails with the share of pixels that moved and the region they moved in; writes baseline / actual / diff intobuild/screenshots-diff/<screen>/.CUBSYNC_SCREENSHOTS=updateaccepts the current rendering as the new baseline.docs/TESTING.md: how the check works and when to update.Why the fixtures changed
A baseline is only useful if the same screen renders the same picture twice. These were read from the machine and are now pinned:
SyncService(utcClock = ...), forwarded to the engine's existingUtcClockAppLog(now = ...)Format.clockAppState.now/AndroidAppState.nowAll of these default to the wall clock; the application never passes one.
Proof
outlineby 16 in one channel: 11 of 13 desktop screens failed, e.g.00-transfer: 0.62% of pixels moved (6860 of 1100800, region 1280x822 at 0,10). Reverted../gradlew check: 101 tasks, green.