Cover the OIDC loopback callback paths with real-socket tests

stephen commented

2026-09-06 16:12:19 +00:00

Owner

#258 made the loopback wait cancellable, but its one test never connects to the listener: it binds, cancels after 50ms and asserts on shutdown latency. The state comparison, the request-line bound and the accept loop were all unexercised, so a regression in any of them would have gone green.

What changed

Three tests that drive real connections at the bound listener: a matching state returns the code, a mismatched state aborts as CSRF without echoing the code or the presented state, and a silent socket is dropped at REQUEST_LINE_TIMEOUT so the redirect queued behind it still lands.
The starvation test runs on a paused clock, so it proves the timer fires without spending ten seconds. That needs tokio::time::Instant, since std's ignores the virtual clock.
Loopback tests take a mutex against each other, because they share the four fixed callback ports.
No production code changed. REQUEST_LINE_TIMEOUT is untouched: lowering it to suit a test would weaken the thing under test.

Testing
cargo test, cargo fmt --all -- --check and cargo clippy --locked --all-targets clean; each new test was also run against a mutant of the property it names (state comparison deleted, request-line bound removed) and only that test failed.

Closes #259

Midwork-Id: lane=claude-8 repo=rasterstate/fj clone=fj-259 branch=fix/oidc-loopback-coverage head=6903f3b4bc253c9049651f9dec531d414a81c992 minted=2026-09-06T16:12:13Z

#258 made the loopback wait cancellable, but its one test never connects to the listener: it binds, cancels after 50ms and asserts on shutdown latency. The state comparison, the request-line bound and the accept loop were all unexercised, so a regression in any of them would have gone green. **What changed** - Three tests that drive real connections at the bound listener: a matching `state` returns the code, a mismatched `state` aborts as CSRF without echoing the code or the presented state, and a silent socket is dropped at `REQUEST_LINE_TIMEOUT` so the redirect queued behind it still lands. - The starvation test runs on a paused clock, so it proves the timer fires without spending ten seconds. That needs `tokio::time::Instant`, since `std`'s ignores the virtual clock. - Loopback tests take a mutex against each other, because they share the four fixed callback ports. - No production code changed. `REQUEST_LINE_TIMEOUT` is untouched: lowering it to suit a test would weaken the thing under test. **Testing** `cargo test`, `cargo fmt --all -- --check` and `cargo clippy --locked --all-targets` clean; each new test was also run against a mutant of the property it names (state comparison deleted, request-line bound removed) and only that test failed. Closes #259 Midwork-Id: lane=claude-8 repo=rasterstate/fj clone=fj-259 branch=fix/oidc-loopback-coverage head=6903f3b4bc253c9049651f9dec531d414a81c992 minted=2026-09-06T16:12:13Z

stephen added 1 commit

2026-09-06 16:12:19 +00:00

Cover the OIDC loopback callback paths with real-socket tests

ci / check (pull_request) Successful in 11m3s

Details

ci / live-e2e (pull_request) Successful in 2m16s

Details

ci / coverage (pull_request) Successful in 2m18s

Details

6903f3b4bc

#258 made the loopback wait cancellable, and its one test asserts only that:
it binds, cancels after 50ms, and checks shutdown latency. It never connects.
So the state comparison, the request-line bound and the accept loop itself were
unexercised, and a regression in any of them would have been green.

These drive real connections at the bound listener rather than the pure parser,
which `parse_callback_*` already covers. Each was checked against a mutant of
the property it names: deleting the state comparison fails only the CSRF test,
and removing the request-line bound fails only the starvation test.

The starvation test runs on a paused clock so it does not spend ten seconds of
real time proving a timer fires. That needs `tokio::time::Instant`, since
`std`'s does not follow the virtual clock. `REQUEST_LINE_TIMEOUT` is unchanged;
lowering it to suit a test would weaken the thing under test.

Loopback tests share four fixed ports, so they take a mutex against each other.

Closes #259

stephen merged commit 60b51d49bc into main

2026-09-06 16:27:38 +00:00

stephen deleted branch fix/oidc-loopback-coverage

2026-09-06 16:27:38 +00:00

stephen referenced this pull request from a commit

2026-09-06 16:27:39 +00:00

Cover the OIDC loopback callback paths with real-socket tests (#260)

stephen referenced this pull request from a commit

2026-09-06 17:09:31 +00:00

Default PR merge messages to the PR body (#262)

stephen referenced this pull request

2026-09-06 22:06:06 +00:00