From 6ef39451798ac93b0306882a23444a242e8d968f Mon Sep 17 00:00:00 2001 From: jack <212554440+jackjackbits@users.noreply.github.com> Date: Mon, 10 Aug 2026 07:56:12 +0200 Subject: [PATCH] Deflake gift-wrap tests: settle deadlines, not latency budgets (#1651) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Deflake gift-wrap tests: settle deadlines, not latency budgets The gift-wrap round-trip tests failed three CI runs this week (handleGiftWrap_privateMessageStoresConversationAndMapping, handleGiftWrap_deliveredAckUpdatesExistingMessage, handleGiftWrap_routesEmbeddedPrivateMessageAndDeduplicates), each with the same signature: the async NIP-17 unwrap missed a 5s wait on a loaded runner. 40 local iterations of both suites pass clean — the failures are scheduler starvation, exactly the class TestConstants.settleTimeout documents. Every positive wait in ChatViewModelExtensionsTests and ChatNostrCoordinatorContextTests now uses settleTimeout (30s): the explicit 5.0s literals on the gift-wrap waits, the longTimeout media waits, and the bare-default channel-switch waits. All are expected-true waits, so passing runs return immediately and never pay the deadline. Co-Authored-By: Claude Fable 5 * Deflake SimulatedMesh TTL budget: settle discovery before the baseline publicMessageRelaysAcrossLineTopologyWithinTTLBudget snapshotted its frame baseline after a single 2s advance, but discovery is not quiet by then: every first-seen peer schedules an afterglow re-announce at a random 0.3-0.6s delay (BLEAnnounceHandler), and each of those can cascade another relay round. Whether that traffic lands before or after the snapshot depends on the draw — CI measured the "single message" at 14 and 18 frames against a budget of 12. The test now advances until the mesh goes a full window with no new frames before taking the baseline, so the budget only ever measures the message under test. 30 local iterations green. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: jack Co-authored-by: Claude Fable 5 --- .../ChatNostrCoordinatorContextTests.swift | 4 +-- .../ChatViewModelExtensionsTests.swift | 34 +++++++++---------- .../Simulation/SimulatedMeshTests.swift | 16 ++++++++- 3 files changed, 34 insertions(+), 20 deletions(-) diff --git a/bitchatTests/ChatNostrCoordinatorContextTests.swift b/bitchatTests/ChatNostrCoordinatorContextTests.swift index 80d249c4..6200d4bf 100644 --- a/bitchatTests/ChatNostrCoordinatorContextTests.swift +++ b/bitchatTests/ChatNostrCoordinatorContextTests.swift @@ -356,7 +356,7 @@ struct ChatNostrCoordinatorContextTests { // The NIP-17 unwrap runs off the main actor; wait for the hop back. let convKey = PeerID(nostr_: sender.publicKeyHex) - let routed = await TestHelpers.waitUntil({ context.handledPrivateMessages.count == 1 }) + let routed = await TestHelpers.waitUntil({ context.handledPrivateMessages.count == 1 }, timeout: TestConstants.settleTimeout) #expect(routed) #expect(context.recordedNostrEventIDs == [giftWrap.id]) #expect(context.nostrKeyMapping[convKey] == sender.publicKeyHex) @@ -412,7 +412,7 @@ struct ChatNostrCoordinatorContextTests { // The pipeline itself stays usable: a gift wrap spawned AFTER the // wipe (new generation) still decrypts and delivers. coordinator.inbound.handleGiftWrap(giftWrap, id: recipient) - let delivered = await TestHelpers.waitUntil({ context.handledPrivateMessages.count == 1 }) + let delivered = await TestHelpers.waitUntil({ context.handledPrivateMessages.count == 1 }, timeout: TestConstants.settleTimeout) #expect(delivered) } diff --git a/bitchatTests/ChatViewModelExtensionsTests.swift b/bitchatTests/ChatViewModelExtensionsTests.swift index a1d12550..6f9609ee 100644 --- a/bitchatTests/ChatViewModelExtensionsTests.swift +++ b/bitchatTests/ChatViewModelExtensionsTests.swift @@ -277,11 +277,11 @@ struct ChatViewModelNostrExtensionTests { LocationChannelManager.shared.select(channel) defer { LocationChannelManager.shared.select(.mesh) } - _ = await TestHelpers.waitUntil({ LocationChannelManager.shared.selectedChannel == channel }) + _ = await TestHelpers.waitUntil({ LocationChannelManager.shared.selectedChannel == channel }, timeout: TestConstants.settleTimeout) let (viewModel, _) = makeTestableViewModel() - _ = await TestHelpers.waitUntil({ viewModel.activeChannel == channel }) + _ = await TestHelpers.waitUntil({ viewModel.activeChannel == channel }, timeout: TestConstants.settleTimeout) let signer = try NostrIdentity.generate() let event = NostrEvent( @@ -312,7 +312,7 @@ struct ChatViewModelNostrExtensionTests { viewModel.handleNostrEvent(signed) } return false - }, timeout: TestConstants.longTimeout) + }, timeout: TestConstants.settleTimeout) #expect(didAppend) } @@ -463,7 +463,7 @@ struct ChatViewModelNostrExtensionTests { let didUpdate = await TestHelpers.waitUntil( { isDelivered(status: deliveryStatus(in: viewModel, peerID: convKey, messageID: messageID)) }, - timeout: 5.0 + timeout: TestConstants.settleTimeout ) #expect(didUpdate) } @@ -501,7 +501,7 @@ struct ChatViewModelNostrExtensionTests { let didUpdate = await TestHelpers.waitUntil( { isRead(status: deliveryStatus(in: viewModel, peerID: convKey, messageID: messageID)) }, - timeout: 5.0 + timeout: TestConstants.settleTimeout ) #expect(didUpdate) } @@ -529,7 +529,7 @@ struct ChatViewModelNostrExtensionTests { let didStore = await TestHelpers.waitUntil( { viewModel.privateChats[convKey]?.first?.content == "Hello from gift wrap" }, - timeout: 5.0 + timeout: TestConstants.settleTimeout ) #expect(didStore) #expect(viewModel.nostrKeyMapping[convKey] == sender.publicKeyHex) @@ -563,7 +563,7 @@ struct ChatViewModelNostrExtensionTests { // (sent even for blocked senders) to know processing finished. let didAck = await TestHelpers.waitUntil( { viewModel.sentGeoDeliveryAcks.contains(messageID) }, - timeout: 5.0 + timeout: TestConstants.settleTimeout ) #expect(didAck) #expect(viewModel.privateChats[convKey] == nil) @@ -602,7 +602,7 @@ struct ChatViewModelNostrExtensionTests { let didUpdate = await TestHelpers.waitUntil( { isDelivered(status: deliveryStatus(in: viewModel, peerID: convKey, messageID: messageID)) }, - timeout: 5.0 + timeout: TestConstants.settleTimeout ) #expect(didUpdate) } @@ -1022,10 +1022,10 @@ struct ChatViewModelMediaTransferTests { viewModel.sendVoiceNote(at: url) // Media sends hop through Task.detached; the global executor is - // shared with every parallel test worker, so a loaded runner can - // exceed the 5s default. waitUntil returns as soon as the condition - // holds, so passing runs never pay the longer timeout. - let didSend = await TestHelpers.waitUntil({ transport.sentPrivateFiles.count == 1 }, timeout: TestConstants.longTimeout) + // shared with every parallel test worker, so a loaded runner can be + // starved for seconds. waitUntil returns as soon as the condition + // holds, so passing runs never pay the settle deadline. + let didSend = await TestHelpers.waitUntil({ transport.sentPrivateFiles.count == 1 }, timeout: TestConstants.settleTimeout) #expect(didSend) #expect(transport.sentPrivateFiles.first?.peerID == peerID) #expect(viewModel.privateChats[peerID]?.last?.content.contains("[voice]") == true) @@ -1056,7 +1056,7 @@ struct ChatViewModelMediaTransferTests { viewModel.resolveLegacyPrivateMediaConsent(requestID: firstRequestID, approved: true) let showedSecond = await TestHelpers.waitUntil( { viewModel.legacyPrivateMediaConsentRequest?.peerID == secondPeer }, - timeout: TestConstants.longTimeout + timeout: TestConstants.settleTimeout ) #expect(showedSecond) let secondRequestID = try #require(viewModel.legacyPrivateMediaConsentRequest?.id) @@ -1098,7 +1098,7 @@ struct ChatViewModelMediaTransferTests { ) let advanced = await TestHelpers.waitUntil( { viewModel.legacyPrivateMediaConsentRequest?.peerID == secondPeer }, - timeout: TestConstants.longTimeout + timeout: TestConstants.settleTimeout ) #expect(advanced) #expect(decisions.isEmpty, "Invalidation drops the request rather than resolving its send") @@ -1128,7 +1128,7 @@ struct ChatViewModelMediaTransferTests { let didFail = await TestHelpers.waitUntil({ isFailed(status: viewModel.privateChats[peerID]?.last?.deliveryStatus) - }, timeout: TestConstants.longTimeout) + }, timeout: TestConstants.settleTimeout) #expect(didFail) #expect(!FileManager.default.fileExists(atPath: url.path)) #expect(transport.sentPrivateFiles.isEmpty) @@ -1144,7 +1144,7 @@ struct ChatViewModelMediaTransferTests { viewModel.selectedPrivateChatPeer = peerID viewModel.sendImage(from: sourceURL) - let didSend = await TestHelpers.waitUntil({ transport.sentPrivateFiles.count == 1 }, timeout: TestConstants.longTimeout) + let didSend = await TestHelpers.waitUntil({ transport.sentPrivateFiles.count == 1 }, timeout: TestConstants.settleTimeout) #expect(didSend) #expect(transport.sentPrivateFiles.first?.peerID == peerID) #expect(transport.sentPrivateFiles.first?.packet.mimeType == "image/jpeg") @@ -1165,7 +1165,7 @@ struct ChatViewModelMediaTransferTests { let didNotify = await TestHelpers.waitUntil({ viewModel.messages.contains(where: { $0.sender == "system" && $0.content.contains("Failed to prepare image") }) - }, timeout: TestConstants.longTimeout) + }, timeout: TestConstants.settleTimeout) #expect(didNotify) #expect(transport.sentPrivateFiles.isEmpty) #expect(viewModel.privateChats[peerID]?.isEmpty != false) diff --git a/bitchatTests/Simulation/SimulatedMeshTests.swift b/bitchatTests/Simulation/SimulatedMeshTests.swift index c03ed21f..b4630076 100644 --- a/bitchatTests/Simulation/SimulatedMeshTests.swift +++ b/bitchatTests/Simulation/SimulatedMeshTests.swift @@ -56,7 +56,21 @@ struct SimulatedMeshTests { mesh.connect(1, 2) mesh.announceAll() - mesh.advanceTime(by: 2) + // Discovery is not quiet after one advance: every first-seen peer + // schedules an afterglow re-announce at a RANDOM 0.3–0.6s delay + // (BLEAnnounceHandler), and each of those can cascade another relay + // round. Whether that traffic lands before or after a one-shot + // baseline snapshot depends on the draw — the budget assertion below + // flaked on CI at 14 and 18 frames for exactly that reason. Advance + // until the mesh goes a full window with no new frames, so the + // baseline only ever measures the message under test. + var settled = mesh.deliveredFrameCount + for _ in 0..<20 { + mesh.advanceTime(by: 2) + let now = mesh.deliveredFrameCount + if now == settled { break } + settled = now + } let baseline = mesh.deliveredFrameCount let capture = TransportEventCapture()