Skip to content

Stop the window wiring tests needing the keyboard focus - #622

Merged
hannesa2 merged 1 commit into
masterfrom
fix-wiring-tests-depending-on-focus
Sep 13, 2026
Merged

hannesa2 merged 1 commit into
masterfrom
fix-wiring-tests-depending-on-focus

Conversation

@goneng

@goneng goneng commented Sep 13, 2026 •

Copy link
Copy Markdown
Collaborator

-selectedRef reads the first responder to decide whether the sidebar or
the history list is being asked, so PBGitWindowWiringTests took the
keyboard focus to say which one it meant. A test cannot hold the keyboard:
anything that takes it hands the window back its own first responder, and
the test is then asking about a list that nothing is focused on.

The window controller can now be asked what a given responder names, and
answers -selectedRef by handing it the first responder as before. The
tests hand the list over directly, so no test in the suite touches focus.

What this does not claim

These tests have failed for me in ways I could not reproduce on demand. I
tried twice against the unfixed code and both passed:

attempt result
Finder activated once a second for the whole run 71 passed
a second GitX running, since the test host is GitX.app 71 passed

So this removes a dependency the tests should not have had, and nothing
more. If they fail again, the cause is elsewhere and still open.

One thing found on the way, worth knowing before writing tests here:
-[PBGitHistoryController selectedCommits] is derived from its array
controller and is put back by the controller's own machinery, so setting
it directly does not stick. The tests keep driving the array controller.

Test plan

  • make unit-test: 71 tests, 0 failures
  • No makeFirstResponder: left anywhere in the suite
  • Ran the suite with focus being taken once a second, and with a second
    GitX instance running: passes in both

Which list -selectedRef answers for is read off the first responder, so
the tests took the focus to say which one they meant. A test cannot hold
the keyboard: anything that takes it hands the window back its own first
responder, and the test then asks about a list nothing is focused on.

* Let the window controller be asked which ref a given responder names,
  and keep reading the first responder for the answer it gives itself
* Hand the list over in the tests rather than focusing it, so what they
  describe no longer depends on what else is on screen
@goneng
goneng force-pushed the fix-wiring-tests-depending-on-focus branch from e6863a1 to ac3f726 Compare September 13, 2026 07:33
@hannesa2

Copy link
Copy Markdown
Contributor

Screenshot differs: emulatorApi=arm64 with 4 screenshot(s)

setpoint (old) | diff | actual screenshot (new)

00-commit-context-menu.pngscreenshot

01-history-view.pngscreenshot

02-main-window.pngscreenshot

03-staging-view.pngscreenshot

@goneng
goneng marked this pull request as ready for review September 13, 2026 07:49
@hannesa2
hannesa2 merged commit b0dc7de into master Sep 13, 2026
3 checks passed
@hannesa2
hannesa2 deleted the fix-wiring-tests-depending-on-focus branch September 13, 2026 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test Test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants