adapter: let the turmoil migration tests share a process with mz_ore tests

[bosconi]

Motivation

cargo test -p mz-adapter --lib -- builtin_schema_migration fails on main when the module's tests share a process, which is cargo test's default:

test_builtin_schema_migration ... FAILED
  called `Result::unwrap()` on an `Err` value: SetGlobalDefaultError("a global default trace dispatcher has already been set")
test_evolution_skips_builtin_without_shard ... FAILED   (from #38917)
  Once instance has previously been poisoned

Two initializers install a global tracing subscriber and neither tolerates the other. configure_tracing_for_turmoil in builtin_schema_migration_tests.rs unwraps set_global_default. mz_ore::test::init_logging_default, which every #[mz_ore::test] calls, ends in FmtSubscriber::builder().init(), and in tracing-subscriber 0.3.23 that is try_init().expect("Unable to install global subscriber"). Both run under a Once, so whichever loses the race panics and poisons its Once: the turmoil one for every turmoil test after it, mz_ore's LOG_INIT for every #[mz_ore::test] in the binary that has not run yet. Which one loses is decided by libtest's parallel start of the first few tests, not by sort order. CI never sees any of this because nextest gives each test its own process, which is also why every test in the module passes when run alone.

Found while verifying #38917. The review comment on the first version of this PR pointed out that fixing only the turmoil side leaves the larger half.

Description

Two commits, one per initializer.

• configure_tracing_for_turmoil keeps an already-installed subscriber instead of unwrapping. Only the simulated-time formatting is lost in that ordering; the mz_ore subscriber also writes to the test writer.
• mz_ore::test::init_logging_default uses try_init() and ignores the error, so it can no longer poison LOG_INIT. Its doc comment now says so. This covers every binary that mixes #[mz_ore::test] with a test that installs its own subscriber.

Two other unwrapped set_global_default calls remain, in src/cluster/src/communication.rs (#[cfg(test)]) and src/service/tests/transport.rs. With the mz_ore change they can no longer be poisoned by mz_ore; they can still panic themselves if mz_ore wins, and they are left as they are here because neither currently shares a process with an #[mz_ore::test] that has bitten.

Verification

See the comment below for the runs: cargo test -p mz-ore --lib, the migration module in one process repeated ten times (was 2 of 4 failing before the first commit, and the reviewer measured 13 of 60 runs failing with only the first commit), and cargo clippy -p mz-ore --tests -D warnings. cargo fmt clean.

🤖 Generated with Claude Code

[def-]

QA LLM Review

1. MEDIUM -- Only one half of the subscriber race is fixed; the other half poisons mz_ore's shared LOG_INIT

src/adapter/src/catalog/open/builtin_schema_migration_tests.rs:408

The fix makes configure_tracing_for_turmoil tolerate a pre-installed subscriber, but the reverse ordering is still fatal and is the one with the bigger blast radius: if test_builtin_schema_migration reaches set_global_default first, the next #[mz_ore::test] to call mz_ore::test::init_logging panics inside LOG_INIT.call_once, and because LOG_INIT is a single process-wide Once shared by all 116 #[mz_ore::test] tests in the mz-adapter lib binary, every one of them that has not yet run fails with "Once instance has previously been poisoned". So cargo test on this module still fails, just with a different test blamed.

Details

mz_ore::test::init_logging_default (src/ore/src/test.rs:48) ends in FmtSubscriber::builder()...init(), and fmt::SubscriberBuilder::init is try_init().expect("Unable to install global subscriber") — it panics when a global default already exists, exactly the condition this PR now creates on purpose.

I reproduced it with a standalone crate replicating both initializers verbatim and the module's three test names (so libtest's sort order matches): 13 of 60 cargo test runs failed, each with

---- hydration_history_forced_migration_policy ----
panicked at tracing-subscriber/src/fmt/mod.rs:520:
Unable to install global subscriber: SetGlobalDefaultError("a global default trace dispatcher has already been set")
---- test_migration_steps_resolve_to_builtins ----
panicked at Once instance has previously been poisoned

0 of 30 failed with --test-threads=1, confirming it is the parallel start of the first N sorted tests, not a fixed ordering. The libtest dispatch order only biases the race toward hydration_history_forced_migration_policy (it sorts first); it does not decide it. A filter that selects test_builtin_schema_migration alongside test_migration_steps_resolve_to_builtins but not the hydration_* test makes the losing order the likely one.

Fix: make init_logging non-poisoning too, so neither initializer can poison the other's Once.

--- a/src/ore/src/test.rs
+++ b/src/ore/src/test.rs
@@
-        FmtSubscriber::builder()
+        // Another test in this binary may have installed a global subscriber
+        // already. Panicking here would poison `LOG_INIT` for every
+        // `#[mz_ore::test]` in the process, so keep whatever is installed.
+        let _ = FmtSubscriber::builder()
             .with_env_filter(filter)
             .with_test_writer()
-            .init();
+            .try_init();

fmt::SubscriberBuilder::try_init is inherent, so this needs no extra import. The doc comment on init_logging ("It is safe to call init_logging multiple times") only holds today because nothing else installs a subscriber; this PR makes that no longer true.

The same set_global_default(...).unwrap() pattern is still present at src/cluster/src/communication.rs:711 and src/service/tests/transport.rs:739, and the src/ore/src/test.rs fix covers those binaries too.

[bosconi]

Agreed, and verified: in tracing-subscriber 0.3.23 fmt::SubscriberBuilder::init is try_init().expect("Unable to install global subscriber") (src/fmt/mod.rs:518-520), and LOG_INIT is one process-wide Once, so the reverse ordering poisons every #[mz_ore::test] in the binary that has not run yet. Applied as a second commit, 29c58a8: init_logging_default now uses try_init() and keeps whatever subscriber is installed, with the doc comment updated to say so.

Verification with both commits, against a warm target dir:

• cargo test -p mz-ore --lib: 82 passed, 0 failed.
• cargo test -p mz-adapter --lib -- builtin_schema_migration in one process, ten consecutive runs: 10 of 10 passed (3 passed, 1 ignored each). Before the first commit this failed 2 of 4 tests; with only the first commit you measured 13 of 60 runs failing.
• cargo clippy -p mz-ore --tests -- -D warnings clean; cargo fmt clean.

Left as they are: the unwrapped set_global_default calls in src/cluster/src/communication.rs:711 (#[cfg(test)]) and src/service/tests/transport.rs:739. With this change mz_ore can no longer poison them; they can still panic themselves if mz_ore wins the race in a binary that mixes both, which neither currently does. Happy to fold them in if you would rather close the class in one PR.