Expose navigation handlers as @_spi(Testing) for unit tests#12
Merged
Conversation
Coordinators register routing logic as a trailing closure to push(...) / modallyPresent(...). That closure is only invokable through NVC's internal Combine subscription, so unit tests that just want to assert "when .submit fires, ReviewScene is pushed" have to drive the full view→VM→Combine pipeline through UIKit to reach it. This commit lifts the same closure to two stored properties on NanoViewController<View>: navigationHandler: ((NavigationStep) -> Void)? modalNavigationHandler: ((NavigationStep, @escaping DismissScene) -> Void)? Both are @_spi(Testing) public internal(set), so production callers see them only when they opt in with `@_spi(Testing) import` — NVC writes to them, consumers only read. subscribeToNavigation / subscribeToModalNavigation set the matching hook before installing the Combine sink. Documentation: - Heavy DocC on both properties (rationale, example, trade-off, visibility). - Side-effect note on both subscribeTo* helpers. - New README section "Testing your coordinators" with rationale + worked example for both the push and modal variants. - Worked test snippet in the SignUpDemo's OnboardingCoordinator docstring showing how a consumer would unit-test routing via the SPI. Coverage: - New NavigationHandlerSPITests covers four contracts: (1) push sets navigationHandler, (2) push and Combine sink route through the same closure, (3) modallyPresent sets modalNavigationHandler with a spy DismissScene, (4) the DismissScene spy receives the animated flag and completion forwarded by the routing closure. All 28 tests pass (24 prior + 4 new). The SignUpDemo example still builds. No behaviour change on the production path. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12 +/- ##
=======================================
Coverage 97.42% 97.43%
=======================================
Files 39 39
Lines 1166 1168 +2
=======================================
+ Hits 1136 1138 +2
Misses 30 30
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR exposes SPI-only navigation handler hooks on NanoViewController so coordinator routing can be unit-tested without driving the full ViewModel/Combine/UI pipeline.
Changes:
- Adds
navigationHandlerandmodalNavigationHandlerSPI properties toNanoViewController. - Stores registered push/modal routing closures on those hooks during navigation subscription setup.
- Adds tests and documentation showing direct invocation of the SPI hooks.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
Sources/NanoViewControllerController/NanoViewController.swift |
Adds SPI hook properties and DocC explaining intended test usage. |
Sources/NanoViewControllerController/Coordinating+NanoViewController+NavigationHelpers.swift |
Assigns routing closures to the new SPI hooks before subscribing to navigation publishers. |
Tests/NanoViewControllerControllerTests/NavigationHandlerSPITests.swift |
Adds unit tests for push and modal SPI handler behavior. |
README.md |
Documents coordinator testing with SPI navigation handlers. |
Examples/SignUpDemo/Sources/Onboarding/OnboardingCoordinator.swift |
Adds example documentation for testing coordinator routing. |
Comments suppressed due to low confidence (1)
Sources/NanoViewControllerController/Coordinating+NanoViewController+NavigationHelpers.swift:44
- When a scene instance that was previously pushed is later reused with a modal-style helper, the old
navigationHandlerremains set alongside the new modal hook. Clearscene.navigationHandlerhere so the SPI properties stay mutually exclusive and cannot expose stale routing closures.
scene.modalNavigationHandler = handler
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| of scene: S, | ||
| handler: @escaping (V.ViewModel.NavigationStep) -> Void | ||
| ) { | ||
| scene.navigationHandler = handler |
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.
Summary
@_spi(Testing) public internal(set) var navigationHandlerandmodalNavigationHandlertoNanoViewController<View>so unit tests can drive coordinator routing without going through the view-model's Combine pipeline (no UIKit taps, no text entry, no runloop drains).subscribeToNavigation/subscribeToModalNavigationstash the same closure the Combine sink uses on the matching hook before installing the subscription. Zero production-path behaviour change.OnboardingCoordinatorin SignUpDemo) gains a doc-comment snippet showing the consumer-side usage.Why
Coordinators register routing logic as a trailing closure to
push(...)/modallyPresent(...):That closure is only invokable through NVC's Combine subscription on
scene.navigation. Tests that want to assert "when.submitfires,ReviewSceneis pushed" otherwise have to drive the full view → view-model → Combine pipeline through real UIKit (tap,setText,drainRunLoop) just to reach the switch statement. For coordinator routing assertions, that's an expensive and brittle way to test mechanical wiring.With the SPI hook, consumers do:
Visibility
@_spi(Testing) public internal(set)— production callers see the property asinternal(i.e. invisible across module boundaries) unless they opt in with@_spi(Testing) import. NVC writes through theinternal(set); consumers can only read.Trade-off
Driving the SPI handler directly does not assert that the ViewModel's emitted step actually reaches the coordinator's subscription — only that the handler routes correctly once invoked. The Combine wiring is identical across every
push(...)/modallyPresent(...)call, so a single happy-path UI-driven test (or simply observing that the scene appears on the stack afterstart()) covers it. Documented in the README + property docstrings so consumers can make the call for their own test pyramid.Test plan
just test— 28 tests pass (24 prior + 4 new inNavigationHandlerSPITests).just example-build— SignUpDemo still builds with the added doc comment onOnboardingCoordinator.scene.navigationHandlerisnilfor a controller that hasn't been pushed/presented (default state), set afterpush, and routes through the same closure the Combine sink fires.🤖 Generated with Claude Code