Harden tests: characterize JSON:API, sessions and keychain; replace double-only tests

[dadachi]

Phase 4 (test hardening). This PR adds characterization tests for the code phase 5 will refactor, enables two dormant tests, replaces tests that only exercised test doubles, and fixes a flaky test. The only production change is a test seam: SessionController.init takes a urlCache: parameter that defaults to .shared.

Coverage was measured on the iPhone (-enableCodeCoverage YES, xcrun xccov). App target: 28.51% → 33.97% lines.

• ShopRepository 0% → 81%, ShopsRequest 0% → 96%, ShopsService 5.6% → 100%
• SessionController 70.5% → 92.6%, KeychainStore 87.2% → 93.6%
• DataCacheUpdate 6.0% → 100%, JSONAPIDocument 65.7% → 100%, JSONAPIResource 69.4% → 100%, JSONAPIRelationship 87.5% → 100%
• MeRequest 0% → 100%, MeService 0% → 85.7%, AccountPasswordRepository/Request 0% → 100%
• ForgotPassword/ResendConfirmationInstructions ViewModels 0% → 100%

Changes

1. Dormant tests and unchecked spies

• ShopkeeperTest.shopkeeperDictionaryHasRequiredFields and additionalEntriesInTheDictionaryAreIgnored had no @Test. Both pass as written, so they revealed no bug.
• Some spy fields were set but never asserted. The tests now check them:
  • PasswordEditViewModelTest: lastUpdatePassword. Every field is sent, and each one is trimmed (the confirmation now ends in \n).
  • ForgotPassword/ResendConfirmationInstructions tests: lastSendResetPassword/lastSendConfirmation, with the email trimmed.
  • ShopkeeperEditViewModelTest: updateCalled, lastSignUp (name, email, the selected time zone, trimming) and destroyCalled.

2. Flaky signingOutRemovesCachedResponses

• Cause: URLCache.shared writes stored responses to disk later, on its own queue. When that write lands after removeAllCachedResponses(), the response comes back, and the final #expect(... == nil) fails.
• Reproduced on the iPhone:
  • The old test with -test-iterations 30, next to SessionControllerPermissionsTest and MainViewModelTest, failed 1 of 30 times at that line.
  • A throwaway probe ran the same steps about 2,400 times under the same load. With URLCache.shared it failed twice, always at the post-logout check. With an in-memory URLCache it never failed.
• Other tests that sign out at the same time also empty the shared cache. That can happen between the store and the #require.
• Fix:
  • SessionController takes urlCache: URLCache = .shared.
  • The test uses its own in-memory cache. It still stores a response, checks that the response is there, signs out, and checks that it is gone.
  • New signOutEmptiesTheSharedCacheByDefault checks that the app's default is still URLCache.shared.

3. Characterization tests (current behavior, passing on main)

• JSONAPIDocumentTest (new):
  • data as an object or an array, in order; included as an array or an object.
  • Document and resource meta; only string links are kept.
  • The weak parent.
  • A missing id or type reads as "" (not the initial "0") and has no EntityIdentity.
  • Each EntityType name (shop, shopkeeper, shopkeeper_sign_in, item_tag); other spellings have none.
  • To-one, to-many and null relationships, whose type is the relationship name.
  • A non-object relationship has no name; a non-object body is an empty document.
• DataCacheUpdateTest (new). No production code calls DataCacheUpdate.loadFrom. Every Request adapts doc.data itself.
  • Shops and shopkeepers go into their own lists, data before included.
  • item_tag and unknown types are skipped.
  • An invalid included resource fails the whole document.
  • The data resources' relationships are collected twice (3 + 3 = 6).
  • Relationships to or from a resource without an EntityType are dropped.
  • merged(with:) keeps the order.
• RootPathServicesTest, where RootPathService differs from Service:
  • It does not announce devise_token_auth's 401 as an expired session.
  • It drops a request's query items.
  • A JSON error without error_message has a nil message.
• KeychainStoreTest:
  • Non-archive bytes and an archive without a root are archiveFailure.
  • An archive of another class is archiveFailure(badData). unarchivedObject returns nil, and the badData thrown inside the do block is wrapped by its own catch.
  • An unreadable item stays in the keychain.
  • A LoggedInShopkeeper archive missing token is archiveFailure. The test first checks that the complete archive reads back. It uses its own keychain item, not the app's session.
