test: isolate remaining suites from the app's state

[jvsena42]

Stack — review in order; each targets the one before it.

1. fix: keep transfer unit tests out of the app's UserDefaults #756 — keep transfer unit tests out of the app's UserDefaults (fixes fix: hardware transfer unit tests write mock transfers into the app's UserDefaults #733)
2. fix: stop the test suite wiping the simulator's wallet #757 — stop the test suite wiping the simulator's wallet
3. test: isolate remaining suites from the app's state #758 — isolate remaining suites from the app's state

This PR finishes isolating the unit test target from the app's own state, covering everything the audit turned up that is not the wallet itself.

Between them these suites were resetting around thirty real settings keys, including whether a PIN is enabled, whether biometrics are on and whether payments require a PIN. They were also dropping the user's Blocktank refund address, deleting the bolt11 the app published to their homeserver along with its payment hash and expiry, writing fake contacts into the real private Paykit cache, deleting the home-screen widget layout, clearing the address type settings, wiping the downloaded avatar cache, and overwriting the widget options the home-screen extension reads from the shared app group.

Most of it is snapshot and restore. Where a suite's code takes injected defaults it gets an isolated suite instead. For the settings resets there is a whole-domain snapshot, because enumerating thirty keys is a maintenance trap as settings are added, and restoring the domain also removes keys a test added.

One detail is load-bearing and cost a round trip to find: XCTest runs teardown blocks before tearDown(), not after. Several suites cleared the same key in tearDown that the snapshot had just restored, so the restore was silently defeated. The suites stayed green the whole time — only diffing the persistent domain showed it. Those teardowns are removed and the ordering is documented at the top of the helpers file.

Two audit findings turned out to be wrong on contact with the code, so the fixes differ from what was expected. The corrupt-cache test does clean up after itself; the real defect in that file was the setUp and tearDown clearing the user's own refund address. And the image cache needed no new production seam at all — it already takes a directory, and four of its five tests already injected one, so only the odd one out needed changing.

The currency view model could not be fixed by injection the way it looked like it could. Its initialiser syncs the display currency into the shared app group regardless of which service is passed, so a stub service stops the network call but not the write that outlives the test. Those suites snapshot the app-group keys instead.

Two things are deliberately left. The rate cache can still be written by a detached poll that lands after teardown — no snapshot can win that race, it needs a way to opt out of polling. And the core service fires its database init once synchronously and again asynchronously, so a suite's own init can still be overridden; closing that needs a way to drain the queue that does not exist yet. The database suites do now get their own directory per run, removed wholesale, which also catches the second database file that none of them were cleaning up.

No changelog fragment: test-only changes.

Linked Issues/Tasks

Follows #757. Found while auditing the suite for #733.

Design

N/A — no UI changes.

QA Notes

Manual Tests

• 1a. Note your widgets, currency, address type and PIN settings → run the full unit lane → launch the app: all unchanged.
  • 1b. regression: home screen widget: still configured as before.
• 2. regression: Contacts with a Pubky identity: avatars still present, not re-downloading.

Automated Checks

• Unit tests modified: fifteen suites across settings, Paykit, widgets, address types, image cache, database and currency now snapshot or isolate what they touch.
• Test helpers added: whole-domain and app-group snapshots in BitkitTests/AppStateIsolation.swift.
• Verified locally on iPhone 17 against a live wallet: 1293 tests, 12 skipped, 0 failures, with the app's persistent domain and the shared app group unchanged apart from the rate cache noted above, and nothing removed.
• CI: standard build and test checks run by the PR bot.

[greptile-apps]

Confidence Score: 5/5

The PR appears safe to merge; the previously reported background currency-write and queued database-initialization gaps are fully addressed.

Summary

This PR isolates unit tests from persistent application and extension state.

• Snapshots and restores app-domain and shared app-group defaults touched by settings, Paykit, widgets, address-type, and currency tests.
• Uses isolated image-cache and database directories with complete cleanup.
• Drains queued core initialization before test database initialization.
• Injects an offline currency service to prevent background rate refreshes from escaping teardown.
• Both previous findings are fully addressed in the current code.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    Setup[Test setUp] --> Snapshot[Snapshot app and app-group state]
    Setup --> Drain[Drain queued core initialization]
    Setup --> Temp[Create isolated defaults/cache/database storage]
    Setup --> Offline[Inject offline currency service]
    Snapshot --> Test[Run test]
    Drain --> Test
    Temp --> Test
    Offline --> Test
    Test --> Restore[Teardown blocks restore persistent state]
    Test --> Cleanup[Remove temporary storage]

Loading

_{Reviews (2) · Last reviewed commit: "fix: stop paykit tests deleting the rece..."}

[jvsena42]

Four MEDIUM (all test-only) and one LOW, each independently verified. Details are inline. Two of them share a root cause: setPersistentDomain restores disk but not SettingsViewModel.shared's @AppStorage cache, so later suites read, and write back, the values a reset left in memory. SettingsViewModel.swift:211 and syncAppStorageFromDefaults() already exist because of this. Making that re-sync internal and calling it from the snapshotAppDefaultsDomain teardown block would close both.

Checked and clean:

• CurrencyService.init widening: the only production construction is still static let shared, and the file is not in the widget or notification targets.
• OfflineCurrencyService: refresh() fails, so neither the cached_fx_rates write nor the app-group mirror happens.
• Host app during unit tests renders Text("Running tests..."), so there are no concurrent app writes for the domain restore to clobber.
• persistentDomain / setPersistentDomain round-trip plist types and drop keys the test added. The app-group snapshot handles absent keys.
• The widget suites cover savedWidgets and the four home_screen_*_widget_options_v1 keys. The SamRockSetupRequestTests nested snapshots restore LIFO.
• In PrivatePaykitServiceTests, the in-test defer restores run before the domain restore. QuickPaySpendStore has no in-memory cache.
• drainCoreServiceQueue works for ActivityTests and BlocktankTests: .core is serial and the queued initDb blocks on it.

[ovi-reviewer]

Verdict: ✅ Approve

---

Review: diff 23 files.

Findings:
N/A

Audit:
Awaits Manual Tests.

Coverage:
QA: Manual Tests await all reviewers to approve, author can run it now via comment: @ovi-reviewer test

---

Reviewed by grok-4.6-xhigh via gh-pr-review-loop skill
Commands: @ovi-reviewer test · retest · audit (author or owner)

[piotr-iohk]

QA reviewed on a90e7c3.

Reviewed the domain/app-group snapshot helpers, OfflineCurrencyService seam, core-queue drain, and the fifteen suites that stopped writing through to the host app; no device run this round, test-only / no UI.

Run Tests is green. Run Integration Tests is red on PaymentFlowTests / UtxoSelectionTests: Blocktank 404 on /regtest/chain/deposit. Those files only dropped an unused testDbPath; the deposit call is unchanged from bitkit-ios#757, which is green in the same window. Treating that as infra, not this isolation work.

No findings.

Checked and clean

• Whole-domain snapshot plus the XCTest teardown-block-before-tearDown ordering; suites that used to clear the same key in tearDown dropped that path so the restore stands.
• OfflineCurrencyService keeps CurrencyViewModel.refresh from writing cached_fx_rates / the app-group mirror after restore; production still only constructs CurrencyService.shared.
• Widget suites snapshot savedWidgets and all four home_screen_*_widget_options_v1 keys; the image-cache test that used .shared now injects a temp directory like the rest of that file.
• PrivatePaykitServiceTests snapshots the whole domain, so the reservation store deleting onchainAddress is covered, not only the one cache key.
• Activity/Blocktank/Transfer DB suites use a unique temp dir, drain both ServiceQueue copies, then re-point initDb before unlink.

QA LGTM

[jvsena42]

Test 1a, fixed in 351519f.

Root cause. The leak came from PaymentFlowTests (ChannelPurchaseFlow.swift), not from AddressTypeSettingsTests. testchannelPurchaseFlow calls StartupHandler.createNewWallet, which writes selectedAddressType = nativeSegwit and addressTypesToMonitor = nativeSegwit for the new wallet. The suite had no defaults snapshot, so nothing put the values back. UtxoSelectionTests calls the same function and had the same gap. I polled the app's plist during a full lane: the value flipped from taproot to nativeSegwit during PaymentFlowTests and did not change back.

The same lane also left addressSearch_lastUsedReceiveIndex_taproot behind. A running node writes that key when a transaction arrives. The integration suites stop the node in tearDown(), and XCTest runs tearDown() after the teardown blocks, so the node could still write after the domain restore had already run.

Fix (test-only). In PaymentFlowTests and UtxoSelectionTests, setUp now calls snapshotAppDefaultsDomain() and then registers a teardown block that stops the node. AddressTypeIntegrationTests and BlocktankRefundAddressLiveIntegrationTests already had the snapshot and now get the same stop block. Teardown blocks run last-in, first-out, so the node stops before the restore. StartupHandler behaviour is unchanged: a new wallet should reset the address type.

AddressTypeSettingsTests on its own. I could not reproduce the leak. With a Taproot/EUR/requirePinForPayments = true baseline, I ran the suite alone (28 tests, 0 failures) and the app's plist was byte-identical afterwards, both before and after the fix. xcrun simctl spawn <udid> defaults read to.bitkit reads the simulator's global domain, not the app container, and reports every key as absent. That may explain the "absent" reading. Reading <data container>/Library/Preferences/to.bitkit.plist shows the real values.

Verification (iPhone 17, iOS 26, Taproot baseline, diffing plutil -convert xml1 of the container plist):

Run	Result	Domain diff
Full BitkitTests lane at 4108031	1327 tests, 12 skipped, 0 failures	selectedAddressType taproot → nativeSegwit, addressTypesToMonitor nativeSegwit,taproot → nativeSegwit, addressSearch_lastUsedReceiveIndex_taproot added
PaymentFlowTests + UtxoSelectionTests at fix	3 tests, 0 failures	none
AddressTypeSettingsTests at fix	28 tests, 0 failures	none
Full BitkitTests lane at fix	1327 tests, 12 skipped, 0 failures	none (plist and defaults export both identical)

After the lane, relaunching the app shows Address Type still set to Taproot:

The weather_widget_latest_v1 app-group note is not covered by this commit.

[ovi-reviewer]

Advice: ✅ Approve

---

Reaudit: diff 4 files.

Findings:
N/A

Audit:
Already done in comment.

Coverage:
QA: journeys and manual tests await all reviewers to approve, author can run it now via comment: @ovi-reviewer test

---

Reviewed by claude-opus-5-xhigh via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest · audit (author or owner)

[jvsena42]

Weather widget note: fixed in 64c81c6.

NumberPadTests.mockCurrency sets selectedCurrency = "USD". The setter calls WeatherViewModel.shared.handleCurrencyChange(), which re-formats the cached fee and rewrites weather_widget_latest_v1 and weather_widget_latest_timestamp_v1 in group.bitkit. The suite now adds both keys to its existing snapshotAppGroupDefaults call. The write is synchronous inside the test body, so the teardown restore always runs after it.

Verification on iPhone 17, with the group seeded with the € 0.11 payload and both domains read through defaults export:

Run	Tests	group.bitkit diff	app domain diff
NumberPadTests before fix	28, 0 failures	€ 0.11 → $ 0.12, timestamp rewritten	n/a
NumberPadTests at fix	28, 0 failures	none	n/a
Full BitkitTests lane at fix	1327, 12 skipped, 0 failures	none	none

The on-disk plists of both domains are also byte-identical after the lane.

[ovi-reviewer]

Advice: ✅ Approve

---

Reaudit: diff 1 file.

Findings:
N/A

Audit:
Already done in comment.

Coverage:
QA: journeys and manual tests await all reviewers to approve, author can run it now via comment: @ovi-reviewer test

---

Reviewed by claude-opus-5-xhigh via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest · audit (author or owner)

[piotr-iohk]

QA reviewed on 64c81c6.

Reviewed the address-type restore on the four wallet-creating suites and the weather-widget keys on NumberPadTests; no device run this round, test-only / no UI.

Run Tests is green. Run Integration Tests is green.

No findings.

Checked and clean

• PaymentFlowTests and UtxoSelectionTests snapshot the whole defaults domain, then register a node stop that runs before that restore. tearDown still stops the node, which is a no-op once the first stop finished, and wipeStorage does not write preferences.
• AddressTypeIntegrationTests and BlocktankRefundAddressLiveIntegrationTests use the same stop-before-restore order. createNewWallet is only called from these four suites.
• NumberPadTests is the only suite that sets selectedCurrency. That setter rewrites weather_widget_latest_v1 and weather_widget_latest_timestamp_v1, and both keys are in the app-group snapshot. The fee-percentile key is not on this path.
• The isolation helpers and the CurrencyService test seam are unchanged since the last pass. No Android twin.

QA LGTM

[ovi-reviewer]

Advice: ✅ Approve

---

Reaudit: diff 1 file.
No new findings; the rest is in the review.

Audit:
Already done in review.

QA:
Tested on iOS 26.5 simulator (iPhone 17 Pro)
Test 2 already done in review.

Test 1 ✅ passed

evidence

1.mp4

Tip

Worth a journey

Test 1

• Set local currency to EUR
• Set address type to Taproot
• Enable PIN 1234 and require PIN for payments
• Remove Bitcoin Facts from the Home widget layout
• Set Bitcoin Price to BTC/EUR over Week
• Verify the Weather current fee is shown in euros
• Run the full BitkitTests target
• Relaunch Bitkit and enter PIN 1234
• Verify currency, address type, and PIN settings are unchanged
• Verify the widget layout, Bitcoin Price options, and Weather euro fee are unchanged

Coverage:
Unit tests: 100% - snapshots and restores the two weather-widget app-group keys rewritten when the selected currency changes.
QA: 2 of 2 manual tests passed

---

Reviewed by gpt-5.6-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest · audit (author or owner)