Skip to content

test(auth): run the outage tests on a paused clock without sockets - #4527

Merged
kixelated merged 6 commits into
mainfrom
quest/m1/auth-outage-clock
Sep 29, 2026
Merged

kixelated merged 6 commits into
mainfrom
quest/m1/auth-outage-clock

Conversation

@kixelated

Copy link
Copy Markdown
Collaborator

Problem

#4244 and #4291 moved the relay and moq-auth outage tests to the real clock, because a paused clock auto-advances while the runtime waits on a real socket and a virtual timer fired before macOS delivered loopback. That broke "unit tests mock time", added ~3s of wall time each, and dropped the upper bound: a session that closed late at expires would pass.

Approach

Take the sockets out of both tests, and give each one assertion.

  • moq-auth (an_outage_keeps_the_grant_until_expires): the lease driver talks to a private Post trait. Client implements it over HTTP, and the tests use an in-process Script that logs requests and answers directly. Validation moved out of the HTTP path into a shared ask, so both answer sources are held to the same grant contract. The test runs on the paused clock, answers re-checks as an outage, and asserts both bounds: still live at expires - 1ms, closed as Expired by expires + 1ms, end reported as Expired. The wall-clock expires is bracketed around connect, since the driver maps it onto Tokio's clock there. It folds in a_grant_closes_at_its_expiry, which checked the same bounds without re-checks. an_expired_grant_is_refused and expiry_fires_while_a_recheck_is_stalled move onto Script too, and the axum-on-paused-clock clock_server helper is gone.
  • moq-relay (an_outage_keeps_the_session_until_expires): the outage semantics now belong to moq-auth, so the relay test only proves that a lease reaching expires closes the session and reports the end. An embedded decider grants a lease and then never answers again, which looks like an outage to the relay. supervise holds a moq_net::Session over qmux on a tokio::io::duplex. On the paused clock the test asserts both bounds and checks that the decider sees Expired. No new API was needed: Auth::embedded, Auth::admit, and supervise are already public.

Each test now takes about 0.1s instead of about 3s.

Verification

  • Mutations, for each crate: moving the expiry timer 100ms earlier fails "must not close before expires". Moving it 100ms later fails "closed at expires, not later".
  • Stress: all four paused-clock tests, --stress-count 300, with a yes pinning each of the 32 cores (load average was already about 31). 300/300 passed. This was Linux only; no macOS host was available, but no socket is left for macOS to deliver late.
  • just check: clippy and the 565 affected tests pass. quest check fails on main for an unrelated reason: quest/m2/ts-import-health.md links the finished /quest/m1/srt-import-stats.md.

Impact

  • Public API: none. Post is private to moq-auth::client.
  • Wire: none.

Follow-ups

  • fixed_addresses_keep_tls_name_and_request_host and ipv6_literal_fixed_addresses (moq-tokio websocket) still pause the clock over a real TLS dial. They are safe until a timeout lands on that path. They exercise real DNS/TLS/Host handling, so there was no cheap seam here.
  • Fix the dangling srt-import-stats link in quest/m2/ts-import-health.md.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

kixelated and others added 3 commits September 29, 2026 11:16
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Outcome: the quest is done and this PR stays a draft for review.

Open decisions:

  • Merging a_grant_closes_at_its_expiry into the outage test. Recommendation: keep it merged. The outage test checks the same two bounds and also covers re-checks landing between them, which is the path the fix(auth): keep accepted grants on fixed expiry deadlines #4237 regression used.
  • Whether the relay test should reach the real moq_auth::Client driver instead of an embedded decider. Recommendation: no. That needs a public transport seam, and the outage semantics are tested once, in moq-auth.

Suggested follow-ups:

  • Leave the moq-tokio websocket paused-clock tests as they are until a timeout lands on the TLS dial path. Recommendation: track them under quest/m1/test-flakes-2.md, not in a new quest.
  • Drop the dangling /quest/m1/srt-import-stats.md link from quest/m2/ts-import-health.md. quest check fails on main because of it.

(Written by Claude Opus 5.5)

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

Copy link
Copy Markdown
Collaborator Author

I added the moq-tokio websocket paused-clock tests to quest/m1/test-flakes-2.md.

I also looked into whether moq_auth::Client should get a public seam, so the relay test could drive the real client against an in-process answer source. Recommendation: not now.

  • No consumer needs it. The only in-tree users of Client are moq-relay (Decider::Server) and moq-auth's own tests. Embedders who decide in process already use Auth::embedded with a lease::Producer. doc/lib/rs/moq-auth.md describes that path on purpose as "no trait and no HTTP".
  • The test gain is small. moq-auth already tests that an outage keeps the grant until expires, and the relay test covers enforcement. Both expiry timers end with Reason::Expired, so running the two layers together adds little.
  • The cost is a permanent extension point. It would be additive on a published crate, but every change to it afterwards would be breaking. Post as written uses impl Future returns, which can't be boxed into a trait object, and it splits end into a second method.

If a real consumer appears, for example running the reference serve::Server policy in process with the full lease lifecycle, the smallest shape I'd suggest is:

  • one object-safe trait, shaped like serve::Server::answer: Result<Option<Grant>> per event;
  • one constructor, Client::from_answers(Arc<dyn Answer>), implemented for serve::Server;
  • no change to the relay. The test can re-create Decider::Server with Auth::embedded plus client.connect(admission.request).

(Written by Claude Opus 5.5)

@kixelated
kixelated marked this pull request as ready for review September 29, 2026 18:57
@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Next included review available in 34 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 9e65aaa3-97d9-4be3-9d87-3bdaf9c7b5ed

📥 Commits

Reviewing files that changed from the base of the PR and between 66a60f4 and bb58c5c.

📒 Files selected for processing (5)
  • quest/m1/README.md
  • quest/m1/auth-outage-clock.md
  • quest/m1/test-flakes-2.md
  • rs/moq-auth/src/client.rs
  • rs/moq-relay/tests/auth_lifetime.rs

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 29, 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-29T22:32:18.408176Z bb58c5c New commits
ℹ️ 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
kixelated merged commit 6d0a88d into main Sep 29, 2026
4 of 5 checks passed
@kixelated
kixelated deleted the quest/m1/auth-outage-clock branch September 29, 2026 23:51
@moq-bot moq-bot Bot mentioned this pull request Sep 30, 2026
@moq-bot moq-bot Bot mentioned this pull request Sep 30, 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