test(build-std): Isolate output test to avoid spurious [BLOCKING] messages from concurrent runs
#14943
+37
−12
Conversation
This file contains 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
rustbot
added
the
S-waiting-on-review
epage
changed the title
[BLOCKING] messages from concurrent runs
epage
reviewed
Dec 17, 2024
tests/build-std/main.rs
Outdated
Comment on lines
45
to
63
| fn build_std(&mut self) -> &mut Self; | ||
| fn build_std_arg(&mut self, arg: &str) -> &mut Self; | ||
| fn build_std_isolated(&mut self) -> &mut Self; |
There was a problem hiding this comment.
Can we document when to use each?
epage
reviewed
Dec 17, 2024
tests/build-std/main.rs
Outdated
Comment on lines
118
to
120
| // An isolated CARGO_HOME environment is used elesewhere in this test | ||
| // to ensure no extra index updates is performed. | ||
| // It is achieve by asserting the complete stderr without any wildcard. |
There was a problem hiding this comment.
Suggested change
| // An isolated CARGO_HOME environment is used elesewhere in this test | |
| // to ensure no extra index updates is performed. | |
| // It is achieve by asserting the complete stderr without any wildcard. | |
| // 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 |
weihanglo
force-pushed
the
build-std-test
branch
2 times, most recently
from
4159d5b to
9fedef7
Compare
…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.
weihanglo
force-pushed
the
build-std-test
branch
from
9fedef7 to
ca59614
Compare
epage
approved these changes
Dec 17, 2024
github-merge-queue bot
pushed a commit
that referenced
this pull request
Dec 18, 2024
### What does this PR try to resolve? Blocked on <#14943> (or can just merge this one). Fixes #14935 #14935 failed because since 125e873 [`std_resolve`][1] only includes `sysroot` as primary package. When any custom Cargo feature is provided via `-Zbuild-std-feature`, the default feature set `panic-unwind` would be gone, so no `panic_unwind` crate presents in `std_resolve`. When then calling [`std_resolve.query`][2] with the default set of crates from [`std_crates`][3], which automatically includes `panic_unwind` when `std` presents, it'll result in spec not found because `panic_unwind` was not in `std_resolve` anyway. [1]: https://github.com/rust-lang/cargo/blob/addcc8ca715bc7fe20df66afd6efbf3c77ef43f8/src/cargo/core/compiler/standard_lib.rs#L96 [2]: https://github.com/rust-lang/cargo/blob/addcc8ca715bc7fe20df66afd6efbf3c77ef43f8/src/cargo/core/compiler/standard_lib.rs#L158 [3]: https://github.com/rust-lang/cargo/blob/addcc8ca715bc7fe20df66afd6efbf3c77ef43f8/src/cargo/core/compiler/standard_lib.rs#L156 ### How should we test and review this PR? This patch is kinda a revert of 125e873 in terms of the behavior. With this, now `std_resolve` is always resolved to the same set of packages that Cargo will use to generate the unit graph, (technically the same set of crates + `sysroot`), by sharing the same set of primary packages via `std_crates` functions. Note that when multiple `--target`s provided, if std is specified or there is one might support std, Cargo will always resolve std dep graph. To test it manually, run ``` RUSTFLAGS="-C panic=abort" cargo +nightly-2024-12-15 b -Zbuild-std=std,panic_abort -Zbuild-std-features=panic_immediate_abort ``` change to this PR's cargo with the same nightly rustc, it would succeed. I am a bit reluctant to add an new end-2end build-std test, but I still did it. A bit scared when mock-std gets out-of-sync of features in std in rust-lang/rust.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Dec 18, 2024
Update cargo 6 commits in 769f622e12db0001431d8ae36d1093fb8727c5d9..99dff6d77db779716dda9ca3b29c26addd02c1be 2024-12-14 04:27:35 +0000 to 2024-12-18 00:55:17 +0000 - fix(build-std): make Resolve align to what to build (rust-lang/cargo#14938) - test(build-std): Isolate output test to avoid spurious `[BLOCKING]` messages from concurrent runs (rust-lang/cargo#14943) - docs: fix wrong changelog PR link (rust-lang/cargo#14947) - docs(unstable): Correct stabilization version for MSRV-resolver (rust-lang/cargo#14945) - Update release information for home 0.5.11 (rust-lang/cargo#14939) - Limit release trigger to 0.* tags (rust-lang/cargo#14940)
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.
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_HOMEfor 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-supportCARGO_HOME, rather than the usersCARGO_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: