diff --git a/.github/workflows/check.yaml b/.github/workflows/check.yaml index 89408e7d..4c0cdea7 100644 --- a/.github/workflows/check.yaml +++ b/.github/workflows/check.yaml @@ -92,8 +92,7 @@ jobs: # https://forums.swift.org/t/warnings-as-errors-for-libraries-frameworks/58393/2 - run: swift build -Xswiftc -warnings-as-errors - # Disabling testing temporarily due to intermittent hangs on CI (https://github.com/ably/ably-chat-swift/issues/295) - #- run: swift test -Xswiftc -warnings-as-errors + - run: swift test -Xswiftc -warnings-as-errors build-release-configuration-spm: name: SPM, `release` configuration (Xcode ${{ matrix.tooling.xcodeVersion }}) diff --git a/Sources/AblyChat/ClockProtocol.swift b/Sources/AblyChat/ClockProtocol.swift index d72ec191..4f925007 100644 --- a/Sources/AblyChat/ClockProtocol.swift +++ b/Sources/AblyChat/ClockProtocol.swift @@ -6,7 +6,7 @@ internal protocol ClockProtocol: Sendable { associatedtype Duration: ClockDuration where Duration: Sendable var now: Instant { get } - func sleep(for duration: Duration) async throws + func sleep(until deadline: Instant) async throws } internal struct SystemClock: ClockProtocol, Sendable { @@ -17,19 +17,16 @@ internal struct SystemClock: ClockProtocol, Sendable { SystemInstant(date: Date()) } - internal func sleep(for duration: SystemDuration) async throws { - if duration.timeInterval > 0 { - try await Task.sleep(nanoseconds: UInt64(duration.timeInterval * 1_000_000_000)) + internal func sleep(until deadline: SystemInstant) async throws { + let duration = deadline.timeInterval(since: now) + if duration > 0 { + try await Task.sleep(nanoseconds: UInt64(duration * 1_000_000_000)) } } } // Protocol representing a point in time -internal protocol ClockInstant: Sendable { - static func < (lhs: Self, rhs: Self) -> Bool - static func > (lhs: Self, rhs: Self) -> Bool - static func == (lhs: Self, rhs: Self) -> Bool - +internal protocol ClockInstant: Sendable, Comparable { func advanced(byTimeInterval timeInterval: TimeInterval) -> Self } @@ -52,6 +49,10 @@ internal struct SystemInstant: ClockInstant, Sendable { SystemInstant(date: date.addingTimeInterval(timeInterval)) } + internal func timeInterval(since other: SystemInstant) -> TimeInterval { + date.timeIntervalSince(other.date) + } + internal static func < (lhs: SystemInstant, rhs: SystemInstant) -> Bool { lhs.date < rhs.date } diff --git a/Sources/AblyChat/TimerManager.swift b/Sources/AblyChat/TimerManager.swift index 4552fed1..0658cd72 100644 --- a/Sources/AblyChat/TimerManager.swift +++ b/Sources/AblyChat/TimerManager.swift @@ -12,8 +12,19 @@ internal final class TimerManager { internal func setTimer(interval: TimeInterval, handler: @escaping @MainActor () -> Void) { cancelTimer() + // Calculate the deadline before kicking off the asynchronous work. This is so that using TestClock's advance(by:) behaves reliably; we want to be sure that even if the Task below is scheduled by the system long after advance(by:) is called, its `sleep` call will still return in response to an advance(by:) call that causes `interval` to have elapsed on the clock relative to the time when `setTimer` was called. + let deadline = clock.now.advanced(byTimeInterval: interval) + currentTask = Task { - try? await clock.sleep(for: .seconds(interval)) + // This is for compatibility with the TestClock that we use in the tests; calling `sleep(until:)` with a deadline equal to the current time does _not_ make `sleep(until:)` return immediately; see https://github.com/pointfreeco/swift-clocks/issues/23 (it is not clear from the comments there whether or not this should be considered a misbehaviour; let's handle it either way). + // + // (You might ask "why would you call sleep(until:) with a deadline equal to the current time?", but bear in mind — per the above comment about Task scheduling — that this task might get scheduled long after time has advanced relative to the moment that setTimer was called.) + if clock.now >= deadline { + handler() + return + } + + try? await clock.sleep(until: deadline) guard !Task.isCancelled else { return } diff --git a/Tests/AblyChatTests/Mocks/MockTestClock.swift b/Tests/AblyChatTests/Mocks/MockTestClock.swift index d2082905..af2662b4 100644 --- a/Tests/AblyChatTests/Mocks/MockTestClock.swift +++ b/Tests/AblyChatTests/Mocks/MockTestClock.swift @@ -14,8 +14,8 @@ final class MockTestClock: ClockProtocol { SwiftTestInstant(instant: testClock.now) } - func sleep(for duration: SwiftTestDuration) async throws { - try await testClock.sleep(for: duration.duration) + func sleep(until deadline: SwiftTestInstant) async throws { + try await testClock.sleep(until: deadline.instant) } func advance(by: TimeInterval) async { diff --git a/Tests/AblyChatTests/TypingTimerManagerTests.swift b/Tests/AblyChatTests/TypingTimerManagerTests.swift index 3efdf2ae..38e9984f 100644 --- a/Tests/AblyChatTests/TypingTimerManagerTests.swift +++ b/Tests/AblyChatTests/TypingTimerManagerTests.swift @@ -1,6 +1,7 @@ @testable import AblyChat import Clocks import Foundation +import Semaphore import Testing @MainActor @@ -89,8 +90,10 @@ final class TypingTimerManagerTests { var handlerCalled = false + let semaphoreSignalledByHandler = AsyncSemaphore(value: 0) timerManager.startTypingTimer(for: "client1") { handlerCalled = true + semaphoreSignalledByHandler.signal() } #expect(timerManager.isCurrentlyTyping(clientID: "client1")) @@ -98,6 +101,7 @@ final class TypingTimerManagerTests { // Advance time to trigger timer expiration (heartbeatThrottle + gracePeriod) await mockClock.advance(by: 1.6) + await semaphoreSignalledByHandler.wait() #expect(handlerCalled) #expect(!timerManager.isCurrentlyTyping(clientID: "client1")) #expect(timerManager.currentlyTypingClientIDs().isEmpty) @@ -149,8 +153,10 @@ final class TypingTimerManagerTests { var handlerCalled = false + let semaphoreSignalledByHandler = AsyncSemaphore(value: 0) timerManager.startTypingTimer(for: "client1") { handlerCalled = true + semaphoreSignalledByHandler.signal() } // Advance by heartbeatThrottle only - should still be typing @@ -162,6 +168,7 @@ final class TypingTimerManagerTests { // Advance by grace period - now should expire await mockClock.advance(by: 0.5) + await semaphoreSignalledByHandler.wait() #expect(handlerCalled) #expect(!timerManager.isCurrentlyTyping(clientID: "client1")) }