Repository navigation
Add tests for the index refresh and staging wiring - #576
Merged
Merged
Conversation
Add tests for the two commits of gitx#571 that landed without any, and for the notification ordering that the third one changed. PBGitIndexReconcileTests covers how the file list is derived, but nothing exercised what surrounds it: that staging re-reads the index rather than trusting the flags it has just set, that a refresh announces the file list only once it has been rebuilt from all three commands, and that a request arriving during a refresh is deferred rather than discarded. None of that needs a repository on disk. -[PBGitRepository init] touches nothing outside itself, and the two collaborators that would otherwise reach the filesystem are ordinary overridable methods, so test-local subclasses record the calls instead of making them. - Extract the refresh guard into -beginRefreshOrDeferIt and -endRefreshTakingDeferred so the coalescing contract can be asserted directly, leaving what they do unchanged - Add PBGitIndexRefreshTests: staging and unstaging ask git and then read the index back, a rejected update-index reads nothing back, neither arm of a per-command result announces an index update, and a burst of requests collapses into exactly one re-run - Give the GitXTests target a header search path for Classes/Util, which PBGitRepository_PBGitBinarySupport.h needs
1 task done
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.
Follows up on #571, where you asked for tests covering those changes.
Of the three commits that landed there, only
777e6784carried tests, andthey cover the pure reconcile seam rather than what surrounds it. This adds
nine cases for the rest:
24849b90) - staging and unstaging askgit and then re-read, a rejected
update-indexreads nothing back, andstaged paths reach git NUL-terminated
ordering change folded into
777e6784; neither the success nor thefailure arm of a per-command result may announce an index update
74197e56) - a request arrivingduring a refresh is deferred rather than discarded, and a burst collapses
into exactly one re-run
Notes for review
No repository on disk, no git process, no FSEvents, no async settling. That
works because
-[PBGitRepository init]touches nothing outside itself, andthe two collaborators that would otherwise reach the filesystem are ordinary
overridable methods, so test-local subclasses record the calls instead of
making them. The whole suite runs in about two seconds.
One production change: the refresh guard in
-refreshand its other half inthe completion block are extracted into
-beginRefreshOrDeferItand-endRefreshTakingDeferred, so the coalescing contract can be asserteddirectly. Same logic, same serial queue, just named.
Being straight about what the coalescing cases are: a contract test for the
state machine, not a reproduction of the original bug. Losing the settling
event of a rebase burst only reproduces against a real repository on disk,
and was verified by hand for #571. The other two groups do fail against the
pre-fix code.
The GitXTests target gains a header search path for
Classes/Util, whichPBGitRepository_PBGitBinarySupport.hneeds.Test plan
xcodebuild test -workspace GitX.xcworkspace -scheme GitX -only-testing:GitXTestsguards and confirming it goes red:
[self refresh]afterupdate-indexfails the two stagingcases on the read-back count
[self postIndexUpdated]inpostIndexRefreshSuccess:message:trips both inverted expectations
coalescing cases, while the "nothing was deferred" case correctly stays
green