From 7510ff8481f9673be906a49e3e800cf4007526ab Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Thu, 26 Mar 2026 11:39:32 +1100 Subject: [PATCH 1/2] Prevent repeated upload cancellations Repeated delegate cancellations could keep triggering later queue checks and leak extra expectation fulfills into subsequent tests. Wait for `didFinishUploadingLog` before asserting the successful upload callback path. --- Generated with the help of Codex, https://openai.com Co-Authored-By: Codex --- Sources/Encrypted Logs/EventLogging.swift | 12 +++++++++++- .../EventLoggingUploadManagerTests.swift | 10 +++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/Sources/Encrypted Logs/EventLogging.swift b/Sources/Encrypted Logs/EventLogging.swift index 9834772f..82256640 100644 --- a/Sources/Encrypted Logs/EventLogging.swift +++ b/Sources/Encrypted Logs/EventLogging.swift @@ -21,6 +21,7 @@ public class EventLogging { /// Coordinates one-at-a-time file log dequeuing and upload private let lock = NSLock() private let processingQueue = DispatchQueue(label: "event-logging-upload") + private var uploadsPausedByDelegate = false /// Uploads Events private let uploadManager: EventLoggingUploadManager @@ -98,10 +99,19 @@ extension EventLogging { return } + if uploadsPausedByDelegate { + guard delegate.shouldUploadLogFiles else { + lock.unlock() + return + } + + uploadsPausedByDelegate = false + } + /// If the delegate is reporting that we shouldn't upload log files, pause upload to prevent an endless loop guard delegate.shouldUploadLogFiles else { + uploadsPausedByDelegate = true delegate.uploadCancelledByDelegate(log) - retryUploadsAt(.distantFuture) lock.unlock() return } diff --git a/Tests/Tests/Event Logging/EventLoggingUploadManagerTests.swift b/Tests/Tests/Event Logging/EventLoggingUploadManagerTests.swift index cf45e79a..fe10830e 100644 --- a/Tests/Tests/Event Logging/EventLoggingUploadManagerTests.swift +++ b/Tests/Tests/Event Logging/EventLoggingUploadManagerTests.swift @@ -16,7 +16,15 @@ class EventLoggingUploadManagerTests: XCTestCase { let uploadManager = self.uploadManager(delegate: delegate) waitForExpectation(timeout: 1.0) { exp in - uploadManager.upload(LogFile.containingRandomString(), then: { _ in exp.fulfill() }) + exp.expectedFulfillmentCount = 2 + + delegate.withDidFinishUploadingCallback { _ in + exp.fulfill() + } + + uploadManager.upload(LogFile.containingRandomString(), then: { _ in + exp.fulfill() + }) } XCTAssertTrue(delegate.didStartUploadingTriggered) From 3d32475d175de92e81b2d8c0088ab422f37206ea Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Thu, 26 Mar 2026 16:29:02 +1100 Subject: [PATCH 2/2] Clarify upload pause comments Add brief comments around the delegate pause flow and the two-step test expectation so the flaky-test fix is easier to follow. --- Generated with the help of Codex Co-Authored-By: Codex --- Sources/Encrypted Logs/EventLogging.swift | 4 ++++ .../Tests/Event Logging/EventLoggingUploadManagerTests.swift | 2 ++ 2 files changed, 6 insertions(+) diff --git a/Sources/Encrypted Logs/EventLogging.swift b/Sources/Encrypted Logs/EventLogging.swift index 82256640..743e66c0 100644 --- a/Sources/Encrypted Logs/EventLogging.swift +++ b/Sources/Encrypted Logs/EventLogging.swift @@ -100,11 +100,15 @@ extension EventLogging { } if uploadsPausedByDelegate { + // Delegate cancellation is a policy pause rather than a backoff retry: + // stay idle until uploads are explicitly allowed again guard delegate.shouldUploadLogFiles else { lock.unlock() return } + // Clear the pause state once the delegate allows uploads again so queued + // logs can resume on this run uploadsPausedByDelegate = false } diff --git a/Tests/Tests/Event Logging/EventLoggingUploadManagerTests.swift b/Tests/Tests/Event Logging/EventLoggingUploadManagerTests.swift index fe10830e..31e318a2 100644 --- a/Tests/Tests/Event Logging/EventLoggingUploadManagerTests.swift +++ b/Tests/Tests/Event Logging/EventLoggingUploadManagerTests.swift @@ -16,6 +16,8 @@ class EventLoggingUploadManagerTests: XCTestCase { let uploadManager = self.uploadManager(delegate: delegate) waitForExpectation(timeout: 1.0) { exp in + // The upload callback fires before `didFinishUploadingLog`, so wait for both + // to avoid asserting on delegate state before the upload fully settles exp.expectedFulfillmentCount = 2 delegate.withDidFinishUploadingCallback { _ in