commit b5a2543953234d63eae72ea805d8dbc8106507db
parent d69b0819f9493a101f48f4a68fb0554f3950e836
Author: Michael Camilleri <[email protected]>
Date: Sun, 26 Apr 2026 19:25:30 +0900
Tighten up tests
Diffstat:
6 files changed, 76 insertions(+), 47 deletions(-)
diff --git a/Crossmate/Models/PlayerRoster.swift b/Crossmate/Models/PlayerRoster.swift
@@ -25,7 +25,6 @@ final class PlayerRoster {
private let preferences: PlayerPreferences
private let persistence: PersistenceController
private let container: CKContainer
- private let syncEngine: SyncEngine
private var cachedShare: CKShare?
private var observationTasks: [Task<Void, Never>] = []
@@ -36,8 +35,7 @@ final class PlayerRoster {
authorIdentity: AuthorIdentity,
preferences: PlayerPreferences,
persistence: PersistenceController,
- container: CKContainer,
- syncEngine: SyncEngine
+ container: CKContainer
) {
self.gameID = gameID
self.colorStore = colorStore
@@ -45,7 +43,6 @@ final class PlayerRoster {
self.preferences = preferences
self.persistence = persistence
self.container = container
- self.syncEngine = syncEngine
startObserving()
}
diff --git a/Crossmate/Services/AppServices.swift b/Crossmate/Services/AppServices.swift
@@ -61,13 +61,10 @@ final class AppServices {
Task { await syncEngine.enqueueGame(ckRecordName: ckRecordName) }
}
let colorStore = GamePlayerColorStore()
- store.onGameDeleted = { [syncEngine, colorStore] ckRecordNames in
- if let gameName = ckRecordNames.first(where: { $0.hasPrefix("game-") }),
- let gameID = UUID(uuidString: String(gameName.dropFirst("game-".count))) {
- colorStore.clearColors(forGame: gameID)
- }
- Task { await syncEngine.enqueueDeleteRecords(ckRecordNames) }
- }
+ store.onGameDeleted = Self.makeOnGameDeleted(
+ syncEngine: syncEngine,
+ colorStore: colorStore
+ )
self.colorStore = colorStore
}
@@ -121,7 +118,9 @@ final class AppServices {
preferences: preferences,
persistence: persistence,
authorIdentity: identity,
- syncEngine: syncEngine
+ enqueuePlayerRecord: { [syncEngine] gameID, authorID in
+ await syncEngine.enqueuePlayerRecord(gameID: gameID, authorID: authorID)
+ }
)
await syncEngine.start()
@@ -197,8 +196,7 @@ final class AppServices {
authorIdentity: identity,
preferences: preferences,
persistence: persistence,
- container: ckContainer,
- syncEngine: syncEngine
+ container: ckContainer
)
}
@@ -269,6 +267,22 @@ final class AppServices {
}
}
+ /// Builds the `GameStore.onGameDeleted` callback. Extracted so tests can
+ /// drive the exact same closure that production wires up — keeps the
+ /// colour-cleanup branch from drifting silently.
+ static func makeOnGameDeleted(
+ syncEngine: SyncEngine,
+ colorStore: GamePlayerColorStore
+ ) -> ([String]) -> Void {
+ { ckRecordNames in
+ if let gameName = ckRecordNames.first(where: { $0.hasPrefix("game-") }),
+ let gameID = UUID(uuidString: String(gameName.dropFirst("game-".count))) {
+ colorStore.clearColors(forGame: gameID)
+ }
+ Task { await syncEngine.enqueueDeleteRecords(ckRecordNames) }
+ }
+ }
+
static func run(
_ phase: String,
monitor: SyncMonitor,
diff --git a/Crossmate/Services/NameBroadcaster.swift b/Crossmate/Services/NameBroadcaster.swift
@@ -14,21 +14,25 @@ final class NameBroadcaster {
private let preferences: PlayerPreferences
private let persistence: PersistenceController
private let authorIdentity: AuthorIdentity
- private let syncEngine: SyncEngine
+ private let enqueuePlayerRecord: (UUID, String) async -> Void
private var debounceTask: Task<Void, Never>?
private var observationTask: Task<Void, Never>?
+ /// Testing-only observer fired after each fan-out completes. Receives the
+ /// name that was just broadcast. Production callers should leave this nil.
+ var onFanOutForTesting: ((String) -> Void)?
+
init(
preferences: PlayerPreferences,
persistence: PersistenceController,
authorIdentity: AuthorIdentity,
- syncEngine: SyncEngine
+ enqueuePlayerRecord: @escaping (UUID, String) async -> Void
) {
self.preferences = preferences
self.persistence = persistence
self.authorIdentity = authorIdentity
- self.syncEngine = syncEngine
+ self.enqueuePlayerRecord = enqueuePlayerRecord
startObserving()
}
@@ -81,8 +85,10 @@ final class NameBroadcaster {
)
for gameID in touchedGameIDs {
- await syncEngine.enqueuePlayerRecord(gameID: gameID, authorID: authorID)
+ await enqueuePlayerRecord(gameID, authorID)
}
+
+ onFanOutForTesting?(newName)
}
/// Background-context work — main-actor isolation does not apply here.
diff --git a/Tests/Unit/GamePlayerColorStoreTests.swift b/Tests/Unit/GamePlayerColorStoreTests.swift
@@ -1,3 +1,4 @@
+import CloudKit
import CoreData
import Foundation
import Testing
@@ -168,13 +169,14 @@ struct GamePlayerColorStoreTests {
colorStore.setColor(.red, forGame: gameID, authorID: "_A")
colorStore.setColor(.blue, forGame: gameID, authorID: "_B")
- // Wire cleanup the same way AppServices does after the fix.
- store.onGameDeleted = { [colorStore] ckRecordNames in
- if let gameName = ckRecordNames.first(where: { $0.hasPrefix("game-") }),
- let id = UUID(uuidString: String(gameName.dropFirst("game-".count))) {
- colorStore.clearColors(forGame: id)
- }
- }
+ // Use the production factory so this test fails if the real wiring
+ // ever drops the colour-cleanup branch.
+ let container = CKContainer(identifier: "iCloud.net.inqk.crossmate.v2")
+ let syncEngine = SyncEngine(container: container, persistence: persistence)
+ store.onGameDeleted = AppServices.makeOnGameDeleted(
+ syncEngine: syncEngine,
+ colorStore: colorStore
+ )
try store.deleteGame(id: gameID)
diff --git a/Tests/Unit/NameBroadcasterTests.swift b/Tests/Unit/NameBroadcasterTests.swift
@@ -1,4 +1,3 @@
-import CloudKit
import CoreData
import Foundation
import Testing
@@ -60,13 +59,11 @@ struct NameBroadcasterTests {
persistence: PersistenceController,
authorID: String
) -> NameBroadcaster {
- let container = CKContainer(identifier: "iCloud.net.inqk.crossmate.v2")
- let syncEngine = SyncEngine(container: container, persistence: persistence)
- return NameBroadcaster(
+ NameBroadcaster(
preferences: preferences,
persistence: persistence,
authorIdentity: AuthorIdentity(testing: authorID),
- syncEngine: syncEngine
+ enqueuePlayerRecord: { _, _ in }
)
}
@@ -120,29 +117,44 @@ struct NameBroadcasterTests {
authorID: "_local"
)
- // Allow the observation task to start and register its first
- // withObservationTracking call before we change any values.
+ let spy = FanOutSpy()
+ broadcaster.onFanOutForTesting = { name in spy.record(name) }
+
+ // Allow the observation task to make its first withObservationTracking
+ // registration before we mutate any values.
await Task.yield()
- // First name change — debounce timer #1 starts (250 ms).
+ // First change — debounce timer #1 starts.
prefs.name = "Alice"
- await Task.yield() // let observation task call scheduleDebounce
+ await Task.yield() // observation task → scheduleDebounce
- // Second change before timer fires — cancels #1, starts timer #2.
+ // Second change before timer fires — must cancel #1, start #2.
prefs.name = "Bob"
- await Task.yield() // let observation task cancel #1 and start #2
+ await Task.yield() // observation task → cancel #1, start #2
+
+ // Poll until the (single, hopefully) fan-out fires. Loose deadline so
+ // CI scheduler jitter doesn't fail the test.
+ let deadline = Date().addingTimeInterval(2.0)
+ while spy.count == 0 && Date() < deadline {
+ try await Task.sleep(for: .milliseconds(20))
+ }
- // At 200 ms (< 250 ms debounce), nothing should have been written yet.
- try await Task.sleep(for: .milliseconds(200))
- #expect(fetchPlayerName(authorID: "_local", in: persistence) == nil,
- "debounce should prevent any write before the timer fires")
+ // Grace period to catch a stray second fan-out from an uncancelled
+ // timer — the bug we're guarding against would surface here as count==2.
+ try await Task.sleep(for: .milliseconds(150))
- // At 200 + 300 = 500 ms, timer #2 must have fired.
- try await Task.sleep(for: .milliseconds(300))
- #expect(fetchPlayerName(authorID: "_local", in: persistence) == "Bob",
- "only the final name should be written")
+ #expect(spy.count == 1, "two rapid changes should debounce into one fan-out")
+ #expect(spy.names.last == "Bob", "the final name should win")
+ #expect(fetchPlayerName(authorID: "_local", in: persistence) == "Bob")
// Keep broadcaster alive until assertions are done.
withExtendedLifetime(broadcaster) {}
}
}
+
+@MainActor
+private final class FanOutSpy {
+ private(set) var names: [String] = []
+ var count: Int { names.count }
+ func record(_ name: String) { names.append(name) }
+}
diff --git a/Tests/Unit/PlayerRosterTests.swift b/Tests/Unit/PlayerRosterTests.swift
@@ -87,15 +87,13 @@ struct PlayerRosterTests {
local: UserDefaults(suiteName: "test-pref-\(UUID().uuidString)")!
)
let container = CKContainer(identifier: "iCloud.net.inqk.crossmate.v2")
- let syncEngine = SyncEngine(container: container, persistence: persistence)
return PlayerRoster(
gameID: gameID,
colorStore: store,
authorIdentity: AuthorIdentity(),
preferences: prefs,
persistence: persistence,
- container: container,
- syncEngine: syncEngine
+ container: container
)
}
