EB-1: fix flaky apply_env tests racing on shared EB_* env vars #35

Merged

David merged 1 commit from fix/apply-env-test-pollution-EB-1 into main

2026-06-07 17:11:33 +02:00

David commented

2026-06-07 16:41:03 +02:00

Owner

Summary

Fixes the intermittent check.yml failure (EB-1, run #87) where tests::apply_env_prefix_updates_pattern_and_filename asserted left "My.Prefix+1" != right "MyPrefix" at src/main.rs:1431.

Root cause

The four apply_env_* unit tests mutate the process-global EB_* environment variables via env::set_var. Rust runs unit tests in parallel by default, so when apply_env_prefix_updates_pattern_and_filename (writes EB_SAVE_FILENAME_PREFIX=MyPrefix) and apply_env_prefix_with_special_chars_escapes_in_pattern (writes EB_SAVE_FILENAME_PREFIX=My.Prefix+1) run concurrently, they race on the same variable. The first test then reads the second test's value, producing the observed assertion failure. This is test pollution, not a defect in the prefix-sanitization logic itself.

Fix

Guard the four apply_env tests with a shared ENV_MUTEX so no two run concurrently. Mutex poisoning is ignored via into_inner() so a genuine test panic surfaces as its own failure rather than cascading into the others. The RangeConfig::set_from_env tests already use unique per-test variable names and do not touch shared state, so they need no lock.

Note

The commit also syncs Cargo.lock (explorer-bookmarks 0.3.3 -> 0.4.0) to match the version already declared in Cargo.toml; cargo regenerated it during the build. No dependency changes.

Verification

cargo fmt --check clean
cargo clippy --all-targets -- -D warnings clean (matches CI)
cargo test run 10x consecutively: 36 passed, 0 failed each time

#EB-1

## Summary Fixes the intermittent `check.yml` failure (EB-1, run #87) where `tests::apply_env_prefix_updates_pattern_and_filename` asserted `left "My.Prefix+1" != right "MyPrefix"` at `src/main.rs:1431`. ## Root cause The four `apply_env_*` unit tests mutate the process-global `EB_*` environment variables via `env::set_var`. Rust runs unit tests in parallel by default, so when `apply_env_prefix_updates_pattern_and_filename` (writes `EB_SAVE_FILENAME_PREFIX=MyPrefix`) and `apply_env_prefix_with_special_chars_escapes_in_pattern` (writes `EB_SAVE_FILENAME_PREFIX=My.Prefix+1`) run concurrently, they race on the same variable. The first test then reads the second test's value, producing the observed assertion failure. This is test pollution, not a defect in the prefix-sanitization logic itself. ## Fix Guard the four `apply_env` tests with a shared `ENV_MUTEX` so no two run concurrently. Mutex poisoning is ignored via `into_inner()` so a genuine test panic surfaces as its own failure rather than cascading into the others. The `RangeConfig::set_from_env` tests already use unique per-test variable names and do not touch shared state, so they need no lock. ## Note The commit also syncs `Cargo.lock` (`explorer-bookmarks` 0.3.3 -> 0.4.0) to match the version already declared in `Cargo.toml`; `cargo` regenerated it during the build. No dependency changes. ## Verification - `cargo fmt --check` clean - `cargo clippy --all-targets -- -D warnings` clean (matches CI) - `cargo test` run 10x consecutively: 36 passed, 0 failed each time #EB-1

David added 1 commit

2026-06-07 16:41:03 +02:00

fix(tests): serialize env-mutating apply_env tests

Check / fmt + clippy + build + tests (pull_request) Successful in 9s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

7b674f5ed1

The four apply_env unit tests mutate process-global EB_* environment variables via env::set_var. Rust runs unit tests in parallel by default, so when apply_env_prefix_updates_pattern_and_filename (writes EB_SAVE_FILENAME_PREFIX=MyPrefix) and apply_env_prefix_with_special_chars_escapes_in_pattern (writes EB_SAVE_FILENAME_PREFIX=My.Prefix+1) run concurrently they race on the same variable. The first test then reads the second test's value, producing the intermittent CI failure in check.yml run #87 (assertion left "My.Prefix+1" != right "MyPrefix" at src/main.rs:1431).

Guard the four apply_env tests with a shared ENV_MUTEX so no two run concurrently. Poisoning is ignored (into_inner) so a genuine test panic surfaces as its own failure rather than cascading. The RangeConfig set_from_env tests already use unique per-test variable names and do not need the lock.

#EB-1