Make the launch and panic-wipe tests able to fail

Three tests here could pass while the behaviour was wrong. A review pass
found all three; the mutation runs below are what settled them.

The launch decision had no test at all. Deleting shouldPresentConversationList
along with the restorability predicate took the seam with it, leaving
`presentation == .conversationList -> showSidebar = true` executed only in
production. That one comparison decides whether a public timeline or a
conversation list ends up on screen. It comes back as a pure one-case
function with both branches asserted.

The panic-wipe test asserted only what launch presents, and an earlier
version of its comment claimed that distinguished "pointer absent" from
"pointer rewritten as .mesh". It does distinguish that one. It does not
distinguish a surviving .direct record, which also presents as
.conversationList - so a regression where finishPanicWipe stopped
deleting the key, while suppression kept working, would leave the peer
id on disk and the test would stay green. For a panic wipe that is the
whole feature. It now reads the stored key directly.

Proved rather than assumed: with the deletion removed from
finishPanicWipe, exactly one of nine tests fails, on the new
XCTAssertNil (ConversationStoreLastActiveTests.swift:162, "45 bytes").
The presentation assertion passes against that mutant, which is the
point - it could not have caught it.

The chrome panic test asserted one arm of isPeopleSheetPresented while
its own comment named both. It now selects a private peer first and
asserts the selection is cleared too, so it covers the cross-model
invariant it claimed rather than the half this PR happens to own.

9 XCTest + 31 swift-testing, 0 failures, exit 0 (macOS).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
ecgang 2026-08-26 10:15:16 -06:00
parent c1baf8392b
commit 2f3524fa8a
3 changed files with 53 additions and 6 deletions

View File

@ -171,11 +171,27 @@ final class AppRuntime: ObservableObject {
/// recent-chats section, so the last conversation is one tap away rather
/// than lost.
private func restoreLastActiveConversationOnLaunch() {
if conversations.restoreLastActiveConversation() == .conversationList {
if Self.shouldPresentConversationList(for: conversations.restoreLastActiveConversation()) {
appChromeModel.showSidebar = true
}
}
/// The launch decision itself, split out as a pure function so the mapping
/// is testable without standing up an `AppRuntime`. It is one comparison,
/// but it is the comparison that decides whether a public timeline or a
/// list ends up on screen, and inverting it silently is exactly the kind of
/// mistake the store-level tests cannot see.
static func shouldPresentConversationList(
for presentation: ConversationStore.LaunchPresentation
) -> Bool {
switch presentation {
case .conversationList:
return true
case .deferToChannelRestore:
return false
}
}
/// Drops media that has outlived the retention window, then applies the
/// explicit protection class to files that older builds wrote without
/// one. Expiry runs first so the migration never touches files the

View File

@ -506,6 +506,12 @@ struct AppArchitectureTests {
// left set covers the root timeline exactly like the others. Launch
// restore (#1064) can raise it without the person touching anything.
chromeModel.showSidebar = true
// The OTHER arm of `isPeopleSheetPresented`. Asserting only the latch
// would leave the test green while a selected DM kept the sheet over
// the wipe result — the selection is cleared by pre-existing code in
// ChatViewModel.panicClearAllData, so this pins a cross-model
// invariant that neither half owns alone.
viewModel.startPrivateChat(with: PeerID(str: "1122334455667788"))
chromeModel.panicClearAllData()
@ -514,6 +520,7 @@ struct AppArchitectureTests {
#expect(!chromeModel.isNoticesSheetPresented)
#expect(chromeModel.showingFingerprintFor == nil)
#expect(!chromeModel.showSidebar)
#expect(viewModel.selectedPrivateChatPeer == nil)
}
@Test("Recent chats list direct conversations with people absent from the rosters")

View File

@ -135,9 +135,15 @@ final class ConversationStoreLastActiveTests: XCTestCase {
// beginPanicWipe → setSelectedPrivatePeer(nil) → setActiveChannel(.mesh) → finishPanicWipe
// Without the fix those resets re-persist a `.mesh` record, and the next
// launch would `.deferToChannelRestore` instead of `.conversationList`.
// That is what makes this assertion load-bearing rather than trivially
// true: `.conversationList` is also the no-record answer, so the test
// distinguishes "pointer absent" from "pointer rewritten as .mesh".
//
// The presentation assertion alone is NOT enough, and the earlier
// version of this comment overclaimed. `.conversationList` is the
// answer for an absent record AND for a surviving `.direct` one, so a
// regression where `finishPanicWipe` stopped removing the key while
// suppression still worked would leave the peer id on disk and this
// test would stay green. For a panic wipe that is the whole point of
// the feature, so assert the stored key is gone, not just that launch
// looks right.
let storage = makeStorage()
let session = ConversationStore(storage: storage)
@ -149,14 +155,32 @@ final class ConversationStoreLastActiveTests: XCTestCase {
session.setActiveChannel(.mesh) // re-persist trigger #2 (suppressed)
session.finishPanicWipe() // removes the pointer once
// A fresh store on the SAME storage finds NO record and falls back to
// the conversation list — the pointer is truly absent, not `.mesh`.
// The pointer is truly absent, not rewritten as `.mesh` and not left
// behind as `.direct`. Read the key directly: this is the only
// assertion that separates "erased" from "still on disk", since both
// present as `.conversationList` at launch.
XCTAssertNil(storage.data(forKey: "conversation.lastActive"))
// And launch presents the list rather than deferring to a resurrected
// `.mesh` record — the suppression half of the same fix.
XCTAssertEqual(
ConversationStore(storage: storage).restoreLastActiveConversation(),
.conversationList
)
}
// MARK: - Launch decision
func test_launchPresentsTheListForAConversationListPresentation() {
XCTAssertTrue(AppRuntime.shouldPresentConversationList(for: .conversationList))
}
func test_launchLeavesAPublicChannelRestoreAlone() {
// Inverting this is how a geohash restore would end up buried under the
// people sheet, or a DM launch would land on the public timeline.
XCTAssertFalse(AppRuntime.shouldPresentConversationList(for: .deferToChannelRestore))
}
func test_firstLaunchPresentsConversationList() {
let storage = makeStorage()