commit 0a20c1bf3600ea51dbf4112026d5f65069cc043c
parent dc6c036f999618beb282cf7f5597e85c08615f3e
Author: Michael Camilleri <[email protected]>
Date: Sat, 23 May 2026 16:10:16 +0900
Isolate NotificationState's UserDefaults per test
NotificationStateTests/dedupIsPerAuthor occasionally failed under parallel
suite execution. The cause was a cross-suite race on the shared App-Group
UserDefaults store that backs NotificationState: recordShown calls
pruneShownEntries(now:), which iterates every notif.shownByGame.* key in the
store and drops anything older than now - 2 * dedupWindow. When
SessionMonitorTests (which exercises production recordShown with the default
now = Date()) ran in parallel with the unit test (which wrote fixtures stamped
at 1970 timestamps), the production prune would wipe the fixture entries
mid-test and wasRecentlyShown assertions would flip.
The same UserDefaults backs every other piece of NotificationState —
activePuzzleID, localActiveUntil, the shown-ping-name list — so the race
surface was wider than the one failing test. GameStoreUnreadMovesTests and
SessionMonitorTests both touched the same keys without their own isolation, and
the .serialized annotations on the two unit suites only serialised within each
suite, not across.
NotificationState.defaults now falls through a @TaskLocal override before
reaching the App-Group store. Production never sets the override so behaviour
is unchanged; tests inject a per-call UUID-named suite via a new
IsolatedNotificationStateTrait (exposed as .isolatedNotificationState) that
wraps the test body in $testingDefaults.withValue(...) and tears the suite down
with removePersistentDomain on the way out. The trait is marked recursive so
applying it once at the suite level invokes provideScope per test rather than
once around the whole suite — each test gets its own isolated store regardless
of how parallel execution interleaves.
The three NotificationState-touching suites adopt the trait, and the
.serialized annotations on the two unit suites — which were only present to
dodge within-suite races on the shared global keys — go away. The TaskLocal
carries a small @unchecked Sendable wrapper because UserDefaults itself isn't
Sendable under strict concurrency, though it is thread-safe in practice.
Co-Authored-By: Claude Opus 4.7 <[email protected]>
Diffstat:
5 files changed, 68 insertions(+), 4 deletions(-)
diff --git a/Shared/NotificationState.swift b/Shared/NotificationState.swift
@@ -41,8 +41,26 @@ enum NotificationState {
/// CKSyncEngine catch-up many times over.
static let shownPingNamesCap = 200
- private static var defaults: UserDefaults? {
+ /// `UserDefaults` itself isn't `Sendable` under strict concurrency, but its
+ /// methods are thread-safe in practice. Vouch for that here so the testing
+ /// override can flow through a `TaskLocal`.
+ struct TestingDefaults: @unchecked Sendable {
+ let userDefaults: UserDefaults
+ }
+
+ /// Test-only override for the storage backend. Production never sets
+ /// this; tests inject a per-test UUID-named `UserDefaults` (typically via
+ /// `.isolatedNotificationState`) so suites no longer mutate the shared
+ /// App-Group store. Implemented as a `TaskLocal` so the override flows
+ /// through actor hops and `Task` continuations inside test bodies — a
+ /// plain settable static would race when suites run in parallel.
+ @TaskLocal static var testingDefaults: TestingDefaults?
+
+ nonisolated(unsafe) private static let sharedDefaults: UserDefaults? =
UserDefaults(suiteName: appGroup)
+
+ private static var defaults: UserDefaults? {
+ testingDefaults?.userDefaults ?? sharedDefaults
}
static func activePuzzleID() -> UUID? {
diff --git a/Tests/Support/TestHelpers.swift b/Tests/Support/TestHelpers.swift
@@ -135,3 +135,49 @@ func makeTestGame() throws -> (Game, GameMutator, GameEntity, PersistenceControl
let mutator = GameMutator(game: game, gameID: gameID, movesUpdater: nil)
return (game, mutator, entity, persistence)
}
+
+// MARK: - NotificationState test isolation
+
+/// Wraps a test (or whole suite) in an isolated `UserDefaults` suite so any
+/// `NotificationState` writes never leak to the shared App-Group store, and
+/// concurrent test suites that touch `NotificationState` can't contaminate
+/// each other. Applied via the `.isolatedNotificationState` sugar below.
+///
+/// The trait routes the override through `NotificationState.$testingDefaults`
+/// (a `TaskLocal`), which means the scope flows naturally through actor hops
+/// and `Task` continuations the test body kicks off. Marked recursive so that
+/// applying it to a suite re-invokes `provideScope` per test rather than once
+/// around the whole suite — without that, parallel sibling tests inside one
+/// suite would share a single override and race on global `NotificationState`
+/// keys (e.g. `activePuzzleID`).
+struct IsolatedNotificationStateTrait: TestTrait, SuiteTrait, TestScoping {
+ var isRecursive: Bool { true }
+
+ func provideScope(
+ for test: Test,
+ testCase: Test.Case?,
+ performing function: @Sendable () async throws -> Void
+ ) async throws {
+ let suiteName = "test.notif.\(UUID().uuidString)"
+ guard let userDefaults = UserDefaults(suiteName: suiteName) else {
+ // The standard suite is always constructable in test environments,
+ // so reaching this branch indicates broken test infrastructure
+ // rather than a recoverable error — run the body unscoped so the
+ // failure surfaces in the assertions instead of being masked.
+ try await function()
+ return
+ }
+ defer { UserDefaults().removePersistentDomain(forName: suiteName) }
+ try await NotificationState.$testingDefaults.withValue(
+ NotificationState.TestingDefaults(userDefaults: userDefaults)
+ ) {
+ try await function()
+ }
+ }
+}
+
+extension Trait where Self == IsolatedNotificationStateTrait {
+ /// Isolates a test or suite's `NotificationState` writes into a per-call
+ /// `UserDefaults` suite. See `IsolatedNotificationStateTrait`.
+ static var isolatedNotificationState: Self { IsolatedNotificationStateTrait() }
+}
diff --git a/Tests/Unit/GameStoreUnreadMovesTests.swift b/Tests/Unit/GameStoreUnreadMovesTests.swift
@@ -7,7 +7,7 @@ import Testing
/// Pins down the Date-based unread-badge heuristic on `GameStore`. A shared
/// game gains an unread badge when another author's `MovesEntity` row has a
/// later `updatedAt` than the local user's last open.
-@Suite("GameStore unread badge", .serialized)
+@Suite("GameStore unread badge", .isolatedNotificationState)
@MainActor
struct GameStoreUnreadMovesTests {
diff --git a/Tests/Unit/NotificationStateTests.swift b/Tests/Unit/NotificationStateTests.swift
@@ -3,7 +3,7 @@ import Testing
@testable import Crossmate
-@Suite("Notification state", .serialized)
+@Suite("Notification state", .isolatedNotificationState)
struct NotificationStateTests {
@Test("Active puzzle stays suppressed through the leave grace, then clears")
func activePuzzleSuppressionClears() {
diff --git a/Tests/Unit/Sync/SessionMonitorTests.swift b/Tests/Unit/Sync/SessionMonitorTests.swift
@@ -65,7 +65,7 @@ private func makeDelta(
)
}
-@Suite("SessionMonitor")
+@Suite("SessionMonitor", .isolatedNotificationState)
struct SessionMonitorTests {
@Test("First delta opens a bucket and schedules the end-of-session notification")
