Isolate secret-redaction tests from the shared SECRETS_REGEX global#14040
Isolate secret-redaction tests from the shared SECRETS_REGEX global#14040fbartho wants to merge 1 commit into
Conversation
SECRETS_REGEX is a single process-global Mutex<Arc<SecretsRegex>>, and each secret-redaction test overwrote it directly via set_user_and_enterprise_secret_regexes without any isolation. Under plain `cargo test` (parallel by default, and even sequentially without teardown), one test's regexes would leak into or get clobbered by another's before its assertion ran, causing nondeterministic `left: 0, right: 1` failures that rotated between runs. Add test_set_secrets_regex, an RAII guard (modeled on FeatureFlag::override_enabled) that snapshots the previous global value and restores it on drop. Since SECRETS_REGEX is process-wide rather than thread-local, the guard alone only prevents state from leaking across tests - it does not stop concurrent tests from interleaving mid-test. Pair it with #[serial(secrets_regex)] (a named serial_test group, so these tests don't needlessly serialize against unrelated #[serial] tests elsewhere in the crate) on every test that reads or writes the global. Closes warpdotdev#13973
github-actions
Bot
added
the
external-contributor
|
Every PR must be linked to a same-repo issue before Oz can review it. This PR is linked to #13973, but no linked issue is marked See the contribution guidelines for the full readiness model. Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Every PR must be linked to a same-repo issue before Oz can review it.
This PR is linked to #13973, but no linked issue is marked ready-to-implement yet. Only repository maintainers apply that label, so please wait for a maintainer to mark the issue. Once it is marked, push a new commit or comment /oz-review to re-trigger review.
See the contribution guidelines for the full readiness model.
Powered by Oz
1 participant
Closes #13973
Description
The secret-redaction tests install their own regexes into the single process-global
SECRETS_REGEX(secrets.rs) with no isolation. Under plaincargo test, one test's regexes could overwrite another's before its assertion ran — the loser panicsleft: 0, right: 1, and which test loses rotates run to run, which is why this reported as a rotating flake rather than one consistent failure.Fix:
test_set_secrets_regex()returns a#[must_use]RAII guard (modeled on the existingFeatureFlag::override_enabled→OverrideGuardpattern) that snapshots the global and restores it on drop. Since the global is process-wide (not thread-local), the guard prevents cross-test leakage but not mid-test interleaving — so the affected tests are also#[serial(secrets_regex)], a named group that avoids serializing against unrelated#[serial]tests. Converted all racing call sites in the three files from the issue:grid/secrets_tests.rs(9 tests),block/secret_redaction_tests.rs, andexecute/grep_tests.rs.Linked Issue
ready-to-specorready-to-implement.Testing
Repeated invocations of the affected modules under plain
cargo test— the harness where the flake manifests (nextest's per-process isolation masks it):Before:
cargo test -p warp --lib terminal::model::grid::grid_handler::secrets→FAILED. 3 passed; 7 failed(e.g.assertion left == right failed: left: 0, right: 1).After: same command ×3 →
ok. 10 passedevery run. The other two modules:ai::blocklist::block::secret_redaction→ok. 19 passed×3;…execute::grep→ok. 4 passed×3.Nextest (CI parity, was already green):
cargo nextest run -p warp --lib -E 'test(secrets) + test(secret_redaction) + test(grep_tests)'→ 72/72 passed.cargo clippy -p warp --lib --testsandcargo check -p warp --lib --features test-util: zero warnings.I have manually tested my changes locally with
./script/runScreenshots / Videos
Not applicable: test-only change.
Agent Mode