Can #[serial] usage assume test threads in a process are for the same crate?

[EliahKagan]

How parallelism is implemented for testing, and when and how it uses multiple threads vs. multiple processes, varies across test runners, with nextest making more use of multiple processes. But I am not clear on what guarantees exist as part of the semantics of tests, nor on what assumptions are acceptable to make about how parallelism is achieved when running this project's test suite.

Current use (and non-use) of #[serial] seems to assume that a test process that has multiple tests running in it on different threads has only tests for the same crate. When I added more tests in #1567 that use serial, I also made this assumption.

This is to say that changing the current working directory and, in some cases, environment variables, seems to be well-accounted for with #[serial], in terms of tests in the same crate that could interfere with one another, but not accounted for if tests across separate crates could run on separate threads in the same process.

This is because, if I understand the documentation correctly:

• #[serial] only guarantees blocking to prevent running in the same process at the same time as another test that is also marked #[serial], or as tests that are marked #[parallel] to allow them to run in parallel but not at the same time on the same thread as a #[serial] test.
• Many tests use #[serial] to avoid data races while modifying global state used by code that is tested by various other tests in other crates, and #[parallel] is only used in gix/tests/repository/shallow.rs.

cargo test and cargo nextest seem to set the current working directory to the root of the crate whenever --workspace/--all is used, which would be hard and probably impossible to do correctly while running tests from separate threads in the same process. But I'm not sure if this is a guaranteed behavior.

My direct motivation for asking about this is noted in #1578 (see also #1580), but I am curious about in it general, and also to make sure I didn't introduce data races in the tests suite in #1567.

[Byron]

That's an interesting question, I never thought about it and wasn't aware that nextest may bundle up tests from different crates into the same process.
The only reason I see where this could be an issue is if the CWD of the process is changed.
I believe this is only done when the test is singled-out into its own binary by making it its own standalone file in tests/<standalone>.rs. As long as nextest wouldn't bundle multiple of these up in the same process, it should be fine.

The fact that it's not all breaking already probably tells us that nextest is doing the right thing.

[EliahKagan]

and wasn't aware that nextest may bundle up tests from different crates into the same process.

Although I am not certain, I believe cargo nextest will never do that. As I understand it, cargo nextest has a greater tendency than cargo test to use multiple processes to achieve parallelism. My concern is more that cargo test might do so and, more broadly, that the semantics of the test suite might permit some future testing tool to do it while still following all the rules.

The only reason I see where this could be an issue is if the CWD of the process is changed.

It seems to me some changes to environment variables could also be relevant in far-reaching ways.

I believe this is only done when the test is singled-out into its own binary by making it its own standalone file in tests/<standalone>.rs.

I change directory and also set environment variables in some tests introduced in #1567 (and, building on that, a little bit more in #1583), which is one of my concerns.

The fact that it's not all breaking already probably tells us that nextest is doing the right thing.

Yes, and the move to using nextest more (for the test-fast CI jobs) convinces me that things are probably fine with it. My concern is with what happens in runners other than nextest, and mainly cargo test since that seems to be the only other significant runner, and because cargo test is now being used less on CI (though we were using it a lot before and everything seems to have gone fine).

[Byron]

It seems to me some changes to environment variables could also be relevant in far-reaching ways.

That's true, even though I was under the assumption that tests without #[serial] will not be depending on the environment, which means they can run in parallel to #[serial] tests which do.
The same is not true for #[serial] tests which change the CWD.

Yes, and the move to using nextest more (for the test-fast CI jobs) convinces me that things are probably fine with it. My concern is with what happens in runners other than nextest, and mainly cargo test since that seems to be the only other significant runner, and because cargo test is now being used less on CI (though we were using it a lot before and everything seems to have gone fine).

It's true that while before we were implicitly relying on possibly undocumented behaviour of cargo test, now the same is true for cargo nextest, which seems to be used exclusively now, except for doc-tests which are special.

To my mind, that's acceptable as in theory, there is no need to switch back while cargo nextest does such a good job, it also looks better. And since cargo test might be used by other developers, I'd think regressions won't be invisible for long, which opens them up for fixes that should always be possible.

[EliahKagan]

That's true, even though I was under the assumption that tests without #[serial] will not be depending on the environment

I think we avoid running tests that directly depend on values of environment variables that we know we are changing while we are changing them (or while they have their temporary strange values). But on a Unix-like system, any change to the environment of any kind makes concurrently reading any environment variable unsafe, and unfortunately, as noted here:

The problem is that these operating systems do not provide a thread-safe way to read the environment, and most C libraries, including libc itself, do not advertise which functions read from the environment.

(By "these operating systems" it means systems other than Windows. On Windows, accesses to the environment are synchronized.)

So that's the kind of problem I'm most worried about.

And since cargo test might be used by other developers, I'd think regressions won't be invisible for long, which opens them up for fixes that should always be possible.

I largely agree with this, since I think using cargo test instead than cargo nextest is correlated with being unable to build or install nextest, which itself happens on less used targets, which are also where concurrency problems would most likely be observed if they exist but have not yet been observed on more commonly used targets.

In any case, it's the test suite and not the code under test that would have the concurrency problems. So most likely nothing very bad will happen. If I learn more about what guarantees exist for cargo nextest and cargo test specifically, I'll provide the updated information.