Skip to content

fix(rs): repair the test call sites the main merge missed - #3287

Closed
kixelated wants to merge 1 commit into
devfrom
claude/fix-dev-compile
Closed

fix(rs): repair the test call sites the main merge missed#3287
kixelated wants to merge 1 commit into
devfrom
claude/fix-dev-compile

Conversation

@kixelated

Copy link
Copy Markdown
Collaborator

Summary

origin/dev at 3227c19 ("Merge main into dev") does not compile: the merge carried three tests over from main without updating them for dev's APIs. The first two are hard errors, and both crates are dependents of widely-used ones, so just check and just test fail for essentially every branch based on dev (that is how I ran into this, from #3282).

  • moq-video (encode/producer.rs): a test called catalog.track_info(), but dev's moq_mux::catalog::Producer::track_info takes a priority: u8 (feat(net)!: divide one connection's bandwidth estimate among its tracks #2854). The production call site in the same file was updated by the merge; this one was missed.

  • moq-rtc (lib.rs): the WHIP/WHEP round-trip test built origins with moq_net::Origin::random().produce() and announced by passing a Route to create_broadcast. On dev an origin is origin::Info plus a driver (moq_tokio::origin::spawn, already a dev-dependency), and announcing is separate from creating: create_broadcast(path) then announce(prefix, Route), in create-populate-announce order. The announcement assertions move from path/broadcast.is_some() to prefix/active.

  • moq-video again: with the first fix in place, idle_capture_publishes_a_discontinuity_before_resume compiled and failed ([1, 1], expected [1, 0, 1]) — a real behavior difference the compile error had been hiding.

    Root cause: the marker is still published (container::Producer::discontinuity appends an empty group and finishes it). What changed is delivery. A group's staleness is measured by its reach, which is bounded by its successor's start, so the empty group's reach is the resumed group's timestamp — equal to the live edge, and >= a Duration::ZERO budget. The default subscription therefore sheds it. That is correct for a live consumer, which takes the marker as the live edge when it arrives with no successor; it is wrong only for a test that walks the finished track afterwards. Fixed by subscribing with a replay window, the same thing the moq-binary / moq-json stream tests do for the same reason.

Test-only changes; no production code touched.

Public API changes

None.

Test plan

  • just check and just test (diff-scoped to moq-rtc, moq-video, and their dependents): 376/376 pass. Both failed on origin/dev before this.
  • cargo test -p moq-rtc: 44 pass, including whip_and_whep_round_trip_opus, which had not been able to run.
  • A full just check-all / just test all is running; I will note anything else it turns up.

Not fixed here

just drafts check fails on draft-lcurley-moq-hang.md:425, a {{field-timeline}} cross-reference with no anchor (from #3109). The timeline field exists on the json and binary rendition schemas, but the draft never got a ### timeline {#field-timeline} section, and the sentence claims the field "carries the same meaning here as for a media rendition" when media renditions have no such field. Writing that section is spec authoring rather than a build repair, so it wants an author. Nothing in CI runs the drafts check, so it blocks no workflow.

(Written by claude-opus-5)

The main-into-dev merge (3227c19) left three tests behind, and the
first two stopped the workspace compiling, so `just check` and `just
test` failed for every branch based on dev.

`moq-video` called `catalog.track_info()` in a test while dev's
`moq-mux` signature takes a priority; the production call site in the
same file was already updated. `moq-rtc` built its origins with
`moq_net::Origin::random()` and announced through
`create_broadcast(path, Route)`, both replaced on dev by
`origin::Info` plus a separate `announce(prefix, Route)`.

`moq-video`'s idle-capture test then compiled and failed. The
discontinuity marker is still published; the test read the finished
track through the default `Duration::ZERO` budget, which sheds the
empty group as soon as the resumed group stamps its reach. A live
consumer takes that marker as the live edge, so the fix is a
subscription with a replay window, matching the other tests that walk a
finished multi-group track.

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

chatgpt-codex-connector Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-01T17:44:58.442001Z 16af941 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@kixelated

Copy link
Copy Markdown
Collaborator Author

Closing: every breakage this PR repaired landed first, so nothing here is still needed.

The one thing still outstanding is unrelated to the build: just drafts check fails on draft-lcurley-moq-hang.md:425, a {{field-timeline}} cross-reference with no anchor (from #3109). The timeline field is real on the json and binary rendition schemas, but the draft has no ### timeline {#field-timeline} section, and the sentence claims the field "carries the same meaning here as for a media rendition" when media renditions do not have it. That wants an author rather than a build fix, and nothing in CI runs the drafts check.

(Written by claude-opus-5)

@kixelated kixelated closed this Sep 1, 2026
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.

1 participant