• SessionControllerTest / SessionControllerPermissionsTest:
  • Signed-out state, and a saved session that waits for its permissions.
  • Signing in: success resets didExpireSession; a failure stays signed out and rethrows.
  • logout: success, and a failure while signed in, which still clears the session and pops to root.
  • updateShopkeeper with nil and with a new shopkeeper.
  • sessionDidExpire with a nil token or while signed out.
  • A second connection change does not fetch the permissions again.
  • shouldUpdateApp is Int(appBuild) < ios_app_version (build + 1 → true, equal → false).
  • The locale sync: PATCH /shopkeeper_auth with {"locale":"en"} saves the new locale. A rejected sync (devise's 401) keeps the session, because it goes through RootPathService.
  • Confirming the privacy policy and terms: the PATCH paths from MeRequest.swift, and a failure keeps the flags.

4. Real code instead of test doubles

• Removed NativeAppTemplateTests/Demo/ (3 Demo repositories and their 3 tests, 542 lines). Nothing else used them.
  • The isValidEmail cases that only the old Forgot/Resend tests had moved to StringExtensionsTest.
• New ShopRepositoryTest (as in paid #116) runs ShopRepository, ShopsService and the shop requests against TestHTTPServer:
  • reload loads the shops and the limits; a missing meta field fails it.
  • fetchDetail upserts.
  • create and destroy leave shops alone.
  • update replaces the shop; a failed update keeps it.
• New AccountPasswordRepositoryTest: PATCH /{account}/api/v1/shopkeeper/account/password with UpdatePassword.toJson's body; the API error reaches the caller.
• ForgotPasswordViewModelTest and ResendConfirmationInstructionsViewModelTest copied the validation logic inline. They now drive the real ViewModels: validation, trimming, the success message and dismissal, and the fixed error message.
• TestLoginRepository gains loginError.

5. Test-target lint cleanup

• Removed the redundant @Suite attributes (SwiftFormat redundantSwiftTestingSuite, 31 files).
• Replaced == "" and count == 0 with isEmpty.
• TestHTTPServer uses the failable String(bytes:encoding:).
• Removed two redundant #requires that the compiler warned about.

Red checks

I mutated production code in two batches and ran the affected suites on the iPhone. Every new, enabled or strengthened test failed, and the code was then restored.

• Batch 1:
  • JSON:API: ignore an object included; reverse data; drop meta, parent and relationship names; add extra links; default id "0"; rename item_tag; read an array body.
  • DataCacheUpdate: don't pass data relationships to the included pass; swallow included errors; adapt item_tag as a shop; give unknown types an identity.
  • RootPathService: post the expiry notice, send query items, use "" for a missing message.
  • KeychainStore.retrieve: delete the unreadable item and throw notFound.
  • ShopRepository: append on create, skip the upsert append, skip the replace on update, remove on destroy, change the path, drop completed_item_tags_count.
  • AccountPassword: change the path, change the confirmation in the body, swallow the error.
  • Shopkeeper(dictionary:): optional id; reject extra keys.
• Batch 2:
  • SessionController (14 mutations):
    • makeClient uses the wrong token or account.
    • Don't reset didExpireSession on sign-in; don't reset userState after a failed sign-in.
    • No shouldPopToRootView on logout; keep the shopkeeper after a failed logout.
    • updateShopkeeper pops to root and skips saving.
    • Remove the isLoggedIn/nil-token guards in sessionDidExpire.
    • Remove the fetchPermissionsIfNeeded guard; use <= for shouldUpdateApp.
    • Skip setting the synced locale.
    • Don't clear the privacy flag; clear the terms flag before the request.
    • Skip the cache purge; default to a new URLCache.
  • RootPathService: post the expiry notice. rejectedLocaleSyncKeepsTheSession fails.
  • KeychainStore: badData → notFound. Only the another-class test fails.
  • DataCacheUpdate: reverse merged.
  • JSONAPIDocument: ignore an object data.
  • ShopRepository: remove the shop on a failed update; default the missing meta to 0.
  • ViewModels:
    • Forgot/Resend: no trim, the wrong success message, broken isEmailInvalid.
    • PasswordEdit: untrimmed current password, wrong password field.
    • ShopkeeperEdit: untrimmed name, the old time zone, no destroy call.
  • isValidEmail: a regex that allows spaces and digits-less local parts, makes @ optional and has no dotted domains.

Local checks

• make lint: 0 violations, 0 files to format.
• Full suite on the iPhone (iPhone 17 Pro Max, iOS 27.0.1): all pass, with the simulator-only LocalizationTest catalog checks skipped and 1 expected failure (WaitUntilTest).

	Test functions	Test cases (incl. arguments)
main	415 (408 passed, 6 skipped, 1 expected failure)	535 (528 passed, 6 skipped, 1 expected failure)
this PR	437 (430 passed, 6 skipped, 1 expected failure)	574 (567 passed, 6 skipped, 1 expected failure)

• Device check: the app binary changes only by the urlCache: parameter. It defaults to URLCache.shared, which signOutEmptiesTheSharedCacheByDefault checks, so there is no runtime behavior to exercise. The whole suite ran hosted in the app on the iPhone.

Notes for phase 5 (not changed here)

• DataCacheUpdate.loadFrom has no production caller in Free. Free could remove the whole layer, beyond what paid #117 did:
  • relationships, which is collected twice and passed to adapters that never read it.
  • EntityIdentity/EntityRelationship, the EntityAdapter protocol, cacheUpdate:.
  • JSONAPIRelationship, links, the resource meta and parent.
• RootPathService and Service duplicate the request and error handling. They differ on the 401 expiry notice, query items and the source header.
• KeychainStore.retrieve uses try? on keychain.data(for:), which turns any read error into notFound. Its badData comes out as archiveFailure(badData).
• SessionController.logout repeats the same three lines in both branches.
• AccountPasswordRepository.update logs a failure as Failure.destroy instead of .update. I left it alone because it is a bug-fix item.

🤖 Generated with Claude Code

https://claude.ai/code/session_01K5hZEFqXhBsUQwSWsgR6eH