From 080c343fd4c1113f6a86938b46a23b53131f13f8 Mon Sep 17 00:00:00 2001 From: Dmitry Ilyin <6576495+widgetii@users.noreply.github.com> Date: Tue, 20 Oct 2026 08:33:36 +0300 Subject: [PATCH 1/3] feat: play one phrase at a time, repeatedly, with room to write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit M11, third of four. The transport half of listening practice. A phrase stops at its own end, set through `forwardPlaybackEndTime` rather than watched for. The time observer reports four times a second, so stopping by hand overshoots by up to 250 ms — audible, and enough to leak the first syllable of the next phrase to a learner who is about to be asked to transcribe it. The player stops itself exactly and reports it as the end of the item, which is the path a finished track already took. The drill plays a phrase a given number of times with a gap after each. The gap is the point rather than a detail: it is where the learner says the phrase back or writes it down, and it is what separates a drill from a loop. Advancing to the next phrase is deliberately not done here — which phrase comes next depends on the text and on what the reader has already passed, and neither belongs in a player. Writing the tests found a real one. `pause()` guarded on `isPlaying`, and during the gap between repeats nothing is playing, so the guard returned before cancelling the pending repeat: press pause, hear silence, hear the phrase start again. Cancelling now happens before the guard, because pausing has to mean stop whether or not sound is coming out at that instant. **A correction.** `audioTimePitchAlgorithm` was written up in the plan as a latent M7 defect — that a rate change would deepen the voice. Measured here rather than assumed: `.timeDomain` is already the default on macOS 26, so M7 never shipped a chipmunk voice and there was nothing to repair. It is set explicitly anyway, and the comment now says why that is pinning a behaviour rather than fixing one: the documented default has been `.spectral` in the past, and this app also runs on macOS 14 and iOS 17. One test was flaky as first written — a 500 ms sleep for a 300 ms gap, which passes here and fails under parallel execution or on a slower machine. It polls to a generous deadline now: the claim is that the repeat happens, not that it happens within half a second. Five mutants planted and killed. Co-Authored-By: Claude Opus 5 --- Sources/HanReaderPlayback/AudioEngine.swift | 33 +++ .../PlaybackModel+Phrase.swift | 74 ++++++ Sources/HanReaderPlayback/PlaybackModel.swift | 73 +++++- .../PhrasePlaybackTests.swift | 226 ++++++++++++++++++ .../PlaybackModelTests.swift | 76 ------ .../PlaybackSpies.swift | 91 +++++++ 6 files changed, 490 insertions(+), 83 deletions(-) create mode 100644 Sources/HanReaderPlayback/PlaybackModel+Phrase.swift create mode 100644 Tests/HanReaderPlaybackTests/PhrasePlaybackTests.swift create mode 100644 Tests/HanReaderPlaybackTests/PlaybackSpies.swift diff --git a/Sources/HanReaderPlayback/AudioEngine.swift b/Sources/HanReaderPlayback/AudioEngine.swift index 20201c2..f0e6cf9 100644 --- a/Sources/HanReaderPlayback/AudioEngine.swift +++ b/Sources/HanReaderPlayback/AudioEngine.swift @@ -29,6 +29,12 @@ public protocol AudioEngine: AnyObject { /// when not. func setRate(_ rate: Double) + /// Stop here rather than at the end of the track, or nil to play on. + /// + /// Needed because listening practice plays one phrase, and a phrase that + /// runs a quarter-second into the next one has given the answer away. + func setStopTime(_ time: Double?) + /// Called about four times a second while playing. What reaches the UI is /// throttled further — see `PlaybackModel.report(_:)`. var onTime: ((Double) -> Void)? { get set } @@ -119,6 +125,16 @@ final class AVPlayerAudioEngine: AudioEngine { let asset = AVURLAsset(url: url) let item = AVPlayerItem(asset: asset) + // Pinned, not fixed. Measured on macOS 26 this is already the + // default, so M7 was never shipping a chipmunk voice at 0.75 — a + // claim made in the plan and checked only now. What it does buy is + // certainty: the documented default has been `.spectral` in the + // past, this app also runs on macOS 14 and iOS 17, and a recording + // slowed down for a learner must be a slower voice rather than a + // deeper one on every one of them. `.timeDomain` is Apple's + // recommendation for speech and is what WorkAudioBook bundles + // SoundTouch to do. + item.audioTimePitchAlgorithm = .timeDomain player.replaceCurrentItem(with: item) finishObserver = NotificationCenter.default.addObserver( @@ -173,4 +189,21 @@ final class AVPlayerAudioEngine: AudioEngine { player.rate = Float(rate) } } + + /// `forwardPlaybackEndTime` rather than watching the clock. + /// + /// The time observer reports four times a second, so stopping by hand + /// overshoots by up to 250 ms — audible, and enough to leak the first + /// syllable of the next phrase to a learner who is about to be asked to + /// transcribe it. The player stops itself at exactly this point and + /// reports it as the end of the item, which is the same path a finished + /// track already takes. + func setStopTime(_ time: Double?) { + guard let item = player.currentItem else { return } + guard let time else { + item.forwardPlaybackEndTime = .invalid + return + } + item.forwardPlaybackEndTime = CMTime(seconds: max(0, time), preferredTimescale: 600) + } } diff --git a/Sources/HanReaderPlayback/PlaybackModel+Phrase.swift b/Sources/HanReaderPlayback/PlaybackModel+Phrase.swift new file mode 100644 index 0000000..c13059b --- /dev/null +++ b/Sources/HanReaderPlayback/PlaybackModel+Phrase.swift @@ -0,0 +1,74 @@ +// HanReader — MIT licensed. See LICENSE. + +import Foundation + +/// Playing one phrase at a time, which is what listening practice is. +/// +/// Separate from the transport's own body because it is a different mode of +/// use: the rest of `PlaybackModel` plays a recording, and this plays a +/// span of one, repeatedly, with silence in between for the learner to +/// write in. +extension PlaybackModel { + // MARK: - One phrase at a time + + /// Plays a single phrase, stopping exactly at its end. + public func play(segment range: Range, drill: Drill? = nil) { + guard state == .ready else { return } + cancelDrill() + if let drill { + self.drill = drill + } + segment = range + repeatsPlayed = 0 + playSegmentOnce() + } + + /// Goes back to playing the whole recording. + public func clearSegment() { + cancelDrill() + segment = nil + engine.setStopTime(nil) + onStateChange?() + } + + func playSegmentOnce() { + guard let segment, state == .ready else { return } + activateSession() + engine.setStopTime(segment.upperBound) + engine.seek(to: segment.lowerBound) + playhead = segment.lowerBound + observedTime = segment.lowerBound + engine.play() + isPlaying = true + clearPauseReasons() + onStateChange?() + } + + /// One pass through the phrase ended, at exactly its end time. + func finishSegmentPass() { + engine.pause() + isPlaying = false + clearPauseReasons() + repeatsPlayed += 1 + guard repeatsPlayed < drill.repeats else { + onStateChange?() + onSegmentFinished?() + return + } + onStateChange?() + drillTask = Task { [weak self] in + let gap = self?.drill.gap ?? 0 + if gap > 0 { + try? await Task.sleep(for: .seconds(gap)) + } + guard !Task.isCancelled else { return } + self?.playSegmentOnce() + } + } + + func cancelDrill() { + drillTask?.cancel() + drillTask = nil + repeatsPlayed = 0 + } +} diff --git a/Sources/HanReaderPlayback/PlaybackModel.swift b/Sources/HanReaderPlayback/PlaybackModel.swift index 6a3ee70..292efcf 100644 --- a/Sources/HanReaderPlayback/PlaybackModel.swift +++ b/Sources/HanReaderPlayback/PlaybackModel.swift @@ -44,7 +44,7 @@ public final class PlaybackModel { } public private(set) var state: State = .empty - public private(set) var isPlaying = false + public internal(set) var isPlaying = false public private(set) var duration = 0.0 /// The playhead, published **at most once a second**. @@ -53,7 +53,7 @@ public final class PlaybackModel { /// prototype published every tick straight to the main actor, so the /// thread laying out CJK text was woken ten times a second to render a /// string that had not changed. - public private(set) var observedTime = 0.0 + public internal(set) var observedTime = 0.0 /// Where the reader's finger is, while it is down. /// @@ -72,6 +72,46 @@ public final class PlaybackModel { scrubTime != nil } + /// The span being practised, or nil when the whole recording is playing. + /// + /// The setter is internal rather than private because phrase playback + /// lives in its own file, and Swift's `private` reaches extensions in + /// the same file only. Still closed to callers outside this module, + /// which is the boundary that matters. + public internal(set) var segment: Range? + + /// How a phrase is drilled: how many times through, and how long a gap + /// to leave after each. + /// + /// The gap is the whole point of a drill and the reason this is not just + /// a loop: it is where the learner says the phrase back, or writes it. + /// WorkAudioBook spells the same two settings out as "repeat phrase" and + /// "pause between phrases". + public struct Drill: Hashable, Sendable { + public var repeats: Int + public var gap: Double + + public init(repeats: Int = 1, gap: Double = 0) { + self.repeats = max(1, repeats) + self.gap = max(0, gap) + } + + /// Once through, no gap: pressing play on a phrase. + public static let once = Self() + } + + public var drill = Drill.once + + /// Called when a phrase has been played as many times as the drill asks. + /// + /// The transport deliberately does not advance by itself. Which phrase + /// comes next is a question about the text and about what the reader has + /// already passed, and neither belongs down here. + public var onSegmentFinished: (() -> Void)? + + @ObservationIgnored var repeatsPlayed = 0 + @ObservationIgnored var drillTask: Task? + public var rate: PlaybackRate = .normal { didSet { guard rate != oldValue else { return } @@ -88,7 +128,7 @@ public final class PlaybackModel { /// The exact playhead, for persistence. Not the published one, which is /// rounded to the second for the display's benefit; a reader who stops /// mid-sentence should resume mid-sentence. - @ObservationIgnored public private(set) var playhead = 0.0 + @ObservationIgnored public internal(set) var playhead = 0.0 /// True when playback was stopped by something other than the reader — /// a phone call, headphones unplugged, a word being spoken — so it can be @@ -104,7 +144,7 @@ public final class PlaybackModel { /// Paused by the system — a call, an alarm, another app. @ObservationIgnored private var pausedByInterruption = false - @ObservationIgnored private let engine: any AudioEngine + @ObservationIgnored let engine: any AudioEngine @ObservationIgnored private let session: any PlaybackSession @ObservationIgnored private var hasActivatedSession = false @@ -173,6 +213,9 @@ public final class PlaybackModel { playhead = 0 scrubTime = nil clearPauseReasons() + cancelDrill() + segment = nil + engine.setStopTime(nil) deactivateSession() onStateChange?() } @@ -203,7 +246,16 @@ public final class PlaybackModel { } public func pause() { - guard isPlaying else { return } + // Before the guard, not after it. During the gap between repeats + // nothing is playing, so a guard on `isPlaying` returns without + // stopping the pending repeat — and the reader presses pause, hears + // silence, and then hears the phrase start again. Pausing has to + // mean "stop", whether or not sound is coming out at this instant. + cancelDrill() + guard isPlaying else { + onStateChange?() + return + } engine.pause() isPlaying = false // Deliberate, so neither the system nor a finished utterance may @@ -333,6 +385,13 @@ public final class PlaybackModel { } private func finish() { + // A phrase reaching its own end comes through the same path as a + // track reaching the end of the file, because `forwardPlaybackEndTime` + // *is* the end of the item as far as the player is concerned. + if segment != nil { + finishSegmentPass() + return + } engine.pause() isPlaying = false clearPauseReasons() @@ -346,7 +405,7 @@ public final class PlaybackModel { // MARK: - Session - private func activateSession() { + func activateSession() { guard !hasActivatedSession else { return } do { try session.activate() @@ -362,7 +421,7 @@ public final class PlaybackModel { hasActivatedSession = false } - private func clearPauseReasons() { + func clearPauseReasons() { pausedForSpeech = false pausedByInterruption = false } diff --git a/Tests/HanReaderPlaybackTests/PhrasePlaybackTests.swift b/Tests/HanReaderPlaybackTests/PhrasePlaybackTests.swift new file mode 100644 index 0000000..b330248 --- /dev/null +++ b/Tests/HanReaderPlaybackTests/PhrasePlaybackTests.swift @@ -0,0 +1,226 @@ +// HanReader — MIT licensed. See LICENSE. + +import Foundation +import Testing +@testable import HanReaderPlayback + +/// Playing one phrase at a time, which is what listening practice is. +/// +/// The spy engine is what makes this testable at all: whether a phrase stops +/// at its own end, how many times it is played, and whether a pause during +/// the silence between repeats really stops it, are all statements about +/// calls rather than about sound. +@MainActor +@Suite("Phrase playback") +struct PhrasePlaybackTests { + /// Waits for something to become true, rather than sleeping for a + /// guessed interval and hoping. + /// + /// A fixed sleep just over the gap passes here and fails on a slower + /// machine, which is a flaky test dressed as a timing assertion. The + /// claim being made is "this happens", not "this happens within 500 + /// milliseconds", so the deadline is generous and the polling is tight. + private func eventually( + _ condition: () -> Bool, + within seconds: Double = 5, + ) async + -> Bool + { + let deadline = Date().addingTimeInterval(seconds) + while Date() < deadline { + if condition() { + return true + } + try? await Task.sleep(for: .milliseconds(10)) + } + return condition() + } + + private func fixture(duration: Double = 100) -> (SpyEngine, PlaybackModel) { + let engine = SpyEngine() + let model = PlaybackModel(engine: engine, session: SpySession()) + model.load(URL(fileURLWithPath: "/tmp/track.m4a")) + engine.declareDuration(duration) + return (engine, model) + } + + // MARK: - Stopping where the phrase stops + + /// A phrase that runs a quarter-second into the next one has given the + /// answer away, so the stop is the player's own rather than something + /// watched for. The time observer reports four times a second, which + /// would overshoot by up to 250 ms. + @Test("Playing a phrase sets an exact stop time") + func phraseStopsAtItsOwnEnd() { + let (engine, model) = fixture() + model.play(segment: 12 ..< 17) + + #expect(engine.stopTimes.last == 17) + #expect(engine.seeks.last == 12) + #expect(model.isPlaying) + #expect(model.segment == 12 ..< 17) + // And the position is published at once, so the transport does not + // show the previous phrase for a quarter of a second. + #expect(model.displayTime == 12) + } + + @Test("Leaving phrase playback restores the whole recording") + func clearingTheSegmentRemovesTheStopTime() { + let (engine, model) = fixture() + model.play(segment: 12 ..< 17) + model.clearSegment() + + #expect(model.segment == nil) + #expect(engine.stopTimes.last == .some(nil)) + } + + @Test("Unloading forgets the phrase as well as the track") + func unloadingClearsTheSegment() { + let (engine, model) = fixture() + model.play(segment: 12 ..< 17) + model.unload() + + #expect(model.segment == nil) + #expect(!model.isPlaying) + #expect(engine.stopTimes.last == .some(nil)) + } + + // MARK: - The drill + + /// Once through is pressing play on a phrase, and nothing more should + /// happen when it ends. + @Test("A phrase played once stops and says so") + func playedOnce() { + let (engine, model) = fixture() + var finished = 0 + model.onSegmentFinished = { finished += 1 } + + model.play(segment: 12 ..< 17) + #expect(engine.playCount == 1) + engine.reachEnd() + + #expect(!model.isPlaying) + #expect(finished == 1) + #expect(engine.playCount == 1) + } + + /// Three times through, with no gap, is three plays and one report at + /// the end — not three reports. + @Test("A phrase drilled three times plays three times and reports once") + func repeatedThreeTimes() async { + let (engine, model) = fixture() + var finished = 0 + model.onSegmentFinished = { finished += 1 } + + model.play(segment: 12 ..< 17, drill: .init(repeats: 3, gap: 0)) + #expect(engine.playCount == 1) + + engine.reachEnd() + await Task.yield() + #expect(engine.playCount == 2) + #expect(finished == 0) + + engine.reachEnd() + await Task.yield() + #expect(engine.playCount == 3) + #expect(finished == 0) + + engine.reachEnd() + #expect(engine.playCount == 3) + #expect(finished == 1) + #expect(!model.isPlaying) + } + + /// Each repeat starts at the beginning of the phrase, not wherever the + /// previous pass left the playhead. + @Test("Every repeat seeks back to the start of the phrase") + func repeatsSeekBack() async { + let (engine, model) = fixture() + model.play(segment: 12 ..< 17, drill: .init(repeats: 2, gap: 0)) + engine.reachEnd() + await Task.yield() + + #expect(engine.seeks.suffix(2) == [12, 12]) + } + + /// The gap is the point of a drill rather than a detail of it: it is + /// where the learner says the phrase back, or writes it down. + @Test("A gap between repeats delays the next play") + func gapDelaysTheNextPass() async { + let (engine, model) = fixture() + model.play(segment: 12 ..< 17, drill: .init(repeats: 2, gap: 0.3)) + engine.reachEnd() + + // Immediately after the phrase ends, nothing is playing. + #expect(!model.isPlaying) + #expect(engine.playCount == 1) + + #expect(await eventually { engine.playCount == 2 }) + #expect(model.isPlaying) + } + + /// A reader pressing pause during the silence must not find the phrase + /// starting again a second later. The transport has to stay stopped. + @Test("Pausing during the gap stops the drill") + func pausingDuringTheGapStopsIt() async throws { + let (engine, model) = fixture() + model.play(segment: 12 ..< 17, drill: .init(repeats: 3, gap: 0.2)) + engine.reachEnd() + model.pause() + + // A negative claim, so this one does have to wait — comfortably past + // the gap, to be sure the repeat did not merely arrive late. + try await Task.sleep(for: .milliseconds(800)) + #expect(engine.playCount == 1) + #expect(!model.isPlaying) + } + + /// And choosing another phrase mid-drill abandons the old one rather + /// than letting its remaining repeats play over the new one. + @Test("Starting another phrase abandons the drill in progress") + func anotherPhraseCancelsTheDrill() async throws { + let (engine, model) = fixture() + model.play(segment: 12 ..< 17, drill: .init(repeats: 3, gap: 0.2)) + engine.reachEnd() + + model.play(segment: 30 ..< 35) + try await Task.sleep(for: .milliseconds(800)) + + #expect(model.segment == 30 ..< 35) + #expect(engine.seeks.last == 30) + // Two plays: the first phrase once, the second once. The abandoned + // repeat never happened. + #expect(engine.playCount == 2) + } + + @Test("A drill asking for nonsense is clamped rather than trusted") + func drillSettingsAreClamped() { + #expect(PlaybackModel.Drill(repeats: 0, gap: -5).repeats == 1) + #expect(PlaybackModel.Drill(repeats: 0, gap: -5).gap == 0) + } + + // MARK: - Interaction with the rest of the transport + + /// The whole-track finish path must not fire for a phrase, or the first + /// phrase of a recording would look like the end of it. + @Test("A phrase ending is not the recording ending") + func phraseEndIsNotTrackEnd() { + let (engine, model) = fixture(duration: 100) + model.play(segment: 12 ..< 17) + engine.reachEnd() + + // Not parked at the end of the file. + #expect(model.playhead < 20) + #expect(model.segment != nil) + } + + @Test("Speaking a word still ducks a phrase, and resumes it") + func speechStillDucks() { + let (_, model) = fixture() + model.play(segment: 12 ..< 17) + model.pauseForSpeech() + #expect(!model.isPlaying) + model.resumeAfterSpeech() + #expect(model.isPlaying) + } +} diff --git a/Tests/HanReaderPlaybackTests/PlaybackModelTests.swift b/Tests/HanReaderPlaybackTests/PlaybackModelTests.swift index 6e22283..f7b984d 100644 --- a/Tests/HanReaderPlaybackTests/PlaybackModelTests.swift +++ b/Tests/HanReaderPlaybackTests/PlaybackModelTests.swift @@ -4,82 +4,6 @@ import Foundation import Testing @testable import HanReaderPlayback -/// A player that makes no sound and counts what it was asked to do. -/// -/// The transport's hardest requirements are about calls rather than about -/// audio — how many seeks a drag issues, how often state is pushed to a lock -/// screen — and those are assertions only against a spy. -@MainActor -private final class SpyEngine: AudioEngine { - private(set) var seeks: [Double] = [] - private(set) var playCount = 0 - private(set) var pauseCount = 0 - private(set) var rates: [Double] = [] - private(set) var loaded: [URL] = [] - - var duration = 0.0 - var currentTime = 0.0 - - var onTime: ((Double) -> Void)? - var onDuration: ((Double) -> Void)? - var onFinish: (() -> Void)? - - func load(_ url: URL) { - loaded.append(url) - } - - func play() { - playCount += 1 - } - - func pause() { - pauseCount += 1 - } - - func seek(to time: Double) { - seeks.append(time) - currentTime = time - } - - func setRate(_ rate: Double) { - rates.append(rate) - } - - /// Delivers a position the way the real engine does, four times a second. - func tick(_ time: Double) { - currentTime = time - onTime?(time) - } - - func declareDuration(_ seconds: Double) { - duration = seconds - onDuration?(seconds) - } -} - -@MainActor -private final class SpySession: PlaybackSession { - private(set) var activations = 0 - private(set) var deactivations = 0 - var failure: (any Error)? - var onInterruption: ((PlaybackInterruption) -> Void)? - - func activate() throws { - if let failure { - throw failure - } - activations += 1 - } - - func deactivate() { - deactivations += 1 - } - - func interrupt(_ interruption: PlaybackInterruption) { - onInterruption?(interruption) - } -} - private struct Fixture { let engine: SpyEngine let session: SpySession diff --git a/Tests/HanReaderPlaybackTests/PlaybackSpies.swift b/Tests/HanReaderPlaybackTests/PlaybackSpies.swift new file mode 100644 index 0000000..5da8262 --- /dev/null +++ b/Tests/HanReaderPlaybackTests/PlaybackSpies.swift @@ -0,0 +1,91 @@ +// HanReader — MIT licensed. See LICENSE. + +import Foundation +@testable import HanReaderPlayback + +/// A player that makes no sound and counts what it was asked to do. +/// +/// The transport's hardest requirements are about calls rather than about +/// audio — how many seeks a drag issues, how often state is pushed to a lock +/// screen — and those are assertions only against a spy. +@MainActor +final class SpyEngine: AudioEngine { + private(set) var seeks: [Double] = [] + private(set) var playCount = 0 + private(set) var pauseCount = 0 + private(set) var rates: [Double] = [] + private(set) var loaded: [URL] = [] + private(set) var stopTimes: [Double?] = [] + + var duration = 0.0 + var currentTime = 0.0 + + var onTime: ((Double) -> Void)? + var onDuration: ((Double) -> Void)? + var onFinish: (() -> Void)? + + func load(_ url: URL) { + loaded.append(url) + } + + func play() { + playCount += 1 + } + + func pause() { + pauseCount += 1 + } + + func seek(to time: Double) { + seeks.append(time) + currentTime = time + } + + func setRate(_ rate: Double) { + rates.append(rate) + } + + func setStopTime(_ time: Double?) { + stopTimes.append(time) + } + + /// The player reaching the stop time, which it reports as the end of the + /// item — the same path a finished track takes. + func reachEnd() { + onFinish?() + } + + /// Delivers a position the way the real engine does, four times a second. + func tick(_ time: Double) { + currentTime = time + onTime?(time) + } + + func declareDuration(_ seconds: Double) { + duration = seconds + onDuration?(seconds) + } +} + +@MainActor +final class SpySession: PlaybackSession { + private(set) var activations = 0 + private(set) var deactivations = 0 + var failure: (any Error)? + var onInterruption: ((PlaybackInterruption) -> Void)? + + func activate() throws { + if let failure { + throw failure + } + activations += 1 + } + + func deactivate() { + deactivations += 1 + } + + func interrupt(_ interruption: PlaybackInterruption) { + onInterruption?(interruption) + } +} From 946483fcdbbbd0f4d7fb30c1b0797d7881528a48 Mon Sep 17 00:00:00 2001 From: Dmitry Ilyin <6576495+widgetii@users.noreply.github.com> Date: Tue, 20 Oct 2026 08:41:26 +0300 Subject: [PATCH 2/3] fix: take the clock out of the drill tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI failed the gap test again, this time despite polling to a five-second deadline — so it was never slack to be absorbed. A repeat is scheduled on a task that suspends twice before it plays, and "wait a bit and look" is a guess whether the waiting is a sleep or a poll. The gap is now injected. `PlaybackModel` takes a `wait`, defaulting to `Task.sleep`, so a test can let it return at once and assert what was asked for and what followed it rather than how long it took. And a repeat currently waiting one out can be awaited directly instead of yielded at. The suite went from 0.85 seconds with two fixed sleeps in it to 0.001 seconds with no sleeping at all, and gained a test that nothing waits when no gap was asked for. Co-Authored-By: Claude Opus 5 --- .../PlaybackModel+Phrase.swift | 14 +- Sources/HanReaderPlayback/PlaybackModel.swift | 16 +++ .../PhrasePlaybackTests.swift | 128 ++++++++++++------ 3 files changed, 111 insertions(+), 47 deletions(-) diff --git a/Sources/HanReaderPlayback/PlaybackModel+Phrase.swift b/Sources/HanReaderPlayback/PlaybackModel+Phrase.swift index c13059b..0cefa9a 100644 --- a/Sources/HanReaderPlayback/PlaybackModel+Phrase.swift +++ b/Sources/HanReaderPlayback/PlaybackModel+Phrase.swift @@ -57,15 +57,25 @@ extension PlaybackModel { } onStateChange?() drillTask = Task { [weak self] in - let gap = self?.drill.gap ?? 0 + guard let gap = self?.drill.gap, let wait = self?.wait else { return } if gap > 0 { - try? await Task.sleep(for: .seconds(gap)) + await wait(gap) } guard !Task.isCancelled else { return } self?.playSegmentOnce() } } + /// Awaits a repeat that is currently waiting out its gap. + /// + /// A test seam, and the reason there is no sleeping left in these + /// tests: a repeat is scheduled on a task that suspends twice before it + /// plays, so "yield a couple of times and hope" is as much a guess as + /// sleeping was. Awaiting the task itself is exact. + func awaitPendingRepeat() async { + await drillTask?.value + } + func cancelDrill() { drillTask?.cancel() drillTask = nil diff --git a/Sources/HanReaderPlayback/PlaybackModel.swift b/Sources/HanReaderPlayback/PlaybackModel.swift index 292efcf..391de50 100644 --- a/Sources/HanReaderPlayback/PlaybackModel.swift +++ b/Sources/HanReaderPlayback/PlaybackModel.swift @@ -148,12 +148,28 @@ public final class PlaybackModel { @ObservationIgnored private let session: any PlaybackSession @ObservationIgnored private var hasActivatedSession = false + /// Waits out the gap between repeats. + /// + /// Injectable because the alternative is a test that sleeps and hopes. + /// A fixed sleep just over the gap passes on a fast machine and fails + /// under parallel execution or on a slower runner — which is exactly + /// what happened, locally green and red in CI. With the wait supplied, + /// a test can let it return at once and assert the *order* of what + /// happens rather than the clock it happens on. + public typealias Wait = @Sendable (Double) async -> Void + + @ObservationIgnored let wait: Wait + public init( engine: (any AudioEngine)? = nil, session: (any PlaybackSession)? = nil, + wait: @escaping Wait = { seconds in + try? await Task.sleep(for: .seconds(seconds)) + }, ) { self.engine = engine ?? AVPlayerAudioEngine() self.session = session ?? SystemPlaybackSession() + self.wait = wait configure() } diff --git a/Tests/HanReaderPlaybackTests/PhrasePlaybackTests.swift b/Tests/HanReaderPlaybackTests/PhrasePlaybackTests.swift index b330248..4c50481 100644 --- a/Tests/HanReaderPlaybackTests/PhrasePlaybackTests.swift +++ b/Tests/HanReaderPlaybackTests/PhrasePlaybackTests.swift @@ -13,35 +13,45 @@ import Testing @MainActor @Suite("Phrase playback") struct PhrasePlaybackTests { - /// Waits for something to become true, rather than sleeping for a - /// guessed interval and hoping. + /// Records what the drill asked to wait for, and returns at once. /// - /// A fixed sleep just over the gap passes here and fails on a slower - /// machine, which is a flaky test dressed as a timing assertion. The - /// claim being made is "this happens", not "this happens within 500 - /// milliseconds", so the deadline is generous and the polling is tight. - private func eventually( - _ condition: () -> Bool, - within seconds: Double = 5, - ) async - -> Bool - { - let deadline = Date().addingTimeInterval(seconds) - while Date() < deadline { - if condition() { - return true - } - try? await Task.sleep(for: .milliseconds(10)) + /// The gap is the one thing in this transport that is about real time, + /// and a test that actually waits it out is a test that passes here and + /// fails on a loaded CI runner — which is what happened twice, first + /// with a fixed sleep and then with polling to a five-second deadline. + /// With the wait injected there is nothing to race: the test asserts + /// that the gap was requested and that the repeat followed it. + final nonisolated class Waits: @unchecked Sendable { + private let lock = NSLock() + private var requested: [Double] = [] + + func record(_ seconds: Double) { + lock.withLock { requested.append(seconds) } } - return condition() + + var all: [Double] { + lock.withLock { requested } + } + } + + /// A loaded transport with its engine and its record of waits. + private struct Bench { + let engine: SpyEngine + let model: PlaybackModel + let waits: Waits } - private func fixture(duration: Double = 100) -> (SpyEngine, PlaybackModel) { + private func bench(duration: Double = 100) -> Bench { let engine = SpyEngine() - let model = PlaybackModel(engine: engine, session: SpySession()) + let waits = Waits() + let model = PlaybackModel( + engine: engine, + session: SpySession(), + wait: { seconds in waits.record(seconds) }, + ) model.load(URL(fileURLWithPath: "/tmp/track.m4a")) engine.declareDuration(duration) - return (engine, model) + return Bench(engine: engine, model: model, waits: waits) } // MARK: - Stopping where the phrase stops @@ -52,7 +62,8 @@ struct PhrasePlaybackTests { /// would overshoot by up to 250 ms. @Test("Playing a phrase sets an exact stop time") func phraseStopsAtItsOwnEnd() { - let (engine, model) = fixture() + let rig = bench() + let (engine, model) = (rig.engine, rig.model) model.play(segment: 12 ..< 17) #expect(engine.stopTimes.last == 17) @@ -66,7 +77,8 @@ struct PhrasePlaybackTests { @Test("Leaving phrase playback restores the whole recording") func clearingTheSegmentRemovesTheStopTime() { - let (engine, model) = fixture() + let rig = bench() + let (engine, model) = (rig.engine, rig.model) model.play(segment: 12 ..< 17) model.clearSegment() @@ -76,7 +88,8 @@ struct PhrasePlaybackTests { @Test("Unloading forgets the phrase as well as the track") func unloadingClearsTheSegment() { - let (engine, model) = fixture() + let rig = bench() + let (engine, model) = (rig.engine, rig.model) model.play(segment: 12 ..< 17) model.unload() @@ -91,7 +104,8 @@ struct PhrasePlaybackTests { /// happen when it ends. @Test("A phrase played once stops and says so") func playedOnce() { - let (engine, model) = fixture() + let rig = bench() + let (engine, model) = (rig.engine, rig.model) var finished = 0 model.onSegmentFinished = { finished += 1 } @@ -108,7 +122,8 @@ struct PhrasePlaybackTests { /// the end — not three reports. @Test("A phrase drilled three times plays three times and reports once") func repeatedThreeTimes() async { - let (engine, model) = fixture() + let rig = bench() + let (engine, model) = (rig.engine, rig.model) var finished = 0 model.onSegmentFinished = { finished += 1 } @@ -116,12 +131,12 @@ struct PhrasePlaybackTests { #expect(engine.playCount == 1) engine.reachEnd() - await Task.yield() + await model.awaitPendingRepeat() #expect(engine.playCount == 2) #expect(finished == 0) engine.reachEnd() - await Task.yield() + await model.awaitPendingRepeat() #expect(engine.playCount == 3) #expect(finished == 0) @@ -135,42 +150,63 @@ struct PhrasePlaybackTests { /// previous pass left the playhead. @Test("Every repeat seeks back to the start of the phrase") func repeatsSeekBack() async { - let (engine, model) = fixture() + let rig = bench() + let (engine, model) = (rig.engine, rig.model) model.play(segment: 12 ..< 17, drill: .init(repeats: 2, gap: 0)) engine.reachEnd() - await Task.yield() + await model.awaitPendingRepeat() #expect(engine.seeks.suffix(2) == [12, 12]) } /// The gap is the point of a drill rather than a detail of it: it is /// where the learner says the phrase back, or writes it down. - @Test("A gap between repeats delays the next play") + @Test("A gap between repeats is waited out before the next play") func gapDelaysTheNextPass() async { - let (engine, model) = fixture() + let rig = bench() + let (engine, model) = (rig.engine, rig.model) + let waits = rig.waits model.play(segment: 12 ..< 17, drill: .init(repeats: 2, gap: 0.3)) engine.reachEnd() - // Immediately after the phrase ends, nothing is playing. + // The phrase has ended and nothing is playing yet. #expect(!model.isPlaying) #expect(engine.playCount == 1) - #expect(await eventually { engine.playCount == 2 }) + await model.awaitPendingRepeat() + + // The gap was asked for, and the repeat came after it. + #expect(waits.all == [0.3]) + #expect(engine.playCount == 2) #expect(model.isPlaying) } + @Test("No gap is waited for when none was asked for") + func noGapIsNotWaitedFor() async { + let rig = bench() + let (engine, model) = (rig.engine, rig.model) + let waits = rig.waits + model.play(segment: 12 ..< 17, drill: .init(repeats: 2, gap: 0)) + engine.reachEnd() + await model.awaitPendingRepeat() + + #expect(waits.all.isEmpty) + #expect(engine.playCount == 2) + } + /// A reader pressing pause during the silence must not find the phrase /// starting again a second later. The transport has to stay stopped. @Test("Pausing during the gap stops the drill") - func pausingDuringTheGapStopsIt() async throws { - let (engine, model) = fixture() + func pausingDuringTheGapStopsIt() async { + let rig = bench() + let (engine, model) = (rig.engine, rig.model) model.play(segment: 12 ..< 17, drill: .init(repeats: 3, gap: 0.2)) engine.reachEnd() model.pause() - // A negative claim, so this one does have to wait — comfortably past - // the gap, to be sure the repeat did not merely arrive late. - try await Task.sleep(for: .milliseconds(800)) + // Nothing to wait for: `pause` cancelled the task, and awaiting a + // cancelled one returns at once rather than after its gap. + await model.awaitPendingRepeat() #expect(engine.playCount == 1) #expect(!model.isPlaying) } @@ -178,13 +214,14 @@ struct PhrasePlaybackTests { /// And choosing another phrase mid-drill abandons the old one rather /// than letting its remaining repeats play over the new one. @Test("Starting another phrase abandons the drill in progress") - func anotherPhraseCancelsTheDrill() async throws { - let (engine, model) = fixture() + func anotherPhraseCancelsTheDrill() async { + let rig = bench() + let (engine, model) = (rig.engine, rig.model) model.play(segment: 12 ..< 17, drill: .init(repeats: 3, gap: 0.2)) engine.reachEnd() model.play(segment: 30 ..< 35) - try await Task.sleep(for: .milliseconds(800)) + await model.awaitPendingRepeat() #expect(model.segment == 30 ..< 35) #expect(engine.seeks.last == 30) @@ -205,7 +242,8 @@ struct PhrasePlaybackTests { /// phrase of a recording would look like the end of it. @Test("A phrase ending is not the recording ending") func phraseEndIsNotTrackEnd() { - let (engine, model) = fixture(duration: 100) + let rig = bench(duration: 100) + let (engine, model) = (rig.engine, rig.model) model.play(segment: 12 ..< 17) engine.reachEnd() @@ -216,7 +254,7 @@ struct PhrasePlaybackTests { @Test("Speaking a word still ducks a phrase, and resumes it") func speechStillDucks() { - let (_, model) = fixture() + let model = bench().model model.play(segment: 12 ..< 17) model.pauseForSpeech() #expect(!model.isPlaying) From 95bdd906dc8a606fb6825c3c6dd5ad338b29b4ba Mon Sep 17 00:00:00 2001 From: Dmitry Ilyin <6576495+widgetii@users.noreply.github.com> Date: Tue, 20 Oct 2026 08:48:05 +0300 Subject: [PATCH 3/3] fix: stop a pending repeat outliving the reader's next move MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seven review findings, all the same root cause: `cancelDrill()` was treated as something called from three places rather than as an invariant. A repeat waiting out its gap is a command still pending, and the reader can change their mind in between — so every transport event that changes what should be playing now cancels it. The worst was the headphones. `handle(.outputGone)` returned early because nothing was playing during the gap, so the repeat it should have stopped started a moment later through whatever speaker was left. The same early return let an interruption do it. Both cancel before their guard now, as `pause()` already did. Loading another recording left the old track's drill scheduled, so it would wake against the new item, seek it to the previous phrase's bounds and start it there — and the finish that followed would be handled as a phrase pass of a phrase from another text. Pressing play during the gap started the engine and left the repeat to restart the phrase a moment later. Seeking during the gap was worse: the repeat dragged the playhead back and played the old phrase over the reader. Seeking outside the phrase now leaves phrase playback altogether, rather than keeping a stop time that would halt them somewhere unrelated. The gap is now taken after the last pass too. It is the interval the learner writes in, and skipping it on the final pass meant a caller advancing from `onSegmentFinished` moved on the instant the audio stopped. And the repeat is scheduled before the state callback runs, not after, so a `pause()` issued from inside that callback finds a task to cancel. Writing the test for that last one found a hang of my own making: the early return in `pause()` announced a state change, so a listener that pauses in response to one called its own pause for ever. It announces nothing now — nothing observable had changed. One guard is kept although mutation testing shows it unreachable: a completion is reported only if the phrase is still the current one. Every path that changes the segment also cancels the task, so cancellation covers it today — but `segment` has an internal setter, and the cost of being wrong is a listener told "finished" about a phrase the reader has already left. The comment says exactly that rather than implying it is load-bearing. Co-Authored-By: Claude Opus 5 --- .../PlaybackModel+Interruptions.swift | 52 +++++ .../PlaybackModel+Phrase.swift | 40 +++- Sources/HanReaderPlayback/PlaybackModel.swift | 69 +++---- .../PhrasePlaybackTests.swift | 182 +++++++++++++++++- 4 files changed, 288 insertions(+), 55 deletions(-) create mode 100644 Sources/HanReaderPlayback/PlaybackModel+Interruptions.swift diff --git a/Sources/HanReaderPlayback/PlaybackModel+Interruptions.swift b/Sources/HanReaderPlayback/PlaybackModel+Interruptions.swift new file mode 100644 index 0000000..d047c87 --- /dev/null +++ b/Sources/HanReaderPlayback/PlaybackModel+Interruptions.swift @@ -0,0 +1,52 @@ +// HanReader — MIT licensed. See LICENSE. + +import Foundation + +/// Who is allowed to stop the narration, and who is allowed to start it +/// again. +/// +/// Its own file because the rules are not obvious and there are several: +/// the system may take the audio away and give it back, a spoken word may +/// duck it, the reader may have pressed pause, and a drill may be waiting +/// out a gap. Only one of those is allowed to undo each of the others. +extension PlaybackModel { + func handle(_ interruption: PlaybackInterruption) { + switch interruption { + case .began: + // Before the guard: during a gap nothing is playing, and a + // pending repeat would otherwise start during the interruption. + cancelDrill() + guard isPlaying else { return } + engine.pause() + isPlaying = false + pausedByInterruption = true + onStateChange?() + case .ended: + guard pausedByInterruption, state == .ready else { return } + pausedByInterruption = false + engine.play() + isPlaying = true + onStateChange?() + case .outputGone: + // Headphones pulled out. Pausing is not politeness, it is the + // difference between a quiet room and the reader's audio playing + // through the speaker to everyone around them — and it must not + // be resumable automatically, or plugging back in starts it in + // the open. + // + // The reasons are cleared *before* the guard, not after it: the + // output can be lost while already paused by an interruption, + // and leaving that reason set means the interruption ending + // resumes narration into a room with no headphones in it. + clearPauseReasons() + // Likewise, and this is the one that matters most: headphones + // pulled out during the gap would otherwise be followed by the + // phrase starting through whatever speaker is left. + cancelDrill() + guard isPlaying else { return } + engine.pause() + isPlaying = false + onStateChange?() + } + } +} diff --git a/Sources/HanReaderPlayback/PlaybackModel+Phrase.swift b/Sources/HanReaderPlayback/PlaybackModel+Phrase.swift index 0cefa9a..5877670 100644 --- a/Sources/HanReaderPlayback/PlaybackModel+Phrase.swift +++ b/Sources/HanReaderPlayback/PlaybackModel+Phrase.swift @@ -45,25 +45,45 @@ extension PlaybackModel { } /// One pass through the phrase ended, at exactly its end time. + /// + /// The gap is taken **after every pass, including the last**. It is the + /// interval the learner writes in, so skipping it on the final pass + /// means a caller that advances from `onSegmentFinished` moves on the + /// instant the audio stops — no time to answer, which is the one thing + /// the drill exists to provide. func finishSegmentPass() { engine.pause() isPlaying = false clearPauseReasons() repeatsPlayed += 1 - guard repeatsPlayed < drill.repeats else { - onStateChange?() - onSegmentFinished?() - return - } - onStateChange?() + let wasLast = repeatsPlayed >= drill.repeats + let phrase = segment + + // Scheduled *before* the state callback runs, so that a `pause()` + // issued from inside it finds a task to cancel. Assigning afterwards + // meant cancellation found nothing and the repeat started anyway, + // despite the reader having paused. drillTask = Task { [weak self] in - guard let gap = self?.drill.gap, let wait = self?.wait else { return } - if gap > 0 { - await wait(gap) + guard let self else { return } + if drill.gap > 0 { + await wait(drill.gap) } guard !Task.isCancelled else { return } - self?.playSegmentOnce() + guard wasLast else { + playSegmentOnce() + return + } + // Belt and braces, and said plainly: every path that changes + // the segment today also cancels this task, so the check above + // already covers it and mutation testing shows this line is + // unreachable. It is kept because `segment` has an internal + // setter and the cost of being wrong here is a listener told + // "finished" about a phrase the reader has already left — + // which would advance them past the one they just chose. + guard segment == phrase else { return } + onSegmentFinished?() } + onStateChange?() } /// Awaits a repeat that is currently waiting out its gap. diff --git a/Sources/HanReaderPlayback/PlaybackModel.swift b/Sources/HanReaderPlayback/PlaybackModel.swift index 391de50..db45cb3 100644 --- a/Sources/HanReaderPlayback/PlaybackModel.swift +++ b/Sources/HanReaderPlayback/PlaybackModel.swift @@ -139,10 +139,10 @@ public final class PlaybackModel { /// confusion: a system interruption that never invites us back leaves its /// flag set, and with one shared flag the next word the reader tapped /// would end by resuming narration the system had taken away. - @ObservationIgnored private var pausedForSpeech = false + @ObservationIgnored var pausedForSpeech = false /// Paused by the system — a call, an alarm, another app. - @ObservationIgnored private var pausedByInterruption = false + @ObservationIgnored var pausedByInterruption = false @ObservationIgnored let engine: any AudioEngine @ObservationIgnored private let session: any PlaybackSession @@ -202,6 +202,13 @@ public final class PlaybackModel { /// sound, and the stored playhead is a resume point rather than an /// instruction. public func load(_ url: URL, startingAt time: Double? = nil) { + // Before anything else. A repeat still waiting out its gap would + // otherwise wake against the *new* item, seek it to the old + // phrase's bounds and start it there — and the finish that followed + // would be handled as a phrase pass of a phrase from another text. + cancelDrill() + segment = nil + engine.setStopTime(nil) engine.load(url) state = .ready duration = engine.duration @@ -246,6 +253,11 @@ public final class PlaybackModel { public func play() { guard state == .ready else { return } + // Pressing play during the gap between repeats means "play now", so + // the repeat that was going to start by itself is abandoned. + // Without this both happen: the engine is started here and the + // pending task restarts the phrase a moment later. + cancelDrill() // The session is activated here and nowhere else. Activating it at // launch would interrupt whatever the reader was listening to, for an // app that might never play a sound. @@ -268,10 +280,11 @@ public final class PlaybackModel { // silence, and then hears the phrase start again. Pausing has to // mean "stop", whether or not sound is coming out at this instant. cancelDrill() - guard isPlaying else { - onStateChange?() - return - } + // No notification on this path, deliberately. Nothing observable + // changed — `isPlaying` was already false — and announcing it is a + // way to recurse for ever: a listener that responds to a state + // change by pausing would be called by its own pause. + guard isPlaying else { return } engine.pause() isPlaying = false // Deliberate, so neither the system nor a finished utterance may @@ -297,7 +310,18 @@ public final class PlaybackModel { /// Moves the playhead and tells everyone, in one step. public func seek(to time: Double) { guard state == .ready else { return } + // The reader has moved the playhead themselves, so a repeat waiting + // to drag it back to the phrase's start is no longer what they + // asked for. + cancelDrill() let target = clamp(time) + // And moving outside the phrase leaves phrase playback altogether, + // rather than leaving a stop time that would halt them somewhere + // unrelated to where they went. + if let segment, !segment.contains(target) { + self.segment = nil + engine.setStopTime(nil) + } engine.seek(to: target) playhead = target // Published immediately rather than left to the next report, or the @@ -353,39 +377,6 @@ public final class PlaybackModel { onStateChange?() } - private func handle(_ interruption: PlaybackInterruption) { - switch interruption { - case .began: - guard isPlaying else { return } - engine.pause() - isPlaying = false - pausedByInterruption = true - onStateChange?() - case .ended: - guard pausedByInterruption, state == .ready else { return } - pausedByInterruption = false - engine.play() - isPlaying = true - onStateChange?() - case .outputGone: - // Headphones pulled out. Pausing is not politeness, it is the - // difference between a quiet room and the reader's audio playing - // through the speaker to everyone around them — and it must not - // be resumable automatically, or plugging back in starts it in - // the open. - // - // The reasons are cleared *before* the guard, not after it: the - // output can be lost while already paused by an interruption, - // and leaving that reason set means the interruption ending - // resumes narration into a room with no headphones in it. - clearPauseReasons() - guard isPlaying else { return } - engine.pause() - isPlaying = false - onStateChange?() - } - } - // MARK: - Engine reports /// Throttles the engine's four-a-second down to the one-a-second the diff --git a/Tests/HanReaderPlaybackTests/PhrasePlaybackTests.swift b/Tests/HanReaderPlaybackTests/PhrasePlaybackTests.swift index 4c50481..44910cd 100644 --- a/Tests/HanReaderPlaybackTests/PhrasePlaybackTests.swift +++ b/Tests/HanReaderPlaybackTests/PhrasePlaybackTests.swift @@ -35,23 +35,29 @@ struct PhrasePlaybackTests { } /// A loaded transport with its engine and its record of waits. - private struct Bench { + struct Bench { let engine: SpyEngine let model: PlaybackModel let waits: Waits + let session: SpySession } - private func bench(duration: Double = 100) -> Bench { + func bench(duration: Double = 100) -> Bench { + Self.makeBench(duration: duration) + } + + static func makeBench(duration: Double = 100) -> Bench { let engine = SpyEngine() let waits = Waits() + let session = SpySession() let model = PlaybackModel( engine: engine, - session: SpySession(), + session: session, wait: { seconds in waits.record(seconds) }, ) model.load(URL(fileURLWithPath: "/tmp/track.m4a")) engine.declareDuration(duration) - return Bench(engine: engine, model: model, waits: waits) + return Bench(engine: engine, model: model, waits: waits, session: session) } // MARK: - Stopping where the phrase stops @@ -103,7 +109,7 @@ struct PhrasePlaybackTests { /// Once through is pressing play on a phrase, and nothing more should /// happen when it ends. @Test("A phrase played once stops and says so") - func playedOnce() { + func playedOnce() async { let rig = bench() let (engine, model) = (rig.engine, rig.model) var finished = 0 @@ -112,8 +118,11 @@ struct PhrasePlaybackTests { model.play(segment: 12 ..< 17) #expect(engine.playCount == 1) engine.reachEnd() - #expect(!model.isPlaying) + + // Completion arrives after the writing gap, not the instant the + // audio stops — so it is awaited rather than assumed. + await model.awaitPendingRepeat() #expect(finished == 1) #expect(engine.playCount == 1) } @@ -141,6 +150,7 @@ struct PhrasePlaybackTests { #expect(finished == 0) engine.reachEnd() + await model.awaitPendingRepeat() #expect(engine.playCount == 3) #expect(finished == 1) #expect(!model.isPlaying) @@ -262,3 +272,163 @@ struct PhrasePlaybackTests { #expect(model.isPlaying) } } + +/// Nothing outlives the gap. +/// +/// A repeat waiting out its silence is a command still pending, and every +/// one of these is a way the reader can change their mind in between. +@MainActor +@Suite("Drill cancellation") +struct DrillCancellationTests { + private func bench(duration: Double = 100) -> PhrasePlaybackTests.Bench { + PhrasePlaybackTests.makeBench(duration: duration) + } + + /// The worst of the seven: headphones pulled out during the silence, + /// and a moment later the phrase plays through whatever speaker is + /// left. `.outputGone` returned early because nothing was playing. + @Test("Losing the output during the gap cancels the repeat") + func outputGoneDuringGapCancels() async { + let rig = bench() + let (engine, model) = (rig.engine, rig.model) + model.play(segment: 12 ..< 17, drill: .init(repeats: 3, gap: 0.5)) + engine.reachEnd() + + rig.session.interrupt(.outputGone) + await model.awaitPendingRepeat() + #expect(engine.playCount == 1) + #expect(!model.isPlaying) + } + + @Test("An interruption during the gap cancels the repeat") + func interruptionDuringGapCancels() async { + let rig = bench() + let (engine, model) = (rig.engine, rig.model) + model.play(segment: 12 ..< 17, drill: .init(repeats: 3, gap: 0.5)) + engine.reachEnd() + + rig.session.interrupt(.began) + await model.awaitPendingRepeat() + #expect(engine.playCount == 1) + } + + /// A repeat waking against another text's recording would seek it to + /// the previous phrase's bounds and start it there. + @Test("Loading another recording cancels the repeat and the phrase") + func loadingDuringGapCancels() async { + let rig = bench() + let (engine, model) = (rig.engine, rig.model) + model.play(segment: 12 ..< 17, drill: .init(repeats: 3, gap: 0.5)) + engine.reachEnd() + + model.load(URL(fileURLWithPath: "/tmp/other.m4a")) + await model.awaitPendingRepeat() + #expect(model.segment == nil) + #expect(engine.playCount == 1) + } + + /// Pressing play during the gap means play now — not play now *and* + /// again in half a second. + @Test("Play during the gap cancels the repeat") + func playDuringGapCancels() async { + let rig = bench() + let (engine, model) = (rig.engine, rig.model) + model.play(segment: 12 ..< 17, drill: .init(repeats: 3, gap: 0.5)) + engine.reachEnd() + + model.play() + await model.awaitPendingRepeat() + #expect(engine.playCount == 2) + #expect(model.isPlaying) + } + + /// And moving the playhead means the reader has gone somewhere else. + @Test("Seeking during the gap cancels the repeat") + func seekingDuringGapCancels() async { + let rig = bench() + let (engine, model) = (rig.engine, rig.model) + model.play(segment: 12 ..< 17, drill: .init(repeats: 3, gap: 0.5)) + engine.reachEnd() + + model.seek(to: 50) + await model.awaitPendingRepeat() + #expect(engine.playCount == 1) + // Outside the phrase, so phrase playback is left behind rather than + // leaving a stop time somewhere unrelated. + #expect(model.segment == nil) + } + + /// Seeking *inside* the phrase is the case that needs the cancellation: + /// the segment survives, so a pending repeat would drag the playhead + /// back to its start and play it again over the reader. + @Test("Seeking inside the phrase keeps it but still cancels the repeat") + func seekingInsideKeepsTheSegment() async { + let rig = bench() + let (engine, model) = (rig.engine, rig.model) + model.play(segment: 12 ..< 17, drill: .init(repeats: 3, gap: 0.5)) + engine.reachEnd() + + model.seek(to: 15) + await model.awaitPendingRepeat() + + #expect(model.segment == 12 ..< 17) + #expect(model.playhead == 15) + #expect(engine.playCount == 1) + } + + /// Pausing from inside the state callback used to find no task to + /// cancel, because the repeat was scheduled after the callback ran. + @Test("Pausing from the state callback stops the repeat") + func pauseFromStateCallbackStops() async { + let rig = bench() + let (engine, model) = (rig.engine, rig.model) + model.play(segment: 12 ..< 17, drill: .init(repeats: 3, gap: 0.5)) + // Pauses once, the way a view would when it sees playback stop. + var hasPaused = false + model.onStateChange = { [weak model] in + guard !hasPaused, model?.isPlaying == false else { return } + hasPaused = true + model?.pause() + } + engine.reachEnd() + await model.awaitPendingRepeat() + #expect(engine.playCount == 1) + } + + /// The gap is the writing interval, so it belongs after the last pass + /// too. A caller advancing from the callback would otherwise move on + /// the instant the audio stopped. + @Test("The last pass is followed by the gap before completion") + func lastPassWaitsTheGapToo() async { + let rig = bench() + let (engine, model, waits) = (rig.engine, rig.model, rig.waits) + var finished = 0 + model.onSegmentFinished = { finished += 1 } + + model.play(segment: 12 ..< 17, drill: .init(repeats: 1, gap: 2)) + engine.reachEnd() + #expect(finished == 0) + + await model.awaitPendingRepeat() + #expect(waits.all == [2]) + #expect(finished == 1) + } + + /// And a phrase the reader has already left must not report completion + /// for the one they chose instead. + @Test("An abandoned phrase does not report completion") + func abandonedPhraseDoesNotReport() async { + let rig = bench() + let (engine, model) = (rig.engine, rig.model) + var finished = 0 + model.onSegmentFinished = { finished += 1 } + + model.play(segment: 12 ..< 17, drill: .init(repeats: 1, gap: 1)) + engine.reachEnd() + model.play(segment: 30 ..< 35) + await model.awaitPendingRepeat() + + #expect(finished == 0) + #expect(model.segment == 30 ..< 35) + } +}