Bump NanoViewController to transform_result_builder branch#140
Merged
Conversation
Adopts the breaking-change refactor in Sajjon/NanoViewController#11: SceneController/AbstractController merged into NanoViewController<View>; per-feature chrome protocols (TitledScene, LeftBarButtonMaking, etc.) collapsed into a single ControllerConfig surface; transform now returns Output<Publishers, NavigationStep>. * BaseViewModel rewritten over the new AbstractViewModel<FromView, Publishers, NavigationStep: Sendable> while keeping stored cancellables + navigator for migration ergonomics. * Nested `Output` → `Publishers` on every ViewModel; transform return shape updated to wrap with Output(publishers:, navigation:). * Views' populate(with:) takes ViewModel.Publishers instead of .Output. * Scene controllers declare `static let config = ControllerConfig(...)` via ControllerConfigProviding; TermsOfService uses the instance-level controllerConfig override for construction-time chrome. * Zhip-side ControllerConfig+Localization extension adds `init(titleKey: LocalizedStringResource, ...)` so call sites read `titleKey: .CreateNewWallet.title` instead of wrapping in String(localized:). * SignTransactionViewModel: extract `inertOutput()` helper to keep transform within the SwiftLint function-body length budget. * Tests observe ViewModel output via `output.publishers.X` to match the new wrapper. 745 / 745 tests passing. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #140 +/- ##
==========================================
- Coverage 96.79% 96.13% -0.67%
==========================================
Files 312 319 +7
Lines 16130 16542 +412
==========================================
+ Hits 15613 15902 +289
- Misses 517 640 +123 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adopts the upstream NanoViewController breaking refactor (PR #11) that collapses the previous SceneController/AbstractController split and per-chrome protocols (TitledScene, LeftBarButtonMaking, RightBarButtonMaking, BackButtonHiding, NavigationBarLayoutOwner) into a single NanoViewController<View> + ControllerConfigProviding surface, and changes transform(input:) to return Output<Publishers, NavigationStep> with the publisher bag separated from navigation. The bulk of the diff is mechanical migration across every ViewModel / View / scene-controller in the app plus matching test updates.
Changes:
- Rewrite
BaseViewModelover the newAbstractViewModel<FromView, Publishers, NavigationStep: Sendable>shape while preserving storedcancellables+navigatorergonomics. - Rename each ViewModel's nested
OutputtoPublishers, wrap returns inOutput(publishers:navigation:), and migrate scene controllers fromScene<X>toNanoViewController<X> + ControllerConfigProviding(withTermsOfServiceusing an instance-levelcontrollerConfigoverride andLockAppScenefalling back to plainUIViewController); add Zhip-sideControllerConfiglocalization-key convenience init. - Bump
NanoViewControllerfromexact: 0.1.4tobranch: transform_result_builderinPackage.swift/project.yml, and drop the now-removedLeftBarButtonMaking/RightBarButtonMakingwrappers and the obsoleteNavigationBarLayoutingNavigationControllerTests.
Reviewed changes
Copilot reviewed 96 out of 97 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Package.swift, project.yml, Package.resolved | Switch NanoViewController dep from tagged version to a branch |
| Sources/AppFeature/ViewModel/BaseClasses/BaseViewModel.swift | Re-base on new AbstractViewModel; keep cancellables + navigator |
| Sources/AppFeature/Controller/ControllerConfig+Localization.swift | New init(titleKey:…) convenience over String(localized:) |
| Sources/AppFeature/Controller/AbstractViewController+BarButtonItem/{Left,Right}BarButtonMaking.swift | Delete now-obsolete protocol wrappers |
| Sources/AppFeature/Utils/{NavigationBarLayoutOwner,AppAppearance}.swift | Doc updates pointing at ControllerConfig.navigationBarLayout |
| Sources/AppFeature/Navigation/Coordinator/Coordinating/.../Coordinating+DebugPrinting.swift | Drop AbstractController cast in debug printer |
| Sources/AppFeature/Scenes/**/*ViewModel.swift | Rename nested Output→Publishers; return Output(publishers:,navigation:) |
| Sources/AppFeature/Scenes/**/*View.swift | populate(with: ViewModel.Output) → …Publishers |
| Sources/AppFeature/Scenes/**/*.swift (scene controllers) | Migrate Scene<X> to NanoViewController<X> + ControllerConfigProviding; collapse chrome protocols into a single ControllerConfig |
| Sources/AppFeature/Scenes/1_Main/0_LockApp/LockAppScene.swift | Move from AbstractController to plain UIViewController |
| Sources/AppFeature/Scenes/0_Onboarding/1_TermsOfService/TermsOfService.swift | Use instance-level controllerConfig override for per-construction layout |
| Sources/AppFeature/Scenes/.../{Main,PrepareTransaction}ViewModel.swift | Disambiguate Combine.Publishers.Merge3 after nested type shadowing |
| Sources/AppFeature/Scenes/.../SignTransactionViewModel.swift | Extract inertOutput() to keep transform within SwiftLint length budget |
| Tests/Extensions/ViewModel/ViewModelType_Extension.swift | Update test helper return type to Output<Publishers, NavigationStep> |
| Tests/Tests/ViewModels/*Tests.swift | Update assertions to output.publishers.X |
| Tests/Tests/Views/RestoreWalletViewTests.swift | Construct Publishers instead of Output |
| Tests/Tests/Navigation/CoordinatingInfrastructureTests.swift | Use plain UIViewController instead of AbstractController |
| Tests/Tests/Navigation/NavigationBarLayoutingNavigationControllerTests.swift | Delete — exercised types now owned by the package |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // `SingleLineController*` modules). Pinned to a tagged release; | ||
| // bump as new tags ship. | ||
| .package(url: "https://github.com/Sajjon/NanoViewController", exact: "0.1.4"), | ||
| .package(url: "https://github.com/Sajjon/NanoViewController", branch: "transform_result_builder"), |
| NanoViewController: | ||
| url: https://github.com/Sajjon/NanoViewController | ||
| exactVersion: 0.1.4 | ||
| branch: transform_result_builder |
PR #11 in NanoViewController makes `transform(input:)` return all lifetime-bound work in `Output<Publishers, NavigationStep>` and lets the controller own that output for the scene lifetime. The earlier BaseViewModel shim in this repo kept stored `cancellables` + `navigator` to ease migration, but that contradicts the new design: a second private framework layered over the public one. Now Zhip conforms to the package direction directly. * Delete `Sources/AppFeature/ViewModel/BaseClasses/BaseViewModel.swift`. * Every `BaseViewModel<Step, FromView, Publishers>` becomes `AbstractViewModel<FromView, Publishers, Step>` (slot order swap). * Each `transform(input:)` starts with `let navigator = Navigator<…>()` and returns a single `Output(publishers: …, navigation: navigator.navigation) { … }` expression — sinks live inside the trailing `@BindingsBuilder` block with `[navigator]` captures. * `SignTransactionViewModel`: inert branch now constructs the wallet- unavailable Output inline with a local navigator; `inertOutput()` → `static inertPublishers()`. * `AppCoordinator`: `scene.viewModel.navigator.navigation` → `scene.navigation` (the controller forwards Output.navigation). ## Tests * ViewModel tests now observe `output.navigation` instead of `sut.navigator.navigation`. Most `makeSUT()` helpers return `(VM, Output)` so the Output survives the test. * Coordinator tests drive real UIKit: - Bar-button-driven steps: `scene.{left,right}BarButtonSubject.send(())`. - In-content steps: tap actual buttons via a new `Tests/Helpers/UIView+Hierarchy.swift` helper (`tapButton(at:in:)`, `setCheckbox`, `enterPincode`, `setText`, `selectTableRow`). * `SendCoordinatorTests`: 9 routing-only tests are `XCTSkip`ped — the Send pipeline (PrepareTransaction → Review → Sign → Poll, plus QR scan) needs a complete payment form or `AVCaptureMetadataOutput` callback to exercise via the real UI. Per-VM tests cover that logic. * `RestoreWallet` and `CreateNewWallet` coordinator tests now register the relevant use-case mocks so flatMapLatest resolves synchronously. 745 / 745 tests passing (9 deliberate skips). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
| // `SingleLineController*` modules). Pinned to a tagged release; | ||
| // bump as new tags ship. | ||
| .package(url: "https://github.com/Sajjon/NanoViewController", exact: "0.1.4"), | ||
| .package(url: "https://github.com/Sajjon/NanoViewController", branch: "transform_result_builder"), |
The transform_result_builder branch was tagged as 0.1.5. Switch both Package.swift and project.yml from the floating branch pin to the exact tag so the build is reproducible. Resolves the Copilot review note on PR #140 about branch-pinning. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 117 out of 118 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
Package.swift:46
- Pinning the SPM dependency to a branch (
transform_result_builder) instead of a tagged release is a regression in reproducibility — branch tips can move, and any future check-out of this revision can resolve to a differentNanoViewControllerHEAD than thePackage.resolvedsnapshot (especially in CI environments that re-resolve). Before merging, please publish a proper tagged release ofNanoViewController(e.g.0.2.0) and switch bothproject.ymlandPackage.swiftback toexactVersion/exact:pinning.
.package(url: "https://github.com/Sajjon/NanoViewController", branch: "transform_result_builder"),
| // `SingleLineController*` modules). Pinned to a tagged release; | ||
| // bump as new tags ship. | ||
| .package(url: "https://github.com/Sajjon/NanoViewController", exact: "0.1.4"), | ||
| .package(url: "https://github.com/Sajjon/NanoViewController", branch: "transform_result_builder"), |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 117 out of 118 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (1)
Tests/Tests/Navigation/SendCoordinatorTests.swift:187
- Eight previously-passing coordinator tests are now
throw XCTSkip(...), including the entire send→sign→poll pipeline (test_prepareTransactionReviewPayment_pushesReviewTransaction,test_reviewAcceptPayment_pushesSignTransaction,test_signTransactionSign_pushesPollTransactionStatus, all fourtest_poll*variants,test_deeplinkedTransaction_whenNotOnPrepare_isFilteredOut,test_scanQRCode_scannedTransaction_dismissesAndForwardsToSubject). The PR description notes the smoke-test checkboxes for the send flow are still unchecked, which combined with this skipped coverage means the send-pipeline routing logic is currently unverified at the coordinator level.
These coordinator-level transition assertions (e.g. top(as: PollTransactionStatus.self) != nil after .skip) are not equivalent to the per-VM behavioral tests cited in the skip messages, since the VM tests don't exercise the parent coordinator's routing/dismissal logic. Consider either restoring at least one happy-path coordinator chain (perhaps by exposing a test seam on PrepareTransactionViewModel such as a method to inject a Payment, mirroring how RestoreWalletCoordinatorTests was updated to seed mockWallet.restoreWalletResult) or splitting these skips into a follow-up PR that re-introduces the lost coverage.
| /// observing `.walletUnavailable`, so these placeholder publishers are | ||
| /// only briefly attached. `static` because it depends on no instance state — | ||
| /// the navigator is now local to `transform(input:)`. | ||
| private static func inertPublishers() -> Publishers { |
| // `SingleLineController*` modules). Pinned to a tagged release; | ||
| // bump as new tags ship. | ||
| .package(url: "https://github.com/Sajjon/NanoViewController", exact: "0.1.4"), | ||
| .package(url: "https://github.com/Sajjon/NanoViewController", exact: "0.1.5"), |
| func test_transform_callsGetMinimumGasPrice() { | ||
| let sut = makeSUT() | ||
| _ = sut.transform(input: makeInput()) | ||
| let output = sut.transform(input: makeInput()) |
Comment on lines
128
to
222
| @@ -163,85 +159,31 @@ final class SendCoordinatorTests: XCTestCase { | |||
|
|
|||
| // MARK: - Sign → PollTransactionStatus | |||
|
|
|||
| private func makeTransactionResponse() throws -> TransactionResponse { | |||
| try JSONDecoder().decode(TransactionResponse.self, from: Data(#"{"TranID":"abc123","Info":"Sent"}"#.utf8)) | |||
| } | |||
|
|
|||
| private func pushToSignTransaction() throws -> SignTransaction { | |||
| sut.start() | |||
| let prepare = top(as: PrepareTransaction.self)! | |||
| let payment = try makePayment() | |||
| prepare.viewModel.navigator.next(.reviewPayment(payment)) | |||
| drainRunLoop() | |||
| let review = top(as: ReviewTransactionBeforeSigning.self)! | |||
| review.viewModel.navigator.next(.acceptPaymentProceedWithSigning(payment)) | |||
| drainRunLoop() | |||
| return top(as: SignTransaction.self)! | |||
| } | |||
|
|
|||
| func test_signTransactionSign_pushesPollTransactionStatus() throws { | |||
| let sign = try pushToSignTransaction() | |||
|
|
|||
| try sign.viewModel.navigator.next(.sign(makeTransactionResponse())) | |||
| drainRunLoop() | |||
|
|
|||
| XCTAssertTrue(top(as: PollTransactionStatus.self) != nil) | |||
| // Pushing SignTransaction requires reaching it via the full | |||
| // Prepare → Review chain, which in turn requires full payment-form | |||
| // entry (recipient + amount + gas), and triggering `.sign(...)` from | |||
| // SignTransaction needs entry of the wallet password plus a mocked | |||
| // `sendTransaction` response. Covered by `SignTransactionViewModelTests`. | |||
| throw XCTSkip("Full send pipeline requires payment-form + password entry; covered by SignTransactionViewModelTests.") | |||
| } | |||
|
|
|||
| // MARK: - PollTransactionStatus branches | |||
|
|
|||
| private func pushToPoll() throws -> PollTransactionStatus { | |||
| let sign = try pushToSignTransaction() | |||
| try sign.viewModel.navigator.next(.sign(makeTransactionResponse())) | |||
| drainRunLoop() | |||
| return top(as: PollTransactionStatus.self)! | |||
| } | |||
|
|
|||
| func test_pollSkip_bubblesFinishWithoutFetchingBalance() throws { | |||
| let poll = try pushToPoll() | |||
| var received: SendCoordinatorNavigationStep? | |||
| sut.navigator.navigation.sink { received = $0 }.store(in: &cancellables) | |||
|
|
|||
| poll.viewModel.navigator.next(.skip) | |||
| drainRunLoop() | |||
|
|
|||
| if case let .finish(fetch) = received { XCTAssertFalse(fetch) } else { | |||
| XCTFail("expected .finish(false), got \(String(describing: received))") | |||
| } | |||
| throw XCTSkip("PollTransactionStatus reachable only after the full send pipeline; covered by PollTransactionStatusViewModelTests.") | |||
| } | |||
|
|
|||
| func test_pollWaitUntilTimeout_bubblesFinishWithoutFetchingBalance() throws { | |||
| let poll = try pushToPoll() | |||
| var received: SendCoordinatorNavigationStep? | |||
| sut.navigator.navigation.sink { received = $0 }.store(in: &cancellables) | |||
|
|
|||
| poll.viewModel.navigator.next(.waitUntilTimeout) | |||
| drainRunLoop() | |||
|
|
|||
| if case let .finish(fetch) = received { XCTAssertFalse(fetch) } else { | |||
| XCTFail("expected .finish(false), got \(String(describing: received))") | |||
| } | |||
| throw XCTSkip("PollTransactionStatus reachable only after the full send pipeline; covered by PollTransactionStatusViewModelTests.") | |||
| } | |||
|
|
|||
| func test_pollDismiss_bubblesFinishWithFetchingBalance() throws { | |||
| let poll = try pushToPoll() | |||
| var received: SendCoordinatorNavigationStep? | |||
| sut.navigator.navigation.sink { received = $0 }.store(in: &cancellables) | |||
|
|
|||
| poll.viewModel.navigator.next(.dismiss) | |||
| drainRunLoop() | |||
|
|
|||
| if case let .finish(fetch) = received { XCTAssertTrue(fetch) } else { | |||
| XCTFail("expected .finish(true), got \(String(describing: received))") | |||
| } | |||
| throw XCTSkip("PollTransactionStatus reachable only after the full send pipeline; covered by PollTransactionStatusViewModelTests.") | |||
| } | |||
|
|
|||
| func test_pollViewTransactionDetails_opensBrowserWithoutCrashing() throws { | |||
| let poll = try pushToPoll() | |||
|
|
|||
| poll.viewModel.navigator.next(.viewTransactionDetailsInBrowser(id: "abc123")) | |||
| drainRunLoop() | |||
| // openURL returns asynchronously; we just verify the path ran. | |||
| throw XCTSkip("PollTransactionStatus reachable only after the full send pipeline; covered by PollTransactionStatusViewModelTests.") | |||
| } | |||
|
|
|||
| // MARK: - Deep-link filter reject branch | |||
| @@ -250,44 +192,32 @@ final class SendCoordinatorTests: XCTestCase { | |||
| /// transactions must be filtered out so they don't mutate an unrelated | |||
| /// scene's state. | |||
| func test_deeplinkedTransaction_whenNotOnPrepare_isFilteredOut() throws { | |||
| sut.start() | |||
| let prepare = try XCTUnwrap(top(as: PrepareTransaction.self)) | |||
| let payment = try makePayment() | |||
| prepare.viewModel.navigator.next(.reviewPayment(payment)) | |||
| drainRunLoop() | |||
| XCTAssertNotNil(top(as: ReviewTransactionBeforeSigning.self)) | |||
|
|
|||
| let address = try Address(string: "e3090a1309DfAC40352d03dEc6cCD9cAd213e76B") | |||
| deeplinkSubject.send(TransactionIntent(to: address)) | |||
| drainRunLoop() | |||
| // Reject path runs without mutating the scan-QR subject. | |||
| // Reaching a non-PrepareTransaction scene requires UI-driven push | |||
| // through the full Prepare form. Covered indirectly by the routing. | |||
| throw XCTSkip("Reaching the non-PrepareTransaction state requires full payment-form entry; covered indirectly by the PrepareTransaction VM tests.") | |||
| } | |||
|
|
|||
| // MARK: - ScanQRCode result branches | |||
|
|
|||
| func test_scanQRCode_cancel_dismissesWithoutCrashing() throws { | |||
| sut.start() | |||
| let prepare = try XCTUnwrap(top(as: PrepareTransaction.self)) | |||
| prepare.viewModel.navigator.next(.scanQRCode) | |||
| try tapButton(at: 0, in: prepare.view) // scanQR button | |||
| drainRunLoop() | |||
|
|
|||
| let presentedNav = navigationController.presentedViewController as? UINavigationController | |||
| let scan = presentedNav?.topViewController as? ScanQRCode | |||
| scan?.viewModel.navigator.next(.cancel) | |||
| // ScanQRCode `.cancel` is wired to the left-bar button. | |||
| scan?.leftBarButtonSubject.send(()) | |||
| drainRunLoop() | |||
| } | |||
|
|
|||
| func test_scanQRCode_scannedTransaction_dismissesAndForwardsToSubject() throws { | |||
| sut.start() | |||
| let prepare = try XCTUnwrap(top(as: PrepareTransaction.self)) | |||
| prepare.viewModel.navigator.next(.scanQRCode) | |||
| drainRunLoop() | |||
|
|
|||
| let presentedNav = navigationController.presentedViewController as? UINavigationController | |||
| let scan = presentedNav?.topViewController as? ScanQRCode | |||
| let address = try Address(string: "e3090a1309DfAC40352d03dEc6cCD9cAd213e76B") | |||
| let intent = TransactionIntent(to: address) | |||
| scan?.viewModel.navigator.next(.scanQRContainingTransaction(intent)) | |||
| drainRunLoop() | |||
| // The `.scanQRContainingTransaction` step fires when the camera reads | |||
| // a valid QR code — there's no UI control (it's a delegate callback | |||
| // from `AVCaptureMetadataOutput`). Driving this without a real | |||
| // camera buffer would require fake-injecting into `scannedQrCodeString` | |||
| // which is no longer accessible after the navigator removal. | |||
| throw XCTSkip("Real QR-scan callback not drivable in unit tests; covered by ScanQRCodeViewModelTests.") | |||
| } | |||
Comment on lines
+141
to
+152
| if let targets = button.allTargets as Set<AnyHashable>? as? Set<AnyHashable>, !targets.isEmpty { | ||
| for target in button.allTargets { | ||
| if let actions = button.actions(forTarget: target, forControlEvent: .touchUpInside) { | ||
| for action in actions { | ||
| let selector = Selector(action) | ||
| _ = (target as AnyObject).perform(selector, with: button) | ||
| } | ||
| } | ||
| } | ||
| } else { | ||
| button.sendActions(for: .touchUpInside) | ||
| } |
Comment on lines
+109
to
+121
| // Retain `output` (and therefore its `cancellables`) for the lifetime | ||
| // of the test — `Output.cancellables` is what keeps the `.sink` on | ||
| // `copyMyAddressTrigger` alive. Discarding the tuple would also drop | ||
| // the subscription and the copy/toast side effects would never fire. | ||
| let (_, output) = makeSUT() | ||
| var emittedToast: Toast? | ||
| fakeController.toastSubject.sink { emittedToast = $0 }.store(in: &cancellables) | ||
|
|
||
| copySubject.send(()) | ||
|
|
||
| XCTAssertEqual(mockPasteboard.copiedString, mockWallet.storedWallet?.bech32Address.asString) | ||
| XCTAssertNotNil(emittedToast) | ||
| _ = output // keep alive |
| } | ||
|
|
||
| private func makeSUT() -> ChooseWalletViewModel { | ||
| private func makeSUT() -> (ChooseWalletViewModel, Output<ChooseWalletViewModel.Publishers, ChooseWalletUserAction>) { |
ScanQRCodeViewModel now accepts an optional injected `scannedQrCodeString` publisher mirroring `PrepareTransactionViewModel.scannedOrDeeplinkedTransaction`. SendCoordinator plumbs the same publisher through its init so callers upstream can synthesize scans without an AVCaptureMetadataOutput callback. Production callers don't pass anything; the default nil falls back to the view's camera-backed subject, so behaviour is unchanged. SendCoordinatorTests injects a PassthroughSubject through the new param, which un-skips test_scanQRCode_scannedTransaction_dismissesAndForwardsToSubject (now with AAA annotations) and drops the skip count from 9 → 8 across the suite. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…skip all 8 Bumps NVC to branch `testing-spi-navigation-handler` (NVC PR #12) which exposes `navigationHandler` / `modalNavigationHandler` on every `NanoViewController` under `@_spi(Testing)`. The hooks hold the same closure NVC's Combine subscription fires, so unit tests can drive coordinator routing without filling forms, tapping buttons, or draining the runloop for UIKit layout passes. SendCoordinatorTests imports the SPI and the previously-skipped 8 tests now pass: test_prepareTransactionReviewPayment_pushesReviewTransaction test_reviewAcceptPayment_pushesSignTransaction test_signTransactionSign_pushesPollTransactionStatus test_pollSkip_bubblesFinishWithoutFetchingBalance test_pollWaitUntilTimeout_bubblesFinishWithoutFetchingBalance test_pollDismiss_bubblesFinishWithFetchingBalance test_pollViewTransactionDetails_opensBrowserWithoutCrashing test_deeplinkedTransaction_whenNotOnPrepare_isFilteredOut A `chainToPoll()` helper walks `Prepare → Review → Sign → Poll` purely via SPI handlers, then each test drives the specific routing case under test. `test_pollViewTransactionDetails_*` registers a `MockUrlOpener` and re-creates the SUT so `@Injected(\.urlOpener)` resolves to the mock. Helpers live in a `private extension` so the class body stays under SwiftLint's `type_body_length` limit. All tests use AAA annotations. Total: 745 tests, 0 failures, 0 skips (was 745 / 0 / 8). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The prior assertion was "stack count unchanged" — which would pass even
if SendCoordinator's topmost-scene filter were removed entirely. A stray
pre-fill of a non-topmost view's text field doesn't push or pop anything,
so the stack would be unchanged either way.
The real observable side-effect of a missing filter is that the deeplinked
address would bind into `PrepareTransactionView.recipientAddressField.text`
via the VM's `scannedOrDeeplinkedTransaction` → recipientFormatted pipeline,
even though Prepare isn't visible. So we now:
1. Snapshot recipientField.text before the deeplink emission. (Non-empty
in DEBUG: prefillValuesForDebugBuilds populates a placeholder.)
2. Push past Prepare via the SPI handler.
3. Send the deeplinked intent.
4. Assert recipientField.text is byte-for-byte equal to the snapshot.
5. Assert the text does NOT contain the deeplinked address — defends
against any unrelated mutation that happens to coincide with the
snapshot value.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The SPI feature shipped as a tagged release (https://github.com/Sajjon/NanoViewController/releases/tag/v0.1.6). Switch both Package.swift and project.yml from the floating branch pin to the exact tag so the build is reproducible and addresses the previous Copilot review's branch-pinning concern. v0.1.6 includes the @_spi(Testing) navigationHandler / modalNavigationHandler hooks Zhip's SendCoordinatorTests consumes, plus the post-merge follow-up that makes the two hooks mutually exclusive on re-subscription (the internal subscribeToNavigation / subscribeToModalNavigation helpers now store the cancellable on `scene.navigationSubscription` and clear the opposite handler). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The macos-26 runner image dropped iOS 26.1 and Xcode 26.1.1; available sim runtimes are now 26.2 / 26.4 / 26.5. CI was failing at the "Validate simulator destination" step with: Expected simulator not available: iPhone 17 (iOS 26.1) Pin both the workflow and the justfile default to 26.2 — the lowest version available on the current runner image. Mirrors the parallel bump in NanoViewController (commit 651d921). Local override still works: SIM_OS=26.x just test Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The production `preferences` factory resolves to `KeyValueStore(UserDefaults.standard)`. The old assertion was `XCTAssertFalse(... .isTrue(.hasAcceptedTermsOfService))` — which implicitly required UserDefaults.standard to have the key absent or `false`. On a dev machine that ran the simulator app once and tapped through onboarding, `hasAcceptedTermsOfService` lives in the host's `UserDefaults.standard` as `true`, and the test fails. CI passed because the GitHub runner has a virgin UserDefaults; locally it flaked. Rewrite the assertion to be value-independent: snapshot the production value first, then verify that after register-then-reset, mutating the mock to the *opposite* value does NOT leak through the resolved factory. The assertion is now "mock mutation is out of the chain" rather than "production happens to be false", which holds regardless of host state. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ton scope My previous de-flake attempt (aaabe4c) didn't actually fix the test: it just inverted the failure mode. CI started failing in the *opposite* direction (expected true, actual false) because of how Factory's scope caches work. Factory's `.singleton` scope maintains its OWN cache, separate from the container's cache. `Container.shared.manager.reset()` only clears the container's cache — it explicitly logs "Singleton scope not managed by container" when asked to reset Scope.Singleton and refuses. So mocks registered for `.singleton`-scoped factories LEAK across tests unless each test explicitly calls `Scope.singleton.reset()`. What happens on a fresh machine (CI): 1. Earlier test (test_register_overridesDefaultFactory) registers a `preferences` mock, resolves it (cached in Scope.singleton), tears down with `manager.reset()` which leaves the singleton cached. 2. This test runs. `productionValueBefore = ...preferences().isTrue(...)` resolves through the still-cached mock → returns the mock's value (true). productionValueBefore is now `true`, NOT the real production. 3. After the test's own register+reset, the singleton cache is finally invalidated, so the post-reset resolve gets a fresh production KeyValueStore(UserDefaults.standard), which on CI returns false. 4. Assert: false != true → FAIL. Fix: explicitly reset Scope.singleton at the start of the test (so the snapshot captures real production) and again after the test's own reset (so the assertion sees a fresh production resolve, not the freshly-cached mock that `register` may have created). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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
Adopts the breaking-change refactor in NanoViewController#11:
SceneController/AbstractControllermerged intoNanoViewController<View>.TitledScene,LeftBarButtonMaking,RightBarButtonMaking,BackButtonHiding,NavigationBarLayoutOwner) collapsed into oneControllerConfigsurface, provided viaControllerConfigProviding.transform(input:)now returnsOutput<Publishers, NavigationStep>— a wrapper carrying the publisher bag, navigation publisher, and lifetime-bound cancellables.Migration in this repo
BaseViewModelrewritten over the newAbstractViewModel<FromView, Publishers, NavigationStep: Sendable>while keeping storedcancellables+navigatorfor migration ergonomics (subscriptions live as long as the VM, which the controller retains).struct Outputrenamed tostruct Publishers; transform return type nowOutput<Publishers, NavigationStep>(wrapper fromNanoViewControllerCore);return Output(…)becomesreturn Output(publishers: Publishers(…), navigation: navigator.navigation). Existing.store(in: &cancellables)andnavigator.next(…)patterns are preserved.populate(with: ViewModel.Output)→populate(with: ViewModel.Publishers).Scene<XView>→NanoViewController<XView>+ControllerConfigProvidingwithstatic let config = ControllerConfig(…).TermsOfServiceuses an instance-levelcontrollerConfigoverride since its layout is chosen at construction time.ControllerConfig+Localization: addsinit(titleKey: LocalizedStringResource, …)so call sites readtitleKey: .CreateNewWallet.titlerather thantitle: String(localized: .CreateNewWallet.title).SignTransactionViewModel: extractedinertOutput()helper sotransformstays inside the SwiftLint function-body length budget after the wrapper-induced verbosity.LockAppScene: moved from removedAbstractControllerto plainUIViewController(no nav chrome, no VM, no bindings).Combine.Publishers: namespace shadowing fixed inMainViewModel+PrepareTransactionViewModel(Combine.Publishers.Merge3).Removed
Sources/AppFeature/Controller/AbstractViewController+BarButtonItem/LeftBarButtonMaking.swift/RightBarButtonMaking.swift— Zhip wrappers over package protocols that no longer exist.Tests/Tests/Navigation/NavigationBarLayoutingNavigationControllerTests.swift— exercised the now-removed publicNavigationBarLayoutOwner; that coverage belongs in the NanoViewController package itself.Tests
output.Xaccess in ViewModel tests rewritten tooutput.publishers.Xacross 16 test files.RestoreWalletViewTestsconstructsPublishersinstead ofOutput.just test).Test plan
just gen && just test— 745 / 745🤖 Generated with Claude Code