mirror of
https://github.com/block/buzz.git
synced 2026-11-13 21:01:53 +00:00
## Summary Two buzz-relay test fixes from the 23–30 Oct flake audit. Both are test-only. This is one of six PRs split from the reviewed `flakey-tests` branch (siblings below). ### `thread_window_bridge_restarts_both_aux_hops_after_replica_failure` (3 runs) - **Runs:** 40799617319, 40924179886 and 40930100994 (#8697, docs only), all on 10-29 in PostgreSQL Tests, attempt 1 (jobs 123208043033, 123632125866, 123651641392). None of the branches touches thread_window, and each run passed on a re-run. Signature: `failure_postgres_tests.rs:46`, "barrier must precede closure completion", left: 2, right: 1. - **Cause:** `pause_after_aux_page` polls the request until a `thread_window_aux` span closes, then expects exactly one. The fixture's 1,001 reactions make two replica pages at `AUX_LIMIT` 1000, and each page is one round trip on the transaction already held. A loaded runner can deschedule the test thread after page 2's query is written. Postgres then answers first, and the same poll reads page 2 too. - **Change:** `test_support` gains a `PausingProxy`, a trimmed copy of buzz-db's private one with only the hold-everything switch. The replica reader pool connects through it. `AuxPages` pauses it when the first aux span closes. That runs inside the request's poll, after page 1's response is consumed and before page 2's query is written, so page 2 stays pending however fast Postgres answers. The test resumes the proxy after it terminates the reader's backend. The held page then sees the termination, and the closure restarts on the writer, as before. The other three `pause_after_aux_page` callers read from the writer and pass `None`. ### `api::media` tests that need Postgres (fail every time Flake Check selects them) - **Run:** 40954304982 (#8658), 10-29, Flake check (Rust), job 123730434146. All six `test_state()` tests panic at the community-seed `expect` after the 30 s sqlx acquire timeout. - **Cause:** when a hunk lands outside a test body, Flake Check selects every test in the file. It skips only `postgres_tests::`, `postgres_*` binaries and `external_infra`, and its job has no database. These six tests seed `relay.example` through Postgres but sat in plain `mod tests` without `#[ignore]`. So any PR that changes non-test code in `media.rs` fails that lane, and at least 8 open PRs touch the file. No other lane ran these tests either. - **Change:** move `test_state`, the `media_get_*` helpers, `media_request` and the six tests into `tests::postgres_tests`. Mark each test `#[ignore = "requires Postgres"]`, and point `test_state` at `test_support::database_url()`, as the module's other states do. The PostgreSQL lane now runs them, and Flake Check skips them. The change also drops imports `mod tests` no longer uses, and updates the Justfile comment that gave `api::media` as the reason the infra-free lane is scoped to `api::admin`. - When `media.rs` is staged, the pre-commit secret scan reads the whole file and flags the module's existing public ES256 test key (`TEST_EC_PKCS8_PEM`). The key is now written on one line with a sadscan marker, as `invites.rs` already does. Its bytes are unchanged. ### Related issue None found. **Conflicts with #8451 and #8444**, which both already conflict with `main`. The one-line `TEST_EC_PKCS8_PEM` rewrite is the only new conflict hunk with each. The test move merges cleanly with both. ### Testing Verified locally: - Each of the four `pause_after_aux_page` tests passes when run alone against local Postgres through `scripts/postgres-test-run.sh`. - A forced check, not committed: after the barrier, the test gave page 2 another 500 ms over ten re-polls and asserted the page count stayed at 1. It passed with the pause and failed with `None`, with the CI signature (left: 2, right: 1). A true same-poll burst needs the OS to preempt between sqlx's write and read, so it was not forced directly. - `check-postgres-test-discovery.py` accepts the touched files and all 621 files under `crates/`, and `scripts/test-postgres-test-discovery.sh` passes. Each of the six media tests passes when run alone against local Postgres. - `cargo fmt --check` and `cargo clippy -p buzz-relay --all-targets -- -D warnings` are clean under 1.98.1. Only hosted CI can show the full lanes, which were not run locally. The six media tests have never run in CI, so this PR's PostgreSQL lane is their first hosted run. Pushed with `git push --no-verify`, because the pre-push suites fail when the laptop is under load. ### Readiness An agent review of the combined `flakey-tests` branch (b432453af) found no blockers. `buzz-review-completed` is not added, because the human test step (AGENTS.md "Before opening a PR", step 3) hasn't happened yet. ### Siblings - #8739 ci: Hermit fetch deadline and sccache fallback - #8741 test(mobile): RunnerTests title fade and alert wait - #8742 fix(desktop): deep-linked message hold (product change) - #8743 test(desktop): four specs racing scroll settles and focus - #8744 test(desktop): five settle classes and remote-owned-mentions 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Matt Toohey <contact@matttoohey.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
220 lines
8.3 KiB
Rust
220 lines
8.3 KiB
Rust
const DEFAULT_DATABASE_URL: &str = "postgres://buzz:buzz_dev@localhost:5432/buzz"; // sadscan:disable np.postgres.1 -- local test-only credentials
|
|
|
|
/// Serialize lib tests whose assertions depend on which tracing dispatchers
|
|
/// are live: log-capture and callsite-interest tests, and tests that fire the
|
|
/// callsites they assert on. tracing-core caches each callsite's interest
|
|
/// process-wide from whichever dispatchers are live when it is first hit or
|
|
/// rebuilt, so a parallel test's dispatcher (or a thread with none) can enable
|
|
/// or disable another test's callsites. Not reentrant; take it once per test.
|
|
/// Two async sites install a dispatcher without it, because the guard would be
|
|
/// held across `.await`: `router.rs`'s otel-export test and the
|
|
/// `pause_after_aux_page` helper in
|
|
/// `api/bridge/thread_window/postgres_tests/failure_postgres_tests.rs`.
|
|
pub(crate) fn tracing_dispatch_lock() -> std::sync::MutexGuard<'static, ()> {
|
|
static LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
|
|
LOCK.lock().unwrap_or_else(|poisoned| poisoned.into_inner())
|
|
}
|
|
|
|
/// Resolve the database URL shared by PostgreSQL-backed relay tests.
|
|
pub(crate) fn database_url() -> String {
|
|
std::env::var("BUZZ_TEST_DATABASE_URL")
|
|
.or_else(|_| std::env::var("TEST_DATABASE_URL"))
|
|
.or_else(|_| std::env::var("DATABASE_URL"))
|
|
.unwrap_or_else(|_| DEFAULT_DATABASE_URL.to_owned())
|
|
}
|
|
|
|
#[cfg(test)]
|
|
const CHILD_TEST_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
|
|
#[cfg(test)]
|
|
const MAX_CAPTURE_BYTES: u64 = 1024 * 1024;
|
|
|
|
#[cfg(test)]
|
|
struct CapturedStream {
|
|
retained: Vec<u8>,
|
|
total_bytes: u64,
|
|
}
|
|
|
|
#[cfg(test)]
|
|
fn capture_stream(mut stream: impl std::io::Read) -> CapturedStream {
|
|
let mut retained = Vec::new();
|
|
let mut total_bytes = 0_u64;
|
|
let mut chunk = [0_u8; 8192];
|
|
loop {
|
|
let read = stream.read(&mut chunk).expect("read child output pipe");
|
|
if read == 0 {
|
|
break;
|
|
}
|
|
total_bytes = total_bytes.saturating_add(u64::try_from(read).expect("read size fits u64"));
|
|
let remaining = usize::try_from(MAX_CAPTURE_BYTES)
|
|
.expect("capture ceiling fits usize")
|
|
.saturating_sub(retained.len());
|
|
retained.extend_from_slice(&chunk[..read.min(remaining)]);
|
|
}
|
|
CapturedStream {
|
|
retained,
|
|
total_bytes,
|
|
}
|
|
}
|
|
|
|
#[cfg(test)]
|
|
fn join_capture(capture: std::thread::JoinHandle<CapturedStream>, stream: &str) -> Vec<u8> {
|
|
let capture = capture.join().expect("child capture thread must not panic");
|
|
assert!(
|
|
capture.total_bytes <= MAX_CAPTURE_BYTES,
|
|
"child {stream} exceeded {MAX_CAPTURE_BYTES} bytes: {}",
|
|
capture.total_bytes,
|
|
);
|
|
capture.retained
|
|
}
|
|
|
|
/// Run exactly one unit test in an isolated, deadline-bounded child process.
|
|
#[cfg(test)]
|
|
pub(crate) fn run_exact_test_child(test_name: &str, child_env: &str) {
|
|
use std::{
|
|
process::{Command, Stdio},
|
|
thread,
|
|
time::{Duration, Instant},
|
|
};
|
|
|
|
let mut child = Command::new(std::env::current_exe().expect("test executable"))
|
|
.arg("--exact")
|
|
.arg(test_name)
|
|
.arg("--nocapture")
|
|
.env(child_env, "1")
|
|
.stdout(Stdio::piped())
|
|
.stderr(Stdio::piped())
|
|
.spawn()
|
|
.expect("spawn isolated test child");
|
|
let stdout = child.stdout.take().expect("child stdout pipe");
|
|
let stderr = child.stderr.take().expect("child stderr pipe");
|
|
let stdout = thread::spawn(move || capture_stream(stdout));
|
|
let stderr = thread::spawn(move || capture_stream(stderr));
|
|
|
|
let deadline = Instant::now() + CHILD_TEST_TIMEOUT;
|
|
let (status, timed_out) = loop {
|
|
if let Some(status) = child.try_wait().expect("poll isolated test child") {
|
|
break (status, false);
|
|
}
|
|
if Instant::now() >= deadline {
|
|
let _ = child.kill();
|
|
let status = child.wait().expect("reap timed-out test child");
|
|
break (status, true);
|
|
}
|
|
thread::sleep(Duration::from_millis(10));
|
|
};
|
|
|
|
let stdout = join_capture(stdout, "stdout");
|
|
let stderr = join_capture(stderr, "stderr");
|
|
let output = format!(
|
|
"{}{}",
|
|
String::from_utf8_lossy(&stdout),
|
|
String::from_utf8_lossy(&stderr)
|
|
);
|
|
|
|
assert!(
|
|
!timed_out,
|
|
"isolated test child exceeded {CHILD_TEST_TIMEOUT:?}:\n{output}"
|
|
);
|
|
assert!(status.success(), "isolated test child failed:\n{output}");
|
|
assert!(
|
|
output.contains("running 1 test") && output.contains(test_name),
|
|
"exact selector did not run the intended test {test_name}:\n{output}"
|
|
);
|
|
}
|
|
|
|
/// Loopback TCP proxy that can pause the upstream database. While paused,
|
|
/// every session holds the bytes it receives with both sockets open; on
|
|
/// resume the held bytes are delivered in order and the sessions carry on.
|
|
/// A trimmed copy of buzz-db's test-only `PausingProxy`, which is private to
|
|
/// that crate's unit tests.
|
|
#[cfg(test)]
|
|
pub(crate) struct PausingProxy {
|
|
pub(crate) port: u16,
|
|
paused: tokio::sync::watch::Sender<bool>,
|
|
}
|
|
|
|
#[cfg(test)]
|
|
impl PausingProxy {
|
|
pub(crate) async fn spawn(upstream_host: String, upstream_port: u16) -> Self {
|
|
use tokio::io::{AsyncReadExt, AsyncWriteExt};
|
|
|
|
async fn pump(
|
|
mut from: tokio::net::tcp::OwnedReadHalf,
|
|
mut to: tokio::net::tcp::OwnedWriteHalf,
|
|
mut paused: tokio::sync::watch::Receiver<bool>,
|
|
) {
|
|
let mut buf = [0u8; 8192];
|
|
loop {
|
|
let Ok(n) = from.read(&mut buf).await else {
|
|
return;
|
|
};
|
|
if n == 0 {
|
|
return;
|
|
}
|
|
if paused.wait_for(|paused| !paused).await.is_err() {
|
|
return;
|
|
}
|
|
if to.write_all(&buf[..n]).await.is_err() {
|
|
return;
|
|
}
|
|
}
|
|
}
|
|
|
|
let listener = tokio::net::TcpListener::bind("127.0.0.1:0")
|
|
.await
|
|
.expect("bind proxy");
|
|
let port = listener.local_addr().expect("proxy addr").port();
|
|
let (paused, accept_paused) = tokio::sync::watch::channel(false);
|
|
tokio::spawn(async move {
|
|
while let Ok((client, _)) = listener.accept().await {
|
|
let Ok(server) =
|
|
tokio::net::TcpStream::connect((upstream_host.as_str(), upstream_port)).await
|
|
else {
|
|
continue;
|
|
};
|
|
let (client_read, client_write) = client.into_split();
|
|
let (server_read, server_write) = server.into_split();
|
|
tokio::spawn(pump(client_read, server_write, accept_paused.clone()));
|
|
tokio::spawn(pump(server_read, client_write, accept_paused.clone()));
|
|
}
|
|
});
|
|
Self { port, paused }
|
|
}
|
|
|
|
/// Hold or release every session's bytes. A pause applies to all bytes
|
|
/// the proxy reads after this call returns, on any runtime thread.
|
|
pub(crate) fn set_paused(&self, paused: bool) {
|
|
self.paused.send_replace(paused);
|
|
}
|
|
}
|
|
|
|
/// A database handle on which only restriction-state reads fail: its
|
|
/// `search_path` puts a schema first whose `community_bans` is a view that
|
|
/// errors when read, so every other table resolves to `public` as normal.
|
|
/// Returns the handle, an admin pool, and the schema to drop afterwards.
|
|
#[cfg(test)]
|
|
pub(crate) async fn restriction_lookup_failing_db() -> (buzz_db::Db, sqlx::PgPool, String) {
|
|
use sqlx::postgres::PgConnectOptions;
|
|
let url = database_url();
|
|
let admin = sqlx::PgPool::connect(&url)
|
|
.await
|
|
.expect("PostgreSQL must be available");
|
|
let schema = format!("ban_lookup_fails_{}", uuid::Uuid::new_v4().simple());
|
|
sqlx::raw_sql(sqlx::AssertSqlSafe(format!(
|
|
"CREATE SCHEMA {schema}; \
|
|
CREATE VIEW {schema}.community_bans AS \
|
|
SELECT * FROM public.community_bans WHERE 1 / (SELECT count(*) * 0 FROM public.communities) = 1;"
|
|
)))
|
|
.execute(&admin)
|
|
.await
|
|
.expect("create failing restriction schema");
|
|
let options = url
|
|
.parse::<PgConnectOptions>()
|
|
.expect("database url")
|
|
.options([("search_path", format!("{schema}, public"))]);
|
|
let pool = sqlx::PgPool::connect_with(options)
|
|
.await
|
|
.expect("failing restriction pool");
|
|
(buzz_db::Db::from_pool(pool), admin, schema)
|
|
}
|