Files
bitchat/bitchatTests/PTTBurstPlayerTests.swift
T
0152196ac2 Deflake CI, and make the flake class unrepeatable (#1491)
* Deflake two iOS-sim CI tests with unbounded timing assumptions

Both failed on main-adjacent CI runs during this session's PRs. Neither
was a product bug; both asserted things about real time that a loaded
runner is under no obligation to honour.

**NetworkReachabilityGateTests: a wall-clock upper bound.**
test_monitor_duplicateUpdatesDoNotPostponeOfflineCommit slept 500ms for
real, then asserted total elapsed time was under 1.4s to prove a duplicate
mid-window had not restarted the 1.0s debounce. One CI run took 3.75s. No
wall-clock bound can separate "deadline preserved" from "runner is slow",
because Task.sleep and the asyncAfter flush are both real time and neither
is bounded above.

The deadline property was already covered deterministically one level
down: test_debounce_duplicateObservationsPreservePendingDeadline drives
ReachabilityDebounce with injected timestamps and checks pendingRemaining
directly. So the monitor test now asserts only what needs a real monitor —
that a duplicate still yields exactly one committed false through the
debounce — with an injected clock for the arithmetic and a generous
liveness budget. Renamed to say what it actually checks. No coverage lost,
and it runs in 0.14s instead of ~3.8s because the real sleep is gone.

**NoiseEncryptionServiceTests: injected timeouts that also arm during
setup.** #1483 diagnosed and fixed exactly this in the quarantine-restore
test, but two sibling tests kept the shape. Their injected
ordinaryResponderHandshakeTimeout (0.04 and 0.06) also arms during the
establishSessions setup handshake, where bob is the responder — so a
preempted runner fires it mid-setup, tears down the half-open responder,
and message 3 gets answered as a fresh initiation. The CI failure named
setup's own `#expect(finalMessage == nil)` seeing a 96-byte message 2,
which is precisely the signature #1483 recorded.

Raised both to 1.0s, matching #1483's remedy: the scenarios still need the
responder timeout to fire, and it still does, just with room for setup to
complete first. Thin 1-second waitUntil budgets in the file now use the
shared TestConstants.longTimeout; every one of them backs a positive
assertion, so waitUntil still returns the moment the condition holds and
nothing gets slower in the passing case.

No assertion weakened in either file. Verified 3x sequentially and 3x with
all 18 cores saturated.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Raise two more starvation-prone test deadlines

Both surfaced on the CI runs for this PR and #1487, both in tests neither
PR touches, both the same shape as the two already fixed here: a deadline
sized for the work rather than for a runner executing many suites at once.

VoiceNotePlaybackControllerTests waited 5s for a @MainActor Task that
playback schedules for the session acquire and its failure path. When that
Task is not scheduled in time the helper reports *two* failures — the wait,
and the `!isPlaying` the un-run failure path has not reset yet — which
reads like a playback bug rather than a starved scheduler. That is exactly
what CI showed.

GeoRelayDirectoryTests waited 10s for work the directory runs in
Task.detached(priority: .utility); utility priority competes with every
other suite. The retry-scheduling case timed out at exactly 10.06s with the
retry never scheduled, reading like a missing retry rather than a starved
background task.

Raised to 30s each with the reasoning recorded at the helper. Both helpers
return as soon as their condition holds, so nothing slows down when tests
pass — only the genuine-failure case takes longer to report.

Verified 3x with all cores saturated: both suites clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Make the flake class unrepeatable, not just fixed

Raising four deadlines fixed the four tests that happened to fire. The
class was still there: fourteen separate waitUntil helpers, most defaulting
to 1.0s, plus wait call sites with literal budgets. The fifth instance
would have landed the same way, on someone else's unrelated PR.

The rule, now written down in TestConstants.settleTimeout: **a wait
deadline is not a latency budget.** It exists so a genuine hang eventually
fails the suite, so size it for the worst-case scheduler, never for how
long the operation should take. Waits return as soon as their condition
holds, so a generous deadline is free in the passing case and only extends
genuine failures.

- Every wait helper now defaults to TestConstants.settleTimeout (30s), and
  the literal wait call sites below the floor were converted too — sixteen
  sites across ten files.
- TestTimingHygieneTests enforces it by scanning the test sources: wait
  defaults and wait call sites must be at least minimumSettleTimeout, and
  no test may assert an upper bound on elapsed wall-clock time (the
  assertion that started this, which cannot separate correct behaviour from
  a slow machine).
- Both rules waive per line with "test-timing-ok: <reason>", accepted on
  the line or in the comment block above it so the reason has room to be a
  sentence. One legitimate use so far: a NEGATIVE wait in NoiseCoverageTests
  asserting a promotion has *not* completed within 50ms, where a long
  deadline would only make the suite slow while still passing.

Injected production timeouts are deliberately not matched — the Noise
handshake timeouts are the behaviour under test, and short values are
correct there.

Verified the guard actually fails: a canary file with both banned shapes
was flagged with file and line, and the suite went green again once it was
removed. Full suite passes with all 18 cores saturated.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Close the guard's blind spot: named short timeouts

A fifth flake landed on CI while the first guard was in review —
GossipSyncBoardTests timing out at 1.03s — and the guard did not catch it,
because the deadline was `TestConstants.shortTimeout` rather than a
literal. A literals-only scan cannot see a short value behind a symbol,
which is the more common way it is written: 81 wait sites used
shortTimeout (1s) or defaultTimeout (5s), every one of them below the
floor.

Rather than guess which of those were safe to raise, all 81 were converted
and the suite timed. Runtime went 15s -> 72s, which located the genuine
negative waits precisely: ten tests that assert something does *not*
happen and therefore always run their deadline out. Measurement instead of
a heuristic, since a mis-guess in either direction is invisible — too
short reintroduces the flake, too long silently costs a minute a run.

Those 28 sites now use `TestConstants.negativeWaitWindow`, a named
constant whose doc explains the inverted reasoning: for a negative wait,
starvation can only make the assertion *more* likely to hold, so short is
correct, and the name states the polarity instead of leaving a bare
literal that reads like the mistake. Suite is back to 15.3s.

The guard now also rejects `timeout: TestConstants.shortTimeout` and
`defaultTimeout`, while accepting `negativeWaitWindow` by name.

Re-verified with a canary carrying every banned shape — literal default,
both named constants, and an elapsed-time upper bound. All four were
flagged with file and line; the suite went green again on removal.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Delete shortTimeout now that nothing may use it

The hygiene guard bans `TestConstants.shortTimeout` at every wait site
and the last users were converted, so Periphery correctly flagged the
constant itself as dead and failed CI. Remove it from both TestConstants
copies; the banned-name entry stays so the symbol cannot quietly return.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: jack <jackjackbits@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-07-26 23:45:20 +01:00

471 lines
18 KiB
Swift

//
// PTTBurstPlayerTests.swift
// bitchatTests
//
// This is free and unencumbered software released into the public domain.
// For more information, see <https://unlicense.org>
//
import Testing
import AVFoundation
import Foundation
@testable import bitchat
/// Thread-safe: the coordinator invokes it on its private serial queue.
private final class StubAudioSession: SessionApplying, @unchecked Sendable {
private let lock = NSLock()
private var _setCategoryError: Error?
var setCategoryError: Error? {
get { lock.withLock { _setCategoryError } }
set { lock.withLock { _setCategoryError = newValue } }
}
func setCategory(_ category: AudioSessionCoordinator.Category) throws {
try lock.withLock {
if let error = _setCategoryError {
_setCategoryError = nil
throw error
}
}
}
func setActive(_ active: Bool, notifyOthersOnDeactivation: Bool) throws {}
}
private struct StubSessionError: Error {}
private final class StubExclusivePlayback: ExclusivePlayback {
private(set) var pauseCount = 0
func pauseForExclusivity() {
pauseCount += 1
}
}
/// Blocks activation until released, so a test can land events inside the
/// window where the (off-main) session acquire is still in flight.
private final class GatedAudioSession: SessionApplying, @unchecked Sendable {
private let gate = DispatchSemaphore(value: 0)
private let lock = NSLock()
private var _categoryCallCount = 0
private var _activationCalls: [Bool] = []
/// Non-zero once the acquire has reached the session queue (setCategory
/// runs just before the gated setActive).
var categoryCallCount: Int { lock.withLock { _categoryCallCount } }
var activationCalls: [Bool] { lock.withLock { _activationCalls } }
func open() { gate.signal() }
func setCategory(_ category: AudioSessionCoordinator.Category) throws {
lock.withLock { _categoryCallCount += 1 }
}
func setActive(_ active: Bool, notifyOthersOnDeactivation: Bool) throws {
if active { gate.wait() }
lock.withLock { _activationCalls.append(active) }
}
}
@MainActor
private final class MockPlaybackEngine: PTTPlaybackEngine {
private(set) var startCount = 0
private(set) var stopCount = 0
private(set) var scheduledBuffers: [AVAudioPCMBuffer] = []
private struct HeldCompletion {
let type: PTTPlaybackCompletionType
let callback: @Sendable (PTTPlaybackCompletionEvent) -> Void
}
private var heldCompletions: [HeldCompletion] = []
private(set) var requestedCompletionTypes: [PTTPlaybackCompletionType] = []
var startError: Error?
// No object -> the player registers no configuration-change observer.
var configChangeObject: AnyObject? { nil }
func start() throws {
if let error = startError { throw error }
startCount += 1
}
func play() {}
func stop() {
stopCount += 1
}
func schedule(
_ buffer: AVAudioPCMBuffer,
completionType: PTTPlaybackCompletionType,
completionHandler: @escaping @Sendable (PTTPlaybackCompletionEvent) -> Void
) {
// Completions are held, not fired automatically: most tests exercise
// lifecycle, not drain-out. The mock models the important distinction
// between the node consuming bytes and audio actually playing out.
scheduledBuffers.append(buffer)
requestedCompletionTypes.append(completionType)
heldCompletions.append(HeldCompletion(type: completionType, callback: completionHandler))
}
/// Advances the node only to the point where it has consumed each
/// buffer. A `.dataPlayedBack` request must remain pending here.
func fireDataConsumedCallbacks() {
for completion in heldCompletions {
completion.callback(.dataConsumed)
}
}
/// Advances all scheduled audio through audible playback.
func fireDataPlayedBackCallbacks() {
let completions = heldCompletions
heldCompletions = []
for completion in completions {
completion.callback(.dataPlayedBack)
}
}
/// Models AVAudioPlayerNode flushing its callbacks because an external
/// engine reconfiguration stopped the node before MainActor rebuilt it.
func firePlaybackStoppedCallbacks() {
let completions = heldCompletions
heldCompletions = []
for completion in completions {
completion.callback(.playbackStopped)
}
}
/// Plays only the oldest scheduled buffer, leaving the rest as an
/// audible tail that an engine rebuild must preserve.
func fireNextDataPlayedBackCallback() {
guard let index = heldCompletions.firstIndex(where: { $0.type == .dataPlayedBack }) else { return }
let completion = heldCompletions.remove(at: index)
completion.callback(.dataPlayedBack)
}
}
@MainActor
struct PTTBurstPlayerTests {
private func makePlayer(
coordinator: AudioSessionCoordinator
) throws -> (player: PTTBurstPlayer, engines: () -> [MockPlaybackEngine]) {
final class EngineBox { var engines: [MockPlaybackEngine] = [] }
let box = EngineBox()
// Fresh exclusivity slot: parallel tests must not steal this player's
// app-wide playback slot mid-test (the async session acquire opens
// suspension windows the old synchronous start never had).
let player = try #require(PTTBurstPlayer(
coordinator: coordinator,
exclusivity: VoiceNotePlaybackCoordinator(),
makeEngine: {
let engine = MockPlaybackEngine()
box.engines.append(engine)
return engine
}
))
return (player, { box.engines })
}
/// Enough encoded audio to cross `TransportConfig.pttJitterBufferSeconds`
/// so playback starts without waiting for the deadline task.
private func encodeSineFrames(seconds: Double = 1.0) throws -> [Data] {
let encoder = try #require(PTTFrameEncoder())
let format = try #require(PTTAudioFormat.pcmFormat)
let totalFrames = AVAudioFrameCount(seconds * PTTAudioFormat.sampleRate)
let buffer = try #require(AVAudioPCMBuffer(pcmFormat: format, frameCapacity: totalFrames))
buffer.frameLength = totalFrames
let channel = try #require(buffer.floatChannelData?[0])
for i in 0..<Int(totalFrames) {
channel[i] = sinf(2 * .pi * 440 * Float(i) / Float(PTTAudioFormat.sampleRate)) * 0.5
}
return encoder.encode(buffer)
}
/// The jitter-buffered start now acquires the session asynchronously
/// (its blocking IPC runs off the main actor), so tests await the
/// condition instead of asserting right after `enqueue`.
private func waitUntil(
_ condition: () -> Bool,
sourceLocation: SourceLocation = #_sourceLocation
) async {
let deadline = ContinuousClock.now.advanced(by: .seconds(TestConstants.settleTimeout))
while !condition(), ContinuousClock.now < deadline {
await Task.yield()
try? await Task.sleep(nanoseconds: 1_000_000)
}
#expect(condition(), sourceLocation: sourceLocation)
}
// MARK: - Talk-over (bidirectional)
@Test func burstDrainWaitsForAudiblePlaybackNotDataConsumption() async throws {
let coordinator = AudioSessionCoordinator(session: StubAudioSession())
let (player, engines) = try makePlayer(coordinator: coordinator)
player.enqueue(try encodeSineFrames())
await waitUntil { player.isPlaying }
let engine = try #require(engines().first)
#expect(engine.requestedCompletionTypes.allSatisfy { $0 == .dataPlayedBack })
player.finishAfterDrain()
engine.fireDataConsumedCallbacks()
await Task.yield()
#expect(!player.stopped)
#expect(player.isPlaying)
engine.fireDataPlayedBackCallbacks()
await waitUntil { player.stopped }
#expect(!player.isPlaying)
}
@Test func olderPTTSessionAcquireCannotStealNewerPlaybackReservation() async throws {
let session = GatedAudioSession()
let coordinator = AudioSessionCoordinator(session: session)
let exclusivity = VoiceNotePlaybackCoordinator()
final class EngineBox { var engines: [MockPlaybackEngine] = [] }
let box = EngineBox()
let player = try #require(PTTBurstPlayer(
coordinator: coordinator,
exclusivity: exclusivity,
makeEngine: {
let engine = MockPlaybackEngine()
box.engines.append(engine)
return engine
}
))
player.enqueue(try encodeSineFrames())
await waitUntil { session.categoryCallCount == 1 }
// A user taps a voice note while the older inbound burst is blocked
// in audio-session activation. Its immediate play intent is newer.
let voiceNote = StubExclusivePlayback()
exclusivity.activate(voiceNote)
session.open()
await waitUntil { player.stopped }
#expect(box.engines.count == 1)
#expect(box.engines[0].startCount == 0)
#expect(voiceNote.pauseCount == 0)
#expect(!player.isPlaying)
}
@Test func categoryEscalationRestartsEngineAndKeepsStreaming() async throws {
let coordinator = AudioSessionCoordinator(session: StubAudioSession())
let (player, engines) = try makePlayer(coordinator: coordinator)
let frames = try encodeSineFrames()
player.enqueue(frames)
await waitUntil { player.isPlaying }
#expect(engines().count == 1)
#expect(engines()[0].startCount == 1)
#expect(!engines()[0].scheduledBuffers.isEmpty)
// Push-to-talk pressed while the burst plays: capture escalates the
// session category. The playback engine must restart under the new
// configuration, not die. (Escalation fan-out is delivered before
// acquire returns, so no waiting is needed here.)
let capture = try await coordinator.acquire(.capture) {}
#expect(engines().count == 2)
#expect(engines()[0].stopCount == 1)
#expect(engines()[1].startCount == 1)
#expect(player.isPlaying)
// Frames arriving after the restart keep playing on the new engine.
player.enqueue(frames)
#expect(!engines()[1].scheduledBuffers.isEmpty)
coordinator.release(capture)
player.stop()
#expect(!player.isPlaying)
}
@Test func categoryEscalationReplaysOnlyUnfinishedTailAfterBurstEnd() async throws {
let coordinator = AudioSessionCoordinator(session: StubAudioSession())
let (player, engines) = try makePlayer(coordinator: coordinator)
let frames = try encodeSineFrames()
player.enqueue(frames)
await waitUntil { player.isPlaying }
let originalEngine = engines()[0]
let originallyScheduled = originalEngine.scheduledBuffers.count
try #require(originallyScheduled > 1)
// One buffer has played, but deliberately do not yield for its
// MainActor completion task. The completion latch itself must keep
// the rebuild from replaying this already-completed prefix.
originalEngine.fireNextDataPlayedBackCallback()
player.finishAfterDrain()
// A real AVAudioPlayerNode also invokes requested callbacks when its
// engine is stopped by a configuration change. Those callbacks must
// leave the unheard tail pending for the fresh engine.
originalEngine.firePlaybackStoppedCallbacks()
// Capture joins after END while the remaining tail is still handed
// to the old engine. The fresh engine must replay that tail instead
// of looking empty and stopping immediately.
let capture = try await coordinator.acquire(.capture) {}
try #require(engines().count == 2)
let restartedEngine = engines()[1]
#expect(restartedEngine.scheduledBuffers.count == originallyScheduled - 1)
#expect(player.isPlaying)
#expect(!player.stopped)
// Only the fresh engine's audible completions may stop this finished
// burst; the stop callbacks above did not drain it.
#expect(player.isPlaying)
#expect(!player.stopped)
restartedEngine.fireDataPlayedBackCallbacks()
await waitUntil { player.stopped }
#expect(!player.isPlaying)
coordinator.release(capture)
}
@Test func realInterruptionStillStopsPlayback() async throws {
let coordinator = AudioSessionCoordinator(session: StubAudioSession())
let (player, engines) = try makePlayer(coordinator: coordinator)
let frames = try encodeSineFrames()
player.enqueue(frames)
await waitUntil { player.isPlaying }
// A system interruption (phone call) is not an escalation: stop.
await coordinator.handleInterruptionBegan()
#expect(!player.isPlaying)
#expect(engines().count == 1)
#expect(engines()[0].stopCount == 1)
// A stopped burst stays stopped.
let before = engines()[0].scheduledBuffers.count
player.enqueue(frames)
#expect(engines()[0].scheduledBuffers.count == before)
}
// MARK: - Burst END racing the async session acquire
/// Models production ownership (`ChatLiveVoiceCoordinator.finalize`): the
/// assembly — the player's sole strong owner — is discarded on burst END,
/// and only the parked draining reference keeps the player alive. The
/// audio must still play out and the session token must come back once
/// the drain finishes.
@Test func burstEndDuringSessionAcquireStillPlaysWithOnlyDrainOwner() async throws {
let session = GatedAudioSession()
let coordinator = AudioSessionCoordinator(session: session)
final class EngineBox { var engines: [MockPlaybackEngine] = [] }
let box = EngineBox()
var owner: PTTBurstPlayer? = PTTBurstPlayer(
coordinator: coordinator,
exclusivity: VoiceNotePlaybackCoordinator(),
makeEngine: {
let engine = MockPlaybackEngine()
box.engines.append(engine)
return engine
}
)
try #require(owner != nil)
let weakPlayer = { [weak owner] in owner }
let frames = try encodeSineFrames()
owner?.enqueue(frames)
// END lands while activation is still blocked on the session queue.
// With nothing scheduled yet, the drain check must not mistake the
// not-yet-started burst for a played-out one and drop all its audio.
var draining: PTTBurstPlayer? = owner
owner?.onStopped = { draining = nil }
owner?.finishAfterDrain()
#expect(owner?.stopped == false)
owner = nil
session.open()
await waitUntil { weakPlayer()?.isPlaying == true }
#expect(box.engines.count == 1)
#expect(box.engines[0].startCount == 1)
#expect(!box.engines[0].scheduledBuffers.isEmpty)
// Play the tail out: the drain must stop the player, hand the token
// back (deactivation reaches the mock), and unpark the drain owner.
box.engines[0].fireDataPlayedBackCallbacks()
await waitUntil { session.activationCalls == [true, false] }
#expect(draining == nil)
#expect(weakPlayer() == nil)
}
/// Backstop: if every owner drops the player before it stopped, `deinit`
/// must hand the registered session token back — the coordinator retains
/// tokens strongly, so a leaked one would keep the session active (and
/// pin any escalated category) for the app's lifetime.
@Test func ownerlessPlayerDeinitReleasesSessionToken() async throws {
let session = GatedAudioSession()
let coordinator = AudioSessionCoordinator(session: session)
var player: PTTBurstPlayer? = PTTBurstPlayer(
coordinator: coordinator,
exclusivity: VoiceNotePlaybackCoordinator(),
makeEngine: { MockPlaybackEngine() }
)
try #require(player != nil)
let frames = try encodeSineFrames()
player?.enqueue(frames)
// Make sure the acquire is in flight (holding the player alive
// through its call frame) before the last external reference drops.
await waitUntil { session.categoryCallCount == 1 }
player?.finishAfterDrain()
player = nil
session.open()
// The acquire task keeps the player alive just long enough to start;
// when it deallocates, deinit must release the freshly stored token.
await waitUntil { session.activationCalls == [true, false] }
}
// MARK: - Session acquire failure
@Test func sessionAcquireFailureDoesNotStartUnregisteredPlayback() async throws {
let session = StubAudioSession()
let coordinator = AudioSessionCoordinator(session: session)
let (player, engines) = try makePlayer(coordinator: coordinator)
// Playing without a registered holder would leave the engine exposed
// to another holder's last-release deactivating the session under it.
session.setCategoryError = StubSessionError()
let frames = try encodeSineFrames()
player.enqueue(frames)
await waitUntil { player.stopped }
#expect(engines().count == 1)
#expect(engines()[0].startCount == 0)
#expect(!player.isPlaying)
// The failed start latched the player off; later frames are ignored.
player.enqueue(frames)
#expect(engines()[0].scheduledBuffers.isEmpty)
#expect(!player.isPlaying)
}
// MARK: - Engine start failure
@Test func engineStartFailureRebuildsOnceBeforeGivingUp() async throws {
let coordinator = AudioSessionCoordinator(session: StubAudioSession())
let (player, engines) = try makePlayer(coordinator: coordinator)
// A capture racing the start can reconfigure the session while the
// engine spins up (its escalation fan-out no-ops on a never-started
// player): the player must rebuild once against the settled
// configuration instead of latching off.
engines()[0].startError = StubSessionError()
let frames = try encodeSineFrames()
player.enqueue(frames)
await waitUntil { player.isPlaying }
#expect(engines().count == 2)
#expect(engines()[0].startCount == 0)
#expect(engines()[1].startCount == 1)
#expect(!engines()[1].scheduledBuffers.isEmpty)
player.stop()
}
}