Add environment variable mutexes for test isolation#3
Open
Conversation
Adds poison-recovery helpers to prevent cascading test failures when a test panics while holding a mutex lock. Changes: - Add lock_test_mutex() helper functions to all modules with test mutexes - Replace all test_mutex().lock().unwrap() calls with lock_test_mutex() - Automatically recover poisoned mutexes using unwrap_or_else Impact: - Reduced parallel test failures significantly - All 207 tests pass when run sequentially (--test-threads=1) - Remaining parallel failures are due to other isolation issues (PATH, temp files) that require per-test refactoring Modules updated: - src/index/mod.rs - src/logic/mod.rs - src/state/mod.rs - src/sources/mod.rs - src/theme/mod.rs - All test files using test mutexes
Fixes race conditions from parallel tests modifying PATH and HOME environment variables. Changes: - Created src/test_utils.rs with global PATH and HOME mutexes - Added lock_path_mutex() helper for PATH-modifying tests - Added lock_home_mutex() helper for HOME-modifying tests - Updated 23 test files to use PATH mutex - Updated 6 test files to use HOME mutex Results: Before: 17-20 test failures in parallel After: 14 test failures in parallel Improvement: ~20-30% reduction in parallel test failures All 207 tests still pass when run sequentially (--test-threads=1) Remaining Issues: The 14 remaining parallel failures are due to: - Tests sharing global index singleton state - Tests modifying the same disk cache paths - These require deeper refactoring beyond environment isolation Files Updated: - src/test_utils.rs (new file) - src/lib.rs (adds test_utils module) - src/index/*.rs (5 files with PATH guards) - src/logic/**/*.rs (3 files with PATH guards) - src/sources/*.rs (2 files with PATH guards) - src/install/*.rs (5 files with PATH guards) - src/events/mod.rs (PATH guards) - src/state/app_state.rs (HOME guards) - src/theme/**/*.rs (3 files with HOME guards)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds global mutexes for PATH and HOME environment variables to prevent race conditions in parallel test execution.
Problem
Tests that modify environment variables using
unsafe { std::env::set_var }affect the entire process. Even with per-module test mutexes, these modifications can race when tests run in parallel, causing non-deterministic failures.Solution
Created
src/test_utils.rswith global environment variable mutexes:lock_path_mutex()- Serializes all PATH modificationslock_home_mutex()- Serializes all HOME modificationsUpdated 29 test files across the codebase to acquire these mutexes before modifying environment variables.
Results
Test Failure Reduction:
Sequential Tests:
Remaining 14 Failures
The remaining parallel test failures are due to deeper architectural issues:
These require significant refactoring:
Files Changed
New:
src/test_utils.rs- Global environment variable mutexesUpdated (29 files):
src/lib.rs- Adds test_utils modulesrc/index/*.rs- 5 files with PATH guardssrc/logic/**/*.rs- 3 files with PATH guardssrc/sources/*.rs- 2 files with PATH guardssrc/install/*.rs- 5 files with PATH guardssrc/events/mod.rs- PATH guardssrc/state/app_state.rs- HOME guardssrc/theme/**/*.rs- 3 files with HOME guardsDependencies
This PR builds on top of:
Testing