Repository navigation
Fix the screenshot tests all photographing the same view - #695
Merged
Merged
Conversation
goneng
marked this pull request as ready for review
September 19, 2026 16:52
Contributor
|
Thank you for the pull request. One thing I found here, that the 3 new screenshots (the bottom 3 ones) |
Merged
Contributor
This #696 fixes it |
hannesa2
self-requested a review
September 19, 2026 18:18
hannesa2
force-pushed
the
capture-a-distinct-view-per-test
branch
from
September 19, 2026 18:47
0ecba1a to
99203a8
Compare
Contributor
|
I rebased (temporary) it to #696 (Detect new screenshots) to test the detection of new screenshots |
hannesa2
force-pushed
the
capture-a-distinct-view-per-test
branch
from
September 19, 2026 19:42
99203a8 to
7ca988c
Compare
Have each screenshot test reach its view and prove it arrived, so the suite covers three screens instead of one screen three times. - Switch views through the View menu, which is where the Stage view is reachable from. The toolbar button the staging test looked for has never existed, so that test always fell through and photographed the history view under the staging name. - Assert the destination is on screen before the shutter, so a missed switch fails the run instead of quietly yielding a duplicate. - Drop the capture from testMainWindowExists and keep it as a launch assertion, the main window being the history view already. - Select the history view in the context menu test too, since reaching the Stage view persists PBShowStageView and so decides what the next launch opens on.
Drop the discovery-order prefix from the extracted filenames, so that a test skipping its capture no longer renumbers every file after it and makes the next comparison report the whole set as changed. - Name each file after its attachment. - Add a numeric suffix only where two attachments share a name.
goneng
force-pushed
the
capture-a-distinct-view-per-test
branch
from
September 19, 2026 19:58
7ca988c to
ef43669
Compare
Contributor
|
Screenshot differs: emulatorApi=arm64 with 3 screenshot(s) |
This was referenced Sep 20, 2026
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.
Have each screenshot test reach its view and prove it arrived, so the
suite covers three screens instead of one screen three times.
Closes #676.
The suite produced four PNGs but only two distinct images:
main-window,history-viewandstaging-viewwere byte-identical. The staging testlooked for a toolbar button named "Stage", which has never existed - the
Stage view is a sidebar item (
PBSourceViewStageItem). On the miss itfell through and photographed the unchanged history view under the
staging name, so a regression confined to the staging view could not be
caught, and any change to the shared window reported as three
differences.
actually reachable from.
switch fails the run instead of quietly yielding a duplicate.
testMainWindowExistsand keep it as a launchassertion, the main window being the history view already.
the Stage view persists
PBShowStageViewand so decides what the nextlaunch opens on.
the walk, so a skipped capture no longer renumbers every file after it.
Test plan
make ui-testpasses all four tests locally.same run previously gave four files and two distinct hashes.
staging-view.pngnow shows the Stage sidebar selection with theUnstaged Changes, Commit Message and Staged Changes panes, confirmed
by eye;
history-view.pngshows the commit graph.Expect one round of baseline churn
The screenshot comparison on this pull request will report the whole set
as changed, for two stacked reasons: the image set itself changes, and
the workflow extracts the baseline with the base commit's copy of
extract_screenshots.py, so the baseline side is still index-prefixedwhile this side is not. Both settle once this is on master.