mirror of
https://github.com/block/buzz.git
synced 2026-11-14 13:21:02 +00:00
97188e90c6
## Summary Two Rust tests failed on loaded CI runners because their timing budget came from the wall clock or from a production timeout, not from the behavior they assert. Each fix changes only test code or `cfg(test)` code. Commits (cherry-picked unchanged from a local flake-fix series): - `test(cli): freeze the test clock in the assertion refresh tests` - `test(db): give the pool release Postgres tests a test-lane acquire budget` ### Root cause and fix per test **`federated_identity::tests::short_and_long_assertions_refresh_at_the_earlier_point`** and its twin in `desktop_provider` (`crates/buzz-cli`): failed in CI with `assertion left == right failed: lifetime 300`, left 2, right 1, at `federated_identity.rs:634`. - CI sighting, 11 November 2026, "Rust / Unit Tests", run 43122921918 attempt 1 (on #9342; the rerun passed): - https://github.com/block/buzz/actions/runs/43122921918/job/130868244938 - Root cause: under `cfg(test)`, `cached_assertion::unix_now()` was the wall clock plus a thread-local skew. The mock stamps `exp = unix_now() + lifetime`, then `CachedAssertion::get` computes `refresh_at(unix_now(), exp)` after the HTTP round trip, and the next call checks `unix_now() < refresh`. The test needed those reads to agree to the second. When the wall clock crossed a second boundary between them, the refresh point moved 1 s earlier, `advance(refresh - 1)` landed exactly on it, and the assertion was minted one step early. - Fix: a thread-local `FROZEN` base beside `SKEW`, and `test_clock::freeze(at)` to pin it. Under `cfg(test)`, `unix_now()` returns `FROZEN.unwrap_or(real_now) + SKEW`, so `advance` and its existing callers in `client.rs` behave as before. Both tests freeze the clock at the top of their loop body. The mock adapter runs on the same current-thread runtime, so `exp`, `refresh_at` and the check share one "now". Production code paths are unchanged. - Verification: each test run 1,440 times, 24 in parallel, before and after. `federated_identity`: 4 failures before, 0 after. `desktop_provider`: 1 failure before, 0 after. - Not seen in CI: the `desktop_provider` twin. It passed in the job above and is fixed here because it has the same race and failed locally. **`runtime::postgres_tests::pool_release_postgres_tests::silence_after_release_hook_does_not_strand_sqlx_ping`** (`crates/buzz-db`): panicked at `pool_release_postgres_tests.rs:204:10` with ``called `Result::unwrap()` on an `Err` value: PoolTimedOut`` after 0.85 s. - CI sighting, 10 November 2026, "PostgreSQL Domain / PostgreSQL Tests", run 42851548618 attempt 1 (on #8317; the rerun passed): - https://github.com/block/buzz/actions/runs/42851548618/job/129969964659 - Root cause: the test built its pool from the production `Db::read_pool_options`, whose 150 ms `READER_ACQUIRE_TIMEOUT` also bounded `connect_with`'s eager first dial (and `fresh_query`'s). On a busy runner the cold dial through `PausingProxy`, SCRAM and the reader `after_connect` hook overran it, so the test panicked at the initial `connect_with(...).unwrap()` before the release-hook recovery path it asserts ever ran. Production is not exposed: the relay dials the reader pool lazily, so that budget only governs request-time acquires, which fall back to the writer by design. - Fix: a 30 s `TEST_LANE_ACQUIRE_TIMEOUT` on both pool builders in this file: the reader test's `PoolOptions`, and `writer_pool`'s `DbConfig`, whose eagerly dialed 3 s default is the same exposure #9273 fixed for buzz-admin. `test_before_acquire(false)`, `after_connect` and `after_release` stay as in production, and recovery still completes at sqlx's 5 s return deadline inside the test's 10 s `await_capacity` budget. No assertions change. - Verification, against a local Postgres: 10 rounds of `pool_release_postgres_tests --ignored --test-threads=6` per binary. Before, 2 of 10 rounds failed at `:204:10` with `PoolTimedOut`, the CI signature. After, 0 of 10. Under 28 CPU busy loops, with before and after runs interleaved: 6 of 10 failed before, 0 of 10 after. - Not done here: `reader_pool_postgres_tests.rs` already carries its own 5 s test budget on `read_pool_options` for the same reason. This PR does not unify the two constants. One sighting of each failure was found in the CI runs from 7 to 12 November 2026 that were searched. ### Related issue N/A (flake fixes). No existing issue was found, and no open PR touches these tests or the four files changed here. ### Testing Human testing is still required. This PR does not carry `buzz-review-completed`; the author has not yet run these tests themselves on this branch. The repetition counts above were measured when the fixes were written, before this branch was split out. They were not repeated on this branch. #### Local checks on this branch The branch is one commit behind `main`; that commit changes only mobile and docs files, none of them touched here. Ran and passed at push time, on a checkout of this branch's head (`42b3faccc`): the organization push check, `branch-skew`, `push-head-scope`, `file-size-check` and git-lfs. Did not run: - The `rust-tests` and `desktop-tauri-checks` pre-push lanes. They were excluded from the push (`LEFTHOOK_EXCLUDE`) because the machine ran out of disk part-way through them on an earlier attempt. That attempt did not complete, so it proves nothing either way about this branch. - The desktop and mobile pre-push lanes (`desktop-check`, `desktop-typecheck`, `desktop-test`, `mobile-checks`). This diff touches no desktop or mobile file. - `cargo fmt --check`, `cargo clippy`, `just ci`, and the PostgreSQL lane. CI's Unit Tests and PostgreSQL Tests jobs are the gate for this branch. A green run does not prove the flakes are gone: both failures need a loaded runner. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: mtoohey <mtoohey@squareup.com> Signed-off-by: Matt Toohey <contact@matttoohey.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>