Repository navigation
Keep the unit tests from depending on the machine they run on - #620
Merged
Merged
Conversation
goneng
changed the title
Setting amend asks the index to read itself back, and that read-back launches git rather than going through the task output the test answers for. On a runner where the git lookup comes back empty the launch raises instead, which is how the suite came to fail on master on one architecture while passing on the other. * Keep the read-back out of the diff cache tests, the way the refresh tests already do, so the only git they need is the one they answer for
A sweep of the suites for the same kind of dependency that made the amend test fail on one runner and pass on the other. * Put the prune-on-fetch preference back the way it was found, rather than clearing it: the tests run in the application, so that setting belongs to whoever runs them * Say so when the git that builds a test repository fails to run or fails outright, instead of leaving the assertion that follows to report something that reads like a product bug * Hand that git an empty global and system config, so a repository built for a test is the same one everywhere
goneng
force-pushed
the
fix-amend-test-launching-git
branch
from
September 13, 2026 06:50
6922397 to
32a2171
Compare
Contributor
|
Screenshot differs: emulatorApi=arm64 with 3 screenshot(s) |
hannesa2
approved these changes
Sep 13, 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.
master is red on arm64 since #601 merged, and the cause came in with #599.
x86_64 passes, so it shows up on one architecture at a time.
testAmendingIsAskedForAgainsetsamend,-setAmend:asks the index toread itself back, and
-refreshlaunches git through[PBGitBinary path]rather than through the task output these tests answer for.
When that lookup comes back empty on a runner,
PBTaskraises instead of running,and the test fails on an environment detail it never meant to depend on.
The diff cache tests now use an index whose read-back does nothing, which
is what
PBGitIndexRefreshTestsalready does withPBRecordingIndex.The rest of the sweep
Second commit, after looking for the same kind of dependency elsewhere:
PBGitRepositoryFetchArgumentsTestswrote the realPBPruneOnFetchpreference and cleared it afterwards.
The tests run in the application, so running them deleted that setting for whoever ran them.
It is now put back the way it was found.
PBRepositoryFinderTestsandPBGitWindowWiringTestsbuild a real repositorywith
/usr/bin/gitand ignored whether it worked, so a git that cannot runsurfaced as an assertion further down that reads like a product bug.
They now say which command failed.
does not depend on the config of whoever runs it.
PBGitWindowWiringTeststook the keyboard focus to say which list it meant,which a test cannot reliably hold. That one is #622 rather than here, since it
needs a seam in the window controller. It is worth saying that I have not been
able to reproduce on demand the failures I have seen from that suite, so the
focus was a dependency worth removing rather than a diagnosis.
Test plan
make unit-test: 71 tests, 0 failurestestAmendingIsAskedForAgaindrops from 0.331s to 0.001s, since it nolonger launches three git processes it never wanted
[self parentTree]in the cache key with aconstant fails it, so it continues to check that amending is asked for
against HEAD^ rather than HEAD
PBPruneOnFetchto 2, ran the suite, and it is still 2 afterwardsnot start: The file ... doesn't exist" rather than a null comparison