Skip to content

Commit

Permalink
test(build-std): Isolate output test to avoid spurious [BLOCKING] mes…
Browse files Browse the repository at this point in the history
…sages from concurrent runs

47c2095 didn't really fix the flakiness.

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.
  • Loading branch information
weihanglo committed Dec 17, 2024
1 parent addcc8c commit 4159d5b
Showing 1 changed file with 26 additions and 12 deletions.
38 changes: 26 additions & 12 deletions tests/build-std/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@ use cargo_test_support::{basic_manifest, paths, project, rustc_host, str, Execs}
use std::env;
use std::path::Path;

fn enable_build_std(e: &mut Execs, arg: Option<&str>) {
e.env_remove("CARGO_HOME");
e.env_remove("HOME");
fn enable_build_std(e: &mut Execs, arg: Option<&str>, isolated: bool) {
if !isolated {
e.env_remove("CARGO_HOME");
e.env_remove("HOME");
}

// And finally actually enable `build-std` for now
let arg = match arg {
Expand All @@ -42,17 +44,28 @@ fn enable_build_std(e: &mut Execs, arg: Option<&str>) {
trait BuildStd: Sized {
fn build_std(&mut self) -> &mut Self;
fn build_std_arg(&mut self, arg: &str) -> &mut Self;

/// Use an isolated `CARGO_HOME` environment to avoid package cache lock contention.
///
/// Don't use this unless you really need to assert the full stderr
/// to avoid `[BLOCKING]` messages.
fn build_std_isolated(&mut self) -> &mut Self;
fn target_host(&mut self) -> &mut Self;
}

impl BuildStd for Execs {
fn build_std(&mut self) -> &mut Self {
enable_build_std(self, None);
enable_build_std(self, None, false);
self
}

fn build_std_arg(&mut self, arg: &str) -> &mut Self {
enable_build_std(self, Some(arg));
enable_build_std(self, Some(arg), false);
self
}

fn build_std_isolated(&mut self) -> &mut Self {
enable_build_std(self, None, true);
self
}

Expand Down Expand Up @@ -107,9 +120,12 @@ fn basic() {
)
.build();

p.cargo("check").build_std().target_host().run();
// HACK: use an isolated the isolated CARGO_HOME environment (`build_std_isolated`)
// to avoid `[BLOCKING]` messages (from lock contention with other tests)
// from getting in this test's asserts
p.cargo("check").build_std_isolated().target_host().run();
p.cargo("build")
.build_std()
.build_std_isolated()
.target_host()
// Importantly, this should not say [UPDATING]
// There have been multiple bugs where every build triggers and update.
Expand All @@ -120,7 +136,7 @@ fn basic() {
"#]])
.run();
p.cargo("run")
.build_std()
.build_std_isolated()
.target_host()
.with_stderr_data(str![[r#"
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
Expand All @@ -129,7 +145,7 @@ fn basic() {
"#]])
.run();
p.cargo("test")
.build_std()
.build_std_isolated()
.target_host()
.with_stderr_data(str![[r#"
[COMPILING] rustc-std-workspace-std [..]
Expand Down Expand Up @@ -379,13 +395,11 @@ fn test_proc_macro() {
.file("src/lib.rs", "")
.build();

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

0 comments on commit 4159d5b

Please sign in to comment.