Repository navigation
CI: actually run the Swift test suites (#440) - #459
Merged
Merged
Conversation
hotkey-helper and accessibility-helper both have real test targets, wired into their Package.swift - nothing ever invoked them. Same shape as #425: a native-side regression could merge on a green check because the check never looked. transcription-helper has no test target (needs FluidAudio mocked out first) and isn't built in CI either - that gap stays open, called out in the new comment rather than silently left for someone to rediscover. Claude-Session: https://claude.ai/code/session_0144sxo5CJiyk99vaR7sgjuj
The first CI run of accessibility-helper's suite - ever, since #440's CI step is what runs it for the first time - failed two tests neither this session nor anyone else had actually seen fail: retriesFocus- ResolutionWhenTheAXTreeWasBrieflyNotReady and settleGuardPassesOnce- TheTargetStabilizes. Both built a FakeAccessibilityTarget without valueToReadBack, which defaults to nil. InjectionEngine's rung 1 confirms a write by reading the field back (`let after = focused.readValue(), after.contains(text)`) - against a fake with nothing to read back, that binding always fails, so both tests silently fell through to rung 2 and asserted an outcome ("wrote into the field") the fake could never produce. Not a production regression - InjectionEngine.swift is unchanged - just a test fixture missing the one line every other rung-1 test already has. Claude-Session: https://claude.ai/code/session_0144sxo5CJiyk99vaR7sgjuj
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.
Closes #440 (the CI half - see below for what's still open).
The gap
hotkey-helperandaccessibility-helperboth have real test suites wired as.testTargetin theirPackage.swift-HotkeyMatcherTests(XCTest) andAccessibilityInjectionTests(Swift Testing). Nothing ever ran them:.github/workflows/build.ymlonly rannpm ci && npm run build && npm test, and none ofscripts/build-{hotkey,accessibility,transcription}-helper.shcallswift testeither - they only doswift build -c release. Same shape as #425: a native-side regression could merge on a green check because the check never looked at Swift at all.The fix
Two new CI steps:
swift test --package-path native/hotkey-helperandswift test --package-path native/accessibility-helper.What's still open
transcription-helperhas noTests/directory and no test target - itsmain.swift(JSON protocol parsing, WAV/file-path branching, FluidAudio bridging) stays untested. The issue itself scoped that out as a bigger lift (mocking FluidAudio'sAsrManager), and this PR doesn't attempt it. It's also still not built in CI at all, unlike the other two - worth its own follow-up once a test target exists for it, since building it standalone right now would only prove it compiles, not add coverage.Verification
Can't run
swift testin this sandbox - a pre-existing local toolchain gap (no such module 'Testing'), unrelated to this change (swift buildon both packages is clean, before and after).accessibility-helper's suite specifically uses theTestingframework, which needs a recent-enough Xcode -macos-latestGitHub runners should have it, and this PR's own CI run is the actual proof either way.🤖 Generated with Claude Code
Update: the first real CI run of this suite (this PR's own first push) immediately failed two tests -
retriesFocusResolutionWhenTheAXTreeWasBrieflyNotReadyandsettleGuardPassesOnceTheTargetStabilizes- exactly the kind of thing #440 exists to catch. Root cause: both built aFakeAccessibilityTargetwithoutvalueToReadBack(defaults to nil), so rung 1's readback confirmation (focused.readValue(), thenafter.contains(text)) could never succeed and both silently fell through to rung 2. Not a production regression -InjectionEngine.swiftis untouched - just two test fixtures missing the one line every other rung-1 test already has. Fixed in the second commit.