Skip to content

Commit 8682bb4

Browse files
authored
test(build-std): Isolate output test to avoid spurious [BLOCKING] messages from concurrent runs (#14943)
### What does this PR try to resolve? 47c2095 didn't really fix the flakiness. Spun off from <#14938> 2a54190 build-std tests use the users `CARGO_HOME` for downloading registry dependencies of the standard library. This reduces disk needs of the tests, speeds up the tests, and reduces the number of network requests that could fail. However, this means all of the tests access the same locks for the package cache. In one test, we assert on the output and a `[BLOCKING]` message can show up, depending on test execution time from concurrent test runs. We are going to hack around this by having the one test that asserts on test output to use the standard `cargo-test-support` `CARGO_HOME`, rather than the users `CARGO_HOME`. There will then only be one process accessing the lock and no `[BLOCKING]` messages. ### How should we test and review this PR? No more assertion errors like this: ``` ---- test_proc_macro stdout ---- running `/home/runner/work/cargo/cargo/target/debug/cargo fetch -Zbuild-std -Zpublic-dependency` running `/home/runner/work/cargo/cargo/target/debug/cargo test --lib -Zbuild-std -Zpublic-dependency` thread 'test_proc_macro' panicked at tests/build-std/main.rs:394:10: ---- expected: tests/build-std/main.rs:388:27 ++++ actual: stderr 1 + [BLOCKING] waiting for file lock on package cache error: test failed, to rerun pass `-p cargo --test build-std` 2 + [BLOCKING] waiting for file lock on package cache 1 3 | [COMPILING] foo v0.0.0 ([ROOT]/foo) 2 4 | [FINISHED] `test` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s 3 5 | [RUNNING] unittests src/lib.rs (target/debug/deps/foo-[HASH]) ```
2 parents b9026bf + ca59614 commit 8682bb4

File tree

1 file changed

+37
-12
lines changed

1 file changed

+37
-12
lines changed

tests/build-std/main.rs

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,11 @@ use cargo_test_support::{basic_manifest, paths, project, rustc_host, str, Execs}
2525
use std::env;
2626
use std::path::Path;
2727

28-
fn enable_build_std(e: &mut Execs, arg: Option<&str>) {
29-
e.env_remove("CARGO_HOME");
30-
e.env_remove("HOME");
28+
fn enable_build_std(e: &mut Execs, arg: Option<&str>, isolated: bool) {
29+
if !isolated {
30+
e.env_remove("CARGO_HOME");
31+
e.env_remove("HOME");
32+
}
3133

3234
// And finally actually enable `build-std` for now
3335
let arg = match arg {
@@ -40,19 +42,41 @@ fn enable_build_std(e: &mut Execs, arg: Option<&str>) {
4042

4143
// Helper methods used in the tests below
4244
trait BuildStd: Sized {
45+
/// Set `-Zbuild-std` args and will download dependencies of the standard
46+
/// library in users's `CARGO_HOME` (`~/.cargo/`) instead of isolated
47+
/// environment `cargo-test-support` usually provides.
48+
///
49+
/// The environment is not isolated is to avoid excessive network requests
50+
/// and downloads. A side effect is `[BLOCKING]` will show up in stderr,
51+
/// as a sign of package cahce lock contention when running other build-std
52+
/// tests concurrently.
4353
fn build_std(&mut self) -> &mut Self;
54+
55+
/// Like [`BuildStd::build_std`] and is able to specify what crates to build.
4456
fn build_std_arg(&mut self, arg: &str) -> &mut Self;
57+
58+
/// Like [`BuildStd::build_std`] but use an isolated `CARGO_HOME` environment
59+
/// to avoid package cache lock contention.
60+
///
61+
/// Don't use this unless you really need to assert the full stderr
62+
/// and avoid any `[BLOCKING]` message.
63+
fn build_std_isolated(&mut self) -> &mut Self;
4564
fn target_host(&mut self) -> &mut Self;
4665
}
4766

4867
impl BuildStd for Execs {
4968
fn build_std(&mut self) -> &mut Self {
50-
enable_build_std(self, None);
69+
enable_build_std(self, None, false);
5170
self
5271
}
5372

5473
fn build_std_arg(&mut self, arg: &str) -> &mut Self {
55-
enable_build_std(self, Some(arg));
74+
enable_build_std(self, Some(arg), false);
75+
self
76+
}
77+
78+
fn build_std_isolated(&mut self) -> &mut Self {
79+
enable_build_std(self, None, true);
5680
self
5781
}
5882

@@ -107,9 +131,12 @@ fn basic() {
107131
)
108132
.build();
109133

110-
p.cargo("check").build_std().target_host().run();
134+
// HACK: use an isolated the isolated CARGO_HOME environment (`build_std_isolated`)
135+
// to avoid `[BLOCKING]` messages (from lock contention with other tests)
136+
// from getting in this test's asserts
137+
p.cargo("check").build_std_isolated().target_host().run();
111138
p.cargo("build")
112-
.build_std()
139+
.build_std_isolated()
113140
.target_host()
114141
// Importantly, this should not say [UPDATING]
115142
// There have been multiple bugs where every build triggers and update.
@@ -120,7 +147,7 @@ fn basic() {
120147
"#]])
121148
.run();
122149
p.cargo("run")
123-
.build_std()
150+
.build_std_isolated()
124151
.target_host()
125152
.with_stderr_data(str![[r#"
126153
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
@@ -129,7 +156,7 @@ fn basic() {
129156
"#]])
130157
.run();
131158
p.cargo("test")
132-
.build_std()
159+
.build_std_isolated()
133160
.target_host()
134161
.with_stderr_data(str![[r#"
135162
[COMPILING] rustc-std-workspace-std [..]
@@ -379,13 +406,11 @@ fn test_proc_macro() {
379406
.file("src/lib.rs", "")
380407
.build();
381408

382-
// Download dependencies first,
383-
// so we can compare `cargo test` output without any wildcard
384-
p.cargo("fetch").build_std().run();
385409
p.cargo("test --lib")
386410
.env_remove(cargo_util::paths::dylib_path_envvar())
387411
.build_std()
388412
.with_stderr_data(str![[r#"
413+
...
389414
[COMPILING] foo v0.0.0 ([ROOT]/foo)
390415
[FINISHED] `test` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
391416
[RUNNING] unittests src/lib.rs (target/debug/deps/foo-[HASH])

0 commit comments

Comments
 (0)