Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run tests in processes rather than threads with cargo nextest #2527

Closed
SteveLauC opened this issue Oct 25, 2024 · 2 comments
Closed

Run tests in processes rather than threads with cargo nextest #2527

SteveLauC opened this issue Oct 25, 2024 · 2 comments

Comments

@SteveLauC
Copy link
Member

Some tests modify process-wide stuff like signal handler, so we use these locks

nix/test/test.rs

Lines 63 to 80 in 70f8fe0

/// Any test that creates child processes or can be affected by child processes
/// must grab this mutex, regardless of what it does with those children.
///
/// It must hold the mutex until the child processes are waited upon.
pub static FORK_MTX: Mutex<()> = Mutex::new(());
/// Any test that changes the process's current working directory must grab
/// the RwLock exclusively. Any process that cares about the current
/// working directory must grab it shared.
pub static CWD_LOCK: RwLock<()> = RwLock::new(());
/// Any test that changes the process's supplementary groups must grab this
/// mutex
pub static GROUPS_MTX: Mutex<()> = Mutex::new(());
/// Any tests that loads or unloads kernel modules must grab this mutex
pub static KMOD_MTX: Mutex<()> = Mutex::new(());
/// Any test that calls ptsname(3) must grab this mutex.
pub static PTSNAME_MTX: Mutex<()> = Mutex::new(());
/// Any test that alters signal handling must grab this mutex.
pub static SIGNAL_MTX: Mutex<()> = Mutex::new(());

to sequentialize them, TIL that cargo nextest will run tests in processes rather than threads, perhaps this can help us get rid of these locks and potentially fixes some weird test failures.

@asomers
Copy link
Member

asomers commented Nov 15, 2024

cargo-nextest is cool, but I don't think that we should rely on it. Even if we use it during CI, we should also ensure that "cargo test" works too.

@SteveLauC
Copy link
Member Author

we should also ensure that "cargo test" works too.

Makes sense, closing the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants