Uh oh!
There was an error while loading. Please reload this page.
bootstrap: Unify test argument handling - #110576
Conversation
jyn514
commented
Apr 21, 2023
r? bootstrap |
jyn514
commented
Apr 21, 2023
I tried to fix #80124 but ran into #100599 (comment) so this is probably enough experimenting for tonight and I'm going to put it off for another day 😅 |
onur-ozkan
commented
Apr 22, 2023
@bors r+ rollup |
bors
commented
Apr 22, 2023
bootstrap: Unify test argument handling Fixesrust-lang#104198. Does *not* help with rust-lang#80124 because I couldn't figure out a reasonable way to omit `--lib` only for `panic_abort` and not other `std` dependencies. - Remove unnecessary `test_kind` field and `TestKind` struct. These are just subsets of the existing `builder.kind` / `Kind` struct. - Add a new `run_cargo_test` function which handles passing arguments to cargo based on `builder.config` - Switch all Steps in `mod test` to `run_cargo_test` where possible - Combine several steps into one `CrateBootstrap` step. These tests all do the same thing, just with different crate names. - Fix `x test --no-doc`. This is much simpler after the refactors mentioned earlier, but I'm happy to split it into a separate PR if desired. Before, this would panic a lot because steps forgot to pass `--lib`.
bootstrap: Unify test argument handling Fixesrust-lang#104198. Does *not* help with rust-lang#80124 because I couldn't figure out a reasonable way to omit `--lib` only for `panic_abort` and not other `std` dependencies. - Remove unnecessary `test_kind` field and `TestKind` struct. These are just subsets of the existing `builder.kind` / `Kind` struct. - Add a new `run_cargo_test` function which handles passing arguments to cargo based on `builder.config` - Switch all Steps in `mod test` to `run_cargo_test` where possible - Combine several steps into one `CrateBootstrap` step. These tests all do the same thing, just with different crate names. - Fix `x test --no-doc`. This is much simpler after the refactors mentioned earlier, but I'm happy to split it into a separate PR if desired. Before, this would panic a lot because steps forgot to pass `--lib`.
bootstrap: Unify test argument handling Fixesrust-lang#104198. Does *not* help with rust-lang#80124 because I couldn't figure out a reasonable way to omit `--lib` only for `panic_abort` and not other `std` dependencies. - Remove unnecessary `test_kind` field and `TestKind` struct. These are just subsets of the existing `builder.kind` / `Kind` struct. - Add a new `run_cargo_test` function which handles passing arguments to cargo based on `builder.config` - Switch all Steps in `mod test` to `run_cargo_test` where possible - Combine several steps into one `CrateBootstrap` step. These tests all do the same thing, just with different crate names. - Fix `x test --no-doc`. This is much simpler after the refactors mentioned earlier, but I'm happy to split it into a separate PR if desired. Before, this would panic a lot because steps forgot to pass `--lib`.
bootstrap: Unify test argument handling Fixesrust-lang#104198. Does *not* help with rust-lang#80124 because I couldn't figure out a reasonable way to omit `--lib` only for `panic_abort` and not other `std` dependencies. - Remove unnecessary `test_kind` field and `TestKind` struct. These are just subsets of the existing `builder.kind` / `Kind` struct. - Add a new `run_cargo_test` function which handles passing arguments to cargo based on `builder.config` - Switch all Steps in `mod test` to `run_cargo_test` where possible - Combine several steps into one `CrateBootstrap` step. These tests all do the same thing, just with different crate names. - Fix `x test --no-doc`. This is much simpler after the refactors mentioned earlier, but I'm happy to split it into a separate PR if desired. Before, this would panic a lot because steps forgot to pass `--lib`.
matthiaskrgr
commented
Apr 24, 2023
@bors rollup=iffy might cause hang in |
jyn514
commented
Apr 24, 2023
@matthiaskrgr I think these might just be spurious? I'm not sure how they could be related. |
bors
commented
Apr 24, 2023
⌛ Testing commit b3ee81a with merge c00fa943ad370da9d5b5fb5596e5abeab3101162... |
matthiaskrgr
commented
Apr 24, 2023
Hm, I've seen hangs on two rollups both on the same runner, and both included this pr which seemed a bit suspicious to me 😅🤷 |
bors
commented
Apr 24, 2023
💥 Test timed out |
jyn514
commented
Apr 27, 2023
Ok I take it back, the failure I saw locally was unrelated :/ rust-lang/cargo#12042 I am not sure how to investigate this further. I guess I can revert the change for cargo only? |
onur-ozkan
commented
Apr 27, 2023
I will also debug it tonight/early tomorrow. Let's see if I can find the reason |
onur-ozkan
commented
Apr 27, 2023
I have repreduced the same errors(mostly about network errors and command not found errors) on windows machine with the default Commits that are causing the errors:
-- I only had limited time, couldn't work on the fix. But would work on the fix at the weekend if you don't want to work on the fixes(probably multiple fixes needed for both commits) @jyn514 |
jyn514
commented
Apr 27, 2023
that's so strange I couldn't reproduce locally. I'll try again this weekend. Thanks for looking into it :) |
I still can't reproduce this :( @ozkanonur what version of windows are you on? are you using gnu or MSVC? what was the output from the failing tests? I'm on Windows 10 Pro 22H2 using MSVC. |
Windows 11 / MSVC I just run
That's weird, how can Windows 10 work and 11 can't??? To achieve logs, I had to ignore
-- Some of the OS err messages are in Turkish language, but I guess that shouldn't be problem because they are mostly duplicated messages and can be easily translated to English on google. |
jyn514
commented
Apr 29, 2023
Oh! I have a global rustc install and CI doesn't. If you run |
onur-ozkan
commented
Apr 29, 2023
It's fresh Windows 11 just has couple dependencies to build rustc. Just like CI, I also don't have rustc installed globally.
Yes |
- Use `cargo metadata` to determine whether a crate has a library package or not - Collect metadata for all workspaces, not just the root workspace and cargo - Don't pass `--lib` for crates without a library - Use `run_cargo_test` for rust-installer - Don't build documentation in `lint-docs` if `--no-doc` is passed
jyn514
commented
Apr 29, 2023
@bors r=ozkanonur rollup=iffy |
bors
commented
Apr 29, 2023
bors
commented
Apr 29, 2023
bors
commented
Apr 29, 2023
☀️ Test successful - checks-actions |
rust-timer
commented
Apr 30, 2023
Finished benchmarking commit (87b1f89): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. |
Fixes#104198. Does not help with #80124 because I couldn't figure out a reasonable way to omit
--libonly forpanic_abortand not otherstddependencies.test_kindfield andTestKindstruct. These are just subsets of the existingbuilder.kind/Kindstruct.run_cargo_testfunction which handles passing arguments to cargo based onbuilder.configmod testtorun_cargo_testwhere possibleCrateBootstrapstep. These tests all do the same thing, just with different crate names.x test --no-doc. This is much simpler after the refactors mentioned earlier, but I'm happy to split it into a separate PR if desired. Before, this would panic a lot because steps forgot to pass--lib.