Skip to content

fix(test): serialize tests that mutate CORTEX_MAX_TOPOLOGY_BYTES#5

Merged
ammar-s847 merged 1 commit into
mainfrom
fix-disk-test-race
May 26, 2026
Merged

fix(test): serialize tests that mutate CORTEX_MAX_TOPOLOGY_BYTES#5
ammar-s847 merged 1 commit into
mainfrom
fix-disk-test-race

Conversation

@ammar-s847
Copy link
Copy Markdown
Contributor

Summary

CI failed on disk::tests::topology_round_trips_on_disk with:

thread 'disk::tests::topology_round_trips_on_disk' panicked at src/disk/mod.rs:395:41:
called `Result::unwrap()` on an `Err` value:
  TopologyTooLarge { path: ..., bytes: 209, limit: 16 }

Root cause

topology_size_limit_enforced mutates process-global state — it sets CORTEX_MAX_TOPOLOGY_BYTES=16 to force the size guard to fire, then unsets it. cargo test runs tests in parallel by default, so when topology_round_trips_on_disk happens to call read_topology during that window it observes the 16-byte limit and trips the guard against the real ~209-byte topology file.

Local runs happened to schedule the tests in an order that avoided the race; fresh CI hit it.

Fix

Module-level Mutex<()> that both env-touching tests acquire before reading or writing the env var. Lock is poison-safe via unwrap_or_else(|e| e.into_inner()) so a panic in one test can't permanently wedge the rest.

No production code change — the env var contract and the production read path are unchanged.

Test plan

  • cargo test -p hebb --features disk — green
  • Ran the disk tests 5× in a loop — both ordering combinations pass
  • Wait for CI green

`topology_size_limit_enforced` sets the env var to 16 to force the
size guard to fire, then unsets it. `cargo test` runs tests in
parallel by default, so a sibling test (`topology_round_trips_on_disk`)
that calls `read_topology` while the limit is in effect spuriously
fails with `TopologyTooLarge { bytes: 209, limit: 16 }`. Local runs
happened to schedule the tests in an order that avoided the race;
fresh CI hit it.

Fix is a module-level `Mutex` that both env-touching tests acquire.
The lock is poison-safe via `unwrap_or_else(|e| e.into_inner())` so
one test panicking can't permanently wedge the suite.
@ammar-s847 ammar-s847 merged commit 53a4a39 into main May 26, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant