Skip to main content

max / makenotwork

Cache the test template setup error instead of poisoning a Once A single failure to reach the admin database stated its cause on one line and left the other 1207 tests reporting only "Once instance has previously been poisoned" and "JoinError::Panic", so any triage that samples the output sees nothing usable. Replace the Once with a OnceLock holding the setup result, catch the panic inside it, and re-raise the original message from every later call.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-08-23 21:17 UTC
Signed with PGP, not checked
Commit: 14dfdc9a8859eb358e6f3b07e43c76d33d9e2086
Parent: 216a4c2
1 file changed, +128 insertions, -92 deletions
M server/tests/harness/db.rs +128 -92
@@ -6,7 +6,8 @@
6 6
7 7 use sqlx::postgres::PgPoolOptions;
8 8 use sqlx::{Connection, Executor, PgConnection, PgPool};
9 - use std::sync::{Once, OnceLock};
9 + use std::panic::AssertUnwindSafe;
10 + use std::sync::OnceLock;
10 11 use std::time::Duration;
11 12 use uuid::Uuid;
12 13
@@ -55,8 +56,29 @@
55 56 .await;
56 57 }
57 58
58 - /// Ensures template creation runs exactly once, across all threads and runtimes.
59 - static TEMPLATE_INIT: Once = Once::new();
59 + /// Ensures template creation runs exactly once, across all threads and runtimes,
60 + /// and remembers how it went.
61 + ///
62 + /// This was a `std::sync::Once`, which poisons when the closure panics. One
63 + /// failure to reach the database then reported its real cause on a single line
64 + /// out of ~15000, and the other 1207 tests said only "Once instance has
65 + /// previously been poisoned" and "template setup panicked: JoinError::Panic".
66 + /// Caching the setup error instead means every test after the first states the
67 + /// reason setup failed.
68 + static TEMPLATE_INIT: OnceLock<Result<(), String>> = OnceLock::new();
69 +
70 + /// Render a caught panic payload as its message.
71 + fn panic_message(payload: &(dyn std::any::Any + Send)) -> String {
72 + payload.downcast_ref::<&str>().map_or_else(
73 + || {
74 + payload
75 + .downcast_ref::<String>()
76 + .cloned()
77 + .unwrap_or_else(|| "template setup panicked".to_string())
78 + },
79 + |s| (*s).to_string(),
80 + )
81 + }
60 82
61 83 fn admin_url() -> String {
62 84 std::env::var("TEST_DATABASE_URL")
@@ -119,60 +141,68 @@
119 141 /// Create the template database with all migrations. Runs in a dedicated
120 142 /// single-threaded tokio runtime so it works from any context (including
121 143 /// inside `#[tokio::test]` and plain `#[test]`).
122 - fn ensure_template() {
123 - TEMPLATE_INIT.call_once(|| {
124 - let rt = tokio::runtime::Builder::new_current_thread()
125 - .enable_all()
126 - .build()
127 - .expect("build template setup runtime");
144 + fn ensure_template() -> Result<(), String> {
145 + TEMPLATE_INIT
146 + .get_or_init(|| {
147 + std::panic::catch_unwind(AssertUnwindSafe(template_setup))
148 + .map_err(|payload| panic_message(payload.as_ref()))
149 + })
150 + .clone()
151 + }
128 152
129 - rt.block_on(async {
130 - let t0 = std::time::Instant::now();
131 - let admin = admin_url();
132 - let mut conn = PgConnection::connect(&admin)
133 - .await
134 - .expect("connect to admin DB for template setup");
153 + /// The template setup itself. Panics on failure; `ensure_template` catches that
154 + /// and caches the message.
155 + fn template_setup() {
156 + let rt = tokio::runtime::Builder::new_current_thread()
157 + .enable_all()
158 + .build()
159 + .expect("build template setup runtime");
135 160
136 - // Adopt the shared role so the template (and every clone) is owned
137 - // by `mnw_test`, droppable by any member. No-ops on hosts without
138 - // the role, falling back to per-role namespacing below.
139 - assume_shared_role(&mut conn).await;
161 + rt.block_on(async {
162 + let t0 = std::time::Instant::now();
163 + let admin = admin_url();
164 + let mut conn = PgConnection::connect(&admin)
165 + .await
166 + .expect("connect to admin DB for template setup");
140 167
141 - // Namespace the template by the *effective* role: with the shared
142 - // role assumed this collapses to one template for everyone; without
143 - // it, each login role gets its own so a developer's `max` run and
144 - // the Sando gate's `sando` run never collide on a shared cluster.
145 - let (role,): (String,) = sqlx::query_as("SELECT current_user")
146 - .fetch_one(&mut conn)
147 - .await
148 - .expect("query current_user for template namespacing");
149 - let template = format!("{TEMPLATE_DB_BASE}_{}", sanitize_role(&role));
150 - let _ = TEMPLATE_NAME.set(template.clone());
168 + // Adopt the shared role so the template (and every clone) is owned
169 + // by `mnw_test`, droppable by any member. No-ops on hosts without
170 + // the role, falling back to per-role namespacing below.
171 + assume_shared_role(&mut conn).await;
151 172
152 - // Serialize template setup across ALL processes on this cluster, not
153 - // just this process's `Once`. Without it, concurrent test binaries
154 - // each unconditionally drop+recreate the SAME shared-role template,
155 - // so one drops the template another is mid-clone from, the mass
156 - // "template database ... does not exist" flake on the deploy gate.
157 - // Held until `conn` drops at the end of this block.
158 - sqlx::query("SELECT pg_advisory_lock($1)")
159 - .bind(TEMPLATE_LOCK_KEY)
160 - .execute(&mut conn)
161 - .await
162 - .expect("acquire template advisory lock");
173 + // Namespace the template by the *effective* role: with the shared
174 + // role assumed this collapses to one template for everyone; without
175 + // it, each login role gets its own so a developer's `max` run and
176 + // the Sando gate's `sando` run never collide on a shared cluster.
177 + let (role,): (String,) = sqlx::query_as("SELECT current_user")
178 + .fetch_one(&mut conn)
179 + .await
180 + .expect("query current_user for template namespacing");
181 + let template = format!("{TEMPLATE_DB_BASE}_{}", sanitize_role(&role));
182 + let _ = TEMPLATE_NAME.set(template.clone());
163 183
164 - // Reuse the template when it's already migration-current. Dropping a
165 - // live template is what races concurrent clones, and a FORCE drop
166 - // also fails when another role holds connections we can't terminate
167 - // ("permission denied to terminate process"). Only rebuild when the
168 - // template is missing or its migration head is stale.
169 - if template_is_current(&admin, &template).await {
170 - eprintln!("[test-harness] Reusing current template DB {template}");
171 - } else {
172 - // We hold the cross-process lock, so no clone can be reading it.
173 - conn.execute(
174 - format!("DROP DATABASE IF EXISTS \"{template}\" WITH (FORCE)").as_str(),
175 - )
184 + // Serialize template setup across ALL processes on this cluster, not
185 + // just this process's `Once`. Without it, concurrent test binaries
186 + // each unconditionally drop+recreate the SAME shared-role template,
187 + // so one drops the template another is mid-clone from, the mass
188 + // "template database ... does not exist" flake on the deploy gate.
189 + // Held until `conn` drops at the end of this block.
190 + sqlx::query("SELECT pg_advisory_lock($1)")
191 + .bind(TEMPLATE_LOCK_KEY)
192 + .execute(&mut conn)
193 + .await
194 + .expect("acquire template advisory lock");
195 +
196 + // Reuse the template when it's already migration-current. Dropping a
197 + // live template is what races concurrent clones, and a FORCE drop
198 + // also fails when another role holds connections we can't terminate
199 + // ("permission denied to terminate process"). Only rebuild when the
200 + // template is missing or its migration head is stale.
201 + if template_is_current(&admin, &template).await {
202 + eprintln!("[test-harness] Reusing current template DB {template}");
203 + } else {
204 + // We hold the cross-process lock, so no clone can be reading it.
205 + conn.execute(format!("DROP DATABASE IF EXISTS \"{template}\" WITH (FORCE)").as_str())
176 206 .await
177 207 .unwrap_or_else(|e| {
178 208 panic!(
@@ -181,49 +211,51 @@
181 211 )
182 212 });
183 213
184 - conn.execute(format!("CREATE DATABASE \"{template}\"").as_str())
185 - .await
186 - .expect("create template database");
214 + conn.execute(format!("CREATE DATABASE \"{template}\"").as_str())
215 + .await
216 + .expect("create template database");
187 217
188 - // Connect to the template and run all migrations
189 - let tpl_url = replace_db_name(&admin, &template);
190 - let tpl_pool = PgPoolOptions::new()
191 - .max_connections(2)
192 - .acquire_timeout(Duration::from_secs(10))
193 - .connect(&tpl_url)
194 - .await
195 - .expect("connect to template database");
218 + // Connect to the template and run all migrations
219 + let tpl_url = replace_db_name(&admin, &template);
220 + let tpl_pool = PgPoolOptions::new()
221 + .max_connections(2)
222 + .acquire_timeout(Duration::from_secs(10))
223 + .connect(&tpl_url)
224 + .await
225 + .expect("connect to template database");
196 226
197 - let t_migrate = std::time::Instant::now();
198 - #[allow(clippy::large_stack_arrays, reason = "sqlx::migrate! embeds the whole directory")]
199 - sqlx::migrate!("./migrations")
200 - .run(&tpl_pool)
201 - .await
202 - .expect("run migrations on template");
203 - let migrate_ms = t_migrate.elapsed().as_millis();
227 + let t_migrate = std::time::Instant::now();
228 + #[allow(
229 + clippy::large_stack_arrays,
230 + reason = "sqlx::migrate! embeds the whole directory"
231 + )]
232 + sqlx::migrate!("./migrations")
233 + .run(&tpl_pool)
234 + .await
235 + .expect("run migrations on template");
236 + let migrate_ms = t_migrate.elapsed().as_millis();
204 237
205 - // Also create the session store table
206 - let session_store = tower_sessions_sqlx_store::PostgresStore::new(tpl_pool.clone());
207 - session_store
208 - .migrate()
209 - .await
210 - .expect("session store migration on template");
238 + // Also create the session store table
239 + let session_store = tower_sessions_sqlx_store::PostgresStore::new(tpl_pool.clone());
240 + session_store
241 + .migrate()
242 + .await
243 + .expect("session store migration on template");
211 244
212 - tpl_pool.close().await;
245 + tpl_pool.close().await;
213 246
214 - let total_ms = t0.elapsed().as_millis();
215 - eprintln!(
216 - "[test-harness] Template DB created in {total_ms}ms (migrations: {migrate_ms}ms)"
217 - );
218 - }
247 + let total_ms = t0.elapsed().as_millis();
248 + eprintln!(
249 + "[test-harness] Template DB created in {total_ms}ms (migrations: {migrate_ms}ms)"
250 + );
251 + }
219 252
220 - // Release explicitly (also released when `conn` drops); the lock was
221 - // held across the whole reuse-or-rebuild window above.
222 - let _ = sqlx::query("SELECT pg_advisory_unlock($1)")
223 - .bind(TEMPLATE_LOCK_KEY)
224 - .execute(&mut conn)
225 - .await;
226 - });
253 + // Release explicitly (also released when `conn` drops); the lock was
254 + // held across the whole reuse-or-rebuild window above.
255 + let _ = sqlx::query("SELECT pg_advisory_unlock($1)")
256 + .bind(TEMPLATE_LOCK_KEY)
257 + .execute(&mut conn)
258 + .await;
227 259 });
228 260 }
229 261
@@ -244,9 +276,13 @@
244 276 // ensure_template uses std::sync::Once + its own runtime, safe from any context.
245 277 // When called from an async context, we run it on a blocking thread to avoid
246 278 // nesting runtimes.
247 - tokio::task::spawn_blocking(ensure_template)
248 - .await
249 - .expect("template setup panicked");
279 + // The setup error is cached, so every test after the first failure
280 + // reports the reason setup failed rather than a poisoned-Once message.
281 + match tokio::task::spawn_blocking(ensure_template).await {
282 + Ok(Ok(())) => {}
283 + Ok(Err(e)) => panic!("test database setup failed: {e}"),
284 + Err(e) => panic!("test database setup task failed: {e}"),
285 + }
250 286
251 287 let t0 = std::time::Instant::now();
252 288 let admin = admin_url();