Skip to content

Hold the latency test's segment on its first attempt, so a slow host reads as waiting rather than failing (#506) - #517

Merged
ramesh130 merged 3 commits into
mainfrom
issue-506-ios-latency-hold
Oct 13, 2026
Merged

ramesh130 merged 3 commits into
mainfrom
issue-506-ios-latency-hold

Conversation

@ramesh130

Copy link
Copy Markdown
Owner

Closes #506

The failure

The issue did not quote it. The #508 worker's local ios/check (2026-10-13, load average near 56) recorded it in its result bundle:

FaultScriptPlaybackTests.swift:72  testALatencyHoldsTheAddressedSegmentAndThePlayerWaitsItOut()
XCTAssertEqual failed: ("[[SuperPlayerTestkit.FaultScript.Kind.latency], [SuperPlayerTestkit.FaultScript.Kind.latency]]") is not equal to ("[[SuperPlayerTestkit.FaultScript.Kind.latency]]")

None of the failed CI runs still in the log window contain this test.

Cause

segment1.aac was asked for twice, and both requests were held. The test scripted 1 000 ms of latency on segment index 1 with no firstAttempts, so the hold applied to every attempt.

AVFoundation gives up on a segment that has sent no first byte within about one target duration. For this stream that is two seconds ("No response for media file in 2s", docs/testing.md's The fault sweep on iOS). It then asks again from byte zero.

The 1 s hold is measured on the origin's clock. On a loaded host the player can see it run past its 2 s wait. When that happens:

  • the player abandons the first request;
  • the retry is held as well;
  • the recorded faults read [[latency], [latency]].

A host slow enough to overrun every retry would never finish, and the test would fail on playToEnd's 60 s bound instead.

Probe evidence (a throwaway test, not committed). The probe printed the origin's responses for a hold applied to every attempt:

Hold What happened
1 s segment1 asked for once; ended in 11.7 s
5 s, 10 s, 20 s segment1 asked for again every few seconds, from byte zero, interleaved with media.m3u8 reloads; never ended within 90 s, and no segment after 1 was ever asked for

Reproduction (red before the fix, no host load)

The racing side was made to lose by lengthening only the first hold past AVFoundation's wait:

FaultScript.Builder()
    .addLatencyMs(4_000, kind: .mediaSegment, index: 1, firstAttempts: 1)  // the first attempt: 5 s in all
    .addLatencyMs(1_000, kind: .mediaSegment, index: 1)                    // every attempt: 1 s

With the old assertions, this failed on the first run with exactly the recorded message, [[latency], [latency]] against [[latency]].

Fix (test only)

The hold is scripted on the first attempt alone (firstAttempts: 1), the idiom the status, truncation and reset tests in this class already use. A retry, if there is one, is served untouched instead of being held again for the host to overrun.

The assertions are about waiting for the segment, not about whether the hold fit the player's wait:

  • the first segment1 response carried the latency, and every later one carried none;
  • assertUntouched is unchanged, so nothing else was faulted;
  • with consecutive repeats of the same path run together, the segment order starts segment0, segment1, segment2, and no segment appears twice. So every request for segment1 comes before the first request for segment2: the player never skipped ahead of the held segment;
  • every range is still from byte zero, unchanged.

Assertion strength. The old [[.latency]] (exactly one request) is the one claim given up. The number of attempts depends on host speed, which is what #506 says not to assert. Everything else is held as tightly as before or more tightly: the old order check covered only the first two segment requests.

Kept as is:

  • The 1 000 ms value, so a fast host still exercises the wait-it-out path.
  • The test name, so forcedBy's register and the issue's references stay valid. The player still waits for the segment, either by waiting out the hold or by asking for it again.

Verification

  • Slow-host path: the fixed test with the first hold raised to 5 s passes. In that case the player abandons the hold and the retry is served.
  • xcodebuild test -test-iterations 20 -only-testing:SuperPlayerTestkitTests/FaultScriptPlaybackTests/testALatencyHoldsTheAddressedSegmentAndThePlayerWaitsItOut on sp-506: 20/20 passed, on both commits.
  • The whole FaultScriptPlaybackTests class: 9/9 passed.
  • SP_SIMULATOR=sp-506 ios/check passed on the final commit.
  • An earlier ios/check run, while the host's load was near 40, failed one unrelated test:
    • FrameOutputPacingTests/testTheLinkIsAskedForTwiceTheContentsFrameRate: XCTAssertEqualWithAccuracy failed: ("20.0") is not equal to ("60.0").
    • It passed alone straight afterwards. It is not touched here and may be worth an issue.
  • No device run; nothing here needs one.

Review summary

Standards: no hard findings.

  • Applied: the order check, the same finding the spec review made.
  • Applied: the doc comment's "half that… later than that" sentence is now "a one-second hold can outlast that two-second wait on a loaded host".
  • Declined: renaming the test, for the reason above.
  • Follow-up, not in scope: FaultSweepTests.swift:170 also holds every attempt (addLatencyMs(latencyMs, kind: segment, index: faultedSegment)), the same exposure. ADR-0021 rule 4's iOS i5: the fault sweep, the rung register and stale-live-playlist detection #421 addendum already records it as a margin.

Spec:

  • Fixed (one finding, raised by both reviews): the first version collapsed repeats and checked only prefix(3). A sequence s0, s1, s2, s1 would have passed it, so it did not establish that the player waited for the segment. The order is now required to contain no segment twice.
  • Accepted: the looser segment1 count, justified above.
  • Not mine to do: removing the test from the batch's known-flake list is the orchestrator's job; the list is not in the repository.
  • Scope creep: none.

Skipped: no change to docs/testing.md or ADR-0021. The mechanism is already written there (The fault sweep on iOS), and this test asserts no duration, so rule 4 needs no addendum.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MJAhQmFgdRRAKUaMHibQLe

ramesh130 and others added 3 commits October 13, 2026 21:24
…low for the hold reads as waiting rather than failing

AVFoundation gives up on a segment with no first byte within about a target
duration and asks again from byte zero. A 1 s hold addressed to every attempt
was therefore held twice on a loaded host, and the test read
[[latency], [latency]] where it expected [[latency]] (#506). The hold now
addresses the first attempt alone, and the test asserts what it is about: the
held segment was held, any retry was served untouched, and the player asked
for nothing past segment 1 until it had it.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01MJAhQmFgdRRAKUaMHibQLe
…n once a later one was

Run together, the repeated requests showed only that segment 2 was first asked
for after segment 1 was; a fetch of segment 2 between two of segment 1 passed.
The order now has no segment twice, which is what "waits for it" means at the
origin.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01MJAhQmFgdRRAKUaMHibQLe
@ramesh130
ramesh130 merged commit a8617c5 into main Oct 13, 2026
3 checks passed
@ramesh130
ramesh130 deleted the issue-506-ios-latency-hold branch October 13, 2026 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flake (iOS): FaultScriptPlaybackTests testALatencyHolds… fails under load

1 participant