Rollup merge of rust-lang#153143 - ferrocene:jh/bootstrap-test-targets, r=Mark-Simulacrum,kobzol

Allow `./x test` to run tests without doc tests and without benchmarks

# Problem

For Ferrocene we would like to run only the `coretests` and `alloctests` test suites when measuring code coverage. Running `corebenches` and `allocbenches` would alter the numbers, which is not compliant with the certification.

This is currently not possible in bootstrap. By default `./x test` runs unit, integration and doc tests. `./x test --doc` only runs doc tests. So far, so good.

The problem is that while `./x test --no-doc` stops bootstrap from executing doc tests, it surprisingly starts executing benchmarks (next to examples and bins as well). It basically behaves like `cargo test --all-targets`.

# Solution

This PR renames the existing `--no-doc` flag to `--all-targets` and keeps the same behaviour as before. Unfortunately it is not possible to internally switch from `cargo --bins --examples --tests --benches` to `cargo --all-targets` because it will fail e.g. `./x test library/alloc --all-targets` with an error like "use of unstable library feature `test`".

Additionally, this PR add a `./x test --tests` flag (equivalent to `cargo test --tests`) that only executes unit and integration tests.

Both changes we are doing in https://github.com/ferrocene/ferrocene anyways, but believe that they may be useful for other people as well and therefore would like to contribute them.

Note that this PR does _not_ change the behaviour of either `./x test` or `./x test --doc`.

## Note on `test = true`

While this change enables bootstrap to run tests without doc tests and without benchmarks, executing `./x test library/core library/alloc --tests` will still build and execute `corebenches` and `allocbenches`.

What?! 😱 Why all of this effort to enable it then?

Good question! The reason they are still executed is, that they are marked with `test = true` in their respective `Cargo.toml` ([corebenches](https://github.com/rust-lang/rust/blob/3f9853562c73af38a5e6af8b0da1b2734a327e19/library/coretests/Cargo.toml#L24), [allocbenches](https://github.com/rust-lang/rust/blob/3f9853562c73af38a5e6af8b0da1b2734a327e19/library/alloctests/Cargo.toml#L32)).

@bjorn3 mentioned that it is important for upstream that those benchmarks are executed by default, even if no `./x --all-targets` is passed. This is perfectly possible with this PR. Benchmarks marked with `test = true` will be executed when calling either of `./x test`, `./x test --tests` or `./x --all-targets`. Therefore this PR does not include a commit to change that. We will just do this change in https://github.com/ferrocene/ferrocene.

## Open questions

### Error message

I added one commit that adds an error message if a user passes `--no-doc` and points them to `--all-targets` and `--tests`. I think it might be nice to have, but if you think it is not necessary, I can remove it.

# How to test this change

You can see the change in action by running `./x test library/alloc --tests` and `./x test library/alloc --all-targets`. The first will execute `alloctests` and `allocbenches` (which is marked with `test = true`), while the second will additionally run `benches/vec_deque_append.rs` (which is _not_ marked with `test = true`).

Author: JonathanBrouwer

library/alloctests/benches/vec_deque_append.rs

@@ -8,7 +8,7 @@ const BENCH_N: usize = 1000;
 fn main() {
     if cfg!(miri) {
         // Don't benchmark Miri...
-        // (Due to bootstrap quirks, this gets picked up by `x.py miri library/alloc --no-doc`.)
+        // (Due to bootstrap quirks, this gets picked up by `x.py miri library/alloc --all-targets`.)
         return;
     }
     let a: VecDeque<i32> = (0..VECDEQUE_LEN).collect();

src/bootstrap/mk/Makefile.in

@@ -64,7 +64,7 @@ check-aux:
 		library/core \
 		library/alloc \
 		$(BOOTSTRAP_ARGS) \
-		--no-doc
+		--all-targets
 	# Some doctests use file system operations to demonstrate dealing with `Result`,
 	# so we have to run them with isolation disabled.
 	$(Q)MIRIFLAGS="-Zmiri-disable-isolation" \

src/bootstrap/src/core/build_steps/test.rs

@@ -41,7 +41,7 @@ use crate::utils::helpers::{
     linker_args, linker_flags, t, target_supports_cranelift_backend, up_to_date,
 };
 use crate::utils::render_tests::{add_flags_and_try_run_tests, try_run_tests};
-use crate::{CLang, CodegenBackendKind, DocTests, GitRepo, Mode, PathSet, envify};
+use crate::{CLang, CodegenBackendKind, GitRepo, Mode, PathSet, TestTarget, envify};
 
 mod compiletest;
 
@@ -174,7 +174,7 @@ You can skip linkcheck with --skip src/tools/linkchecker"
         );
         run_cargo_test(cargo, &[], &[], "linkchecker self tests", bootstrap_host, builder);
 
-        if builder.doc_tests == DocTests::No {
+        if !builder.test_target.runs_doctests() {
             return;
         }
 
@@ -827,15 +827,14 @@ impl Step for CargoMiri {
 
         // We're not using `prepare_cargo_test` so we have to do this ourselves.
         // (We're not using that as the test-cargo-miri crate is not known to bootstrap.)
-        match builder.doc_tests {
-            DocTests::Yes => {}
-            DocTests::No => {
-                cargo.args(["--lib", "--bins", "--examples", "--tests", "--benches"]);
+        match builder.test_target {
+            TestTarget::AllTargets => {
+                cargo.args(["--lib", "--bins", "--examples", "--tests", "--benches"])
             }
-            DocTests::Only => {
-                cargo.arg("--doc");
-            }
-        }
+            TestTarget::Default => &mut cargo,
+            TestTarget::DocOnly => cargo.arg("--doc"),
+            TestTarget::Tests => cargo.arg("--tests"),
+        };
         cargo.arg("--").args(builder.config.test_args());
 
         // Finally, run everything.
@@ -1213,7 +1212,7 @@ impl Step for RustdocGUI {
 
     fn is_default_step(builder: &Builder<'_>) -> bool {
         builder.config.nodejs.is_some()
-            && builder.doc_tests != DocTests::Only
+            && builder.test_target != TestTarget::DocOnly
             && get_browser_ui_test_version(builder).is_some()
     }
 
@@ -1303,7 +1302,7 @@ impl Step for Tidy {
     }
 
     fn is_default_step(builder: &Builder<'_>) -> bool {
-        builder.doc_tests != DocTests::Only
+        builder.test_target != TestTarget::DocOnly
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -1862,7 +1861,7 @@ impl Step for Compiletest {
     }
 
     fn run(self, builder: &Builder<'_>) {
-        if builder.doc_tests == DocTests::Only {
+        if builder.test_target == TestTarget::DocOnly {
             return;
         }
 
@@ -2980,15 +2979,12 @@ fn prepare_cargo_test(
         cargo.arg("--message-format=json");
     }
 
-    match builder.doc_tests {
-        DocTests::Only => {
-            cargo.arg("--doc");
-        }
-        DocTests::No => {
-            cargo.args(["--bins", "--examples", "--tests", "--benches"]);
-        }
-        DocTests::Yes => {}
-    }
+    match builder.test_target {
+        TestTarget::AllTargets => cargo.args(["--bins", "--examples", "--tests", "--benches"]),
+        TestTarget::Default => &mut cargo,
+        TestTarget::DocOnly => cargo.arg("--doc"),
+        TestTarget::Tests => cargo.arg("--tests"),
+    };
 
     for krate in crates {
         cargo.arg("-p").arg(krate);
@@ -3868,7 +3864,7 @@ impl Step for CodegenCranelift {
         let host = run.build_triple();
         let compilers = RustcPrivateCompilers::new(run.builder, run.builder.top_stage, host);
 
-        if builder.doc_tests == DocTests::Only {
+        if builder.test_target == TestTarget::DocOnly {
             return;
         }
 
@@ -3989,7 +3985,7 @@ impl Step for CodegenGCC {
         let host = run.build_triple();
         let compilers = RustcPrivateCompilers::new(run.builder, run.builder.top_stage, host);
 
-        if builder.doc_tests == DocTests::Only {
+        if builder.test_target == TestTarget::DocOnly {
             return;
         }
 

src/bootstrap/src/core/config/config.rs

@@ -415,6 +415,12 @@ impl Config {
             "flags.exclude" = ?flags_exclude
         );
 
+        if flags_cmd.no_doc() {
+            eprintln!(
+                "WARN: `x.py test --no-doc` is renamed to `--all-targets`. `--no-doc` will be removed in the near future. Additionally `--tests` is added which only executes unit and integration tests."
+            )
+        }
+
         // Set config values based on flags.
         let mut exec_ctx = ExecutionContext::new(flags_verbose, flags_cmd.fail_fast());
         exec_ctx.set_dry_run(if flags_dry_run { DryRun::UserSelected } else { DryRun::Disabled });

src/bootstrap/src/core/config/flags.rs

@@ -15,7 +15,7 @@ use crate::core::build_steps::setup::Profile;
 use crate::core::builder::{Builder, Kind};
 use crate::core::config::Config;
 use crate::core::config::target_selection::{TargetSelectionList, target_selection_list};
-use crate::{Build, CodegenBackendKind, DocTests};
+use crate::{Build, CodegenBackendKind, TestTarget};
 
 #[derive(Copy, Clone, Default, Debug, ValueEnum)]
 pub enum Color {
@@ -357,7 +357,7 @@ pub enum Subcommand {
         should be compiled and run. For example:
             ./x.py test tests/ui
             ./x.py test library/std --test-args hash_map
-            ./x.py test library/std --stage 0 --no-doc
+            ./x.py test library/std --stage 0 --all-targets
             ./x.py test tests/ui --bless
             ./x.py test tests/ui --compare-mode next-solver
         Note that `test tests/* --stage N` does NOT depend on `build compiler/rustc --stage N`;
@@ -382,11 +382,14 @@ pub enum Subcommand {
         #[arg(long, value_name = "ARGS", allow_hyphen_values(true))]
         compiletest_rustc_args: Vec<String>,
         #[arg(long)]
-        /// do not run doc tests
-        no_doc: bool,
+        /// Run all test targets (no doc tests)
+        all_targets: bool,
         #[arg(long)]
-        /// only run doc tests
+        /// Only run doc tests
         doc: bool,
+        /// Only run unit and integration tests
+        #[arg(long)]
+        tests: bool,
         #[arg(long)]
         /// whether to automatically update stderr/stdout files
         bless: bool,
@@ -425,6 +428,11 @@ pub enum Subcommand {
         #[arg(long)]
         /// Ignore `//@ ignore-backends` directives.
         bypass_ignore_backends: bool,
+
+        /// Deprecated. Use `--all-targets` or `--tests` instead.
+        #[arg(long)]
+        #[doc(hidden)]
+        no_doc: bool,
     },
     /// Build and run some test suites *in Miri*
     Miri {
@@ -436,11 +444,19 @@ pub enum Subcommand {
         /// (e.g. libtest, compiletest or rustdoc)
         test_args: Vec<String>,
         #[arg(long)]
-        /// do not run doc tests
-        no_doc: bool,
+        /// Run all test targets (no doc tests)
+        all_targets: bool,
         #[arg(long)]
-        /// only run doc tests
+        /// Only run doc tests
         doc: bool,
+        /// Only run unit and integration tests
+        #[arg(long)]
+        tests: bool,
+
+        /// Deprecated. Use `--all-targets` or `--tests` instead.
+        #[arg(long)]
+        #[doc(hidden)]
+        no_doc: bool,
     },
     /// Build and run some benchmarks
     Bench {
@@ -552,18 +568,26 @@ impl Subcommand {
         }
     }
 
-    pub fn doc_tests(&self) -> DocTests {
+    pub fn test_target(&self) -> TestTarget {
         match *self {
-            Subcommand::Test { doc, no_doc, .. } | Subcommand::Miri { no_doc, doc, .. } => {
-                if doc {
-                    DocTests::Only
-                } else if no_doc {
-                    DocTests::No
-                } else {
-                    DocTests::Yes
+            Subcommand::Test { all_targets, doc, tests, .. }
+            | Subcommand::Miri { all_targets, doc, tests, .. } => match (all_targets, doc, tests) {
+                (true, true, _) | (true, _, true) | (_, true, true) => {
+                    panic!("You can only set one of `--all-targets`, `--doc` and `--tests`.")
                 }
-            }
-            _ => DocTests::Yes,
+                (true, false, false) => TestTarget::AllTargets,
+                (false, true, false) => TestTarget::DocOnly,
+                (false, false, true) => TestTarget::Tests,
+                (false, false, false) => TestTarget::Default,
+            },
+            _ => TestTarget::Default,
+        }
+    }
+
+    pub fn no_doc(&self) -> bool {
+        match *self {
+            Subcommand::Test { no_doc, .. } | Subcommand::Miri { no_doc, .. } => no_doc,
+            _ => false,
         }
     }
 

src/bootstrap/src/lib.rs

@@ -177,13 +177,21 @@ impl std::str::FromStr for CodegenBackendKind {
 }
 
 #[derive(PartialEq, Eq, Copy, Clone, Debug)]
-pub enum DocTests {
-    /// Run normal tests and doc tests (default).
-    Yes,
-    /// Do not run any doc tests.
-    No,
+pub enum TestTarget {
+    /// Run unit, integration and doc tests (default).
+    Default,
+    /// Run unit, integration, doc tests, examples, bins, benchmarks (no doc tests).
+    AllTargets,
     /// Only run doc tests.
-    Only,
+    DocOnly,
+    /// Only run unit and integration tests.
+    Tests,
+}
+
+impl TestTarget {
+    fn runs_doctests(&self) -> bool {
+        matches!(self, TestTarget::DocOnly | TestTarget::Default)
+    }
 }
 
 pub enum GitRepo {
@@ -222,7 +230,7 @@ pub struct Build {
     in_tree_gcc_info: GitInfo,
     local_rebuild: bool,
     fail_fast: bool,
-    doc_tests: DocTests,
+    test_target: TestTarget,
     verbosity: usize,
 
     /// Build triple for the pre-compiled snapshot compiler.
@@ -540,7 +548,7 @@ impl Build {
             initial_sysroot: config.initial_sysroot.clone(),
             local_rebuild: config.local_rebuild,
             fail_fast: config.cmd.fail_fast(),
-            doc_tests: config.cmd.doc_tests(),
+            test_target: config.cmd.test_target(),
             verbosity: config.exec_ctx.verbosity as usize,
 
             host_target: config.host_target,

src/bootstrap/src/utils/change_tracker.rs

@@ -73,7 +73,7 @@ pub fn human_readable_changes(changes: &[ChangeInfo]) -> String {
 /// Keeps track of major changes made to the bootstrap configuration.
 ///
 /// If you make any major changes (such as adding new values or changing default values),
-/// please ensure adding `ChangeInfo` to the end(because the list must be sorted by the merge date)
+/// please ensure adding `ChangeInfo` to the end (because the list must be sorted by the merge date)
 /// of this list.
 pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[
     ChangeInfo {
@@ -611,4 +611,9 @@ pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[
         severity: ChangeSeverity::Info,
         summary: "New option `llvm.offload-clang-dir` to allow building an in-tree llvm offload and openmp runtime with an external clang.",
     },
+    ChangeInfo {
+        change_id: 153143,
+        severity: ChangeSeverity::Warning,
+        summary: "`x.py test --no-doc` is renamed to `--all-targets`. Additionally `--tests` is added which only executes unit and integration tests.",
+    },
 ];

src/doc/rustc-dev-guide/src/tests/intro.md

@@ -62,7 +62,8 @@ would require recompiling the entire standard library, and the entirety of
 package tests:
 
 * `--doc` — Only runs documentation tests in the package.
-* `--no-doc` — Run all tests *except* documentation tests.
+* `--all-targets` — Run all tests *except* documentation tests.
+* `--tests` — Only runs unit and integration tests
 
 [tidy-unit-tests]: https://github.com/rust-lang/rust/blob/HEAD/src/tools/tidy/src/unit_tests.rs
 

src/etc/completions/x.fish

@@ -329,14 +329,16 @@ complete -c x -n "__fish_x_using_subcommand test" -l reproducible-artifact -d 'A
 complete -c x -n "__fish_x_using_subcommand test" -l set -d 'override options in bootstrap.toml' -r -f
 complete -c x -n "__fish_x_using_subcommand test" -l ci -d 'Make bootstrap to behave as it\'s running on the CI environment or not' -r -f -a "{true\t'',false\t''}"
 complete -c x -n "__fish_x_using_subcommand test" -l no-fail-fast -d 'run all tests regardless of failure'
-complete -c x -n "__fish_x_using_subcommand test" -l no-doc -d 'do not run doc tests'
-complete -c x -n "__fish_x_using_subcommand test" -l doc -d 'only run doc tests'
+complete -c x -n "__fish_x_using_subcommand test" -l all-targets -d 'Run all test targets (no doc tests)'
+complete -c x -n "__fish_x_using_subcommand test" -l doc -d 'Only run doc tests'
+complete -c x -n "__fish_x_using_subcommand test" -l tests -d 'Only run unit and integration tests'
 complete -c x -n "__fish_x_using_subcommand test" -l bless -d 'whether to automatically update stderr/stdout files'
 complete -c x -n "__fish_x_using_subcommand test" -l force-rerun -d 'rerun tests even if the inputs are unchanged'
 complete -c x -n "__fish_x_using_subcommand test" -l only-modified -d 'only run tests that result has been changed'
 complete -c x -n "__fish_x_using_subcommand test" -l rustfix-coverage -d 'enable this to generate a Rustfix coverage file, which is saved in `/<build_base>/rustfix_missing_coverage.txt`'
 complete -c x -n "__fish_x_using_subcommand test" -l no-capture -d 'don\'t capture stdout/stderr of tests'
 complete -c x -n "__fish_x_using_subcommand test" -l bypass-ignore-backends -d 'Ignore `//@ ignore-backends` directives'
+complete -c x -n "__fish_x_using_subcommand test" -l no-doc -d 'Deprecated. Use `--all-targets` or `--tests` instead'
 complete -c x -n "__fish_x_using_subcommand test" -s v -l verbose -d 'use verbose output (-vv for very verbose)'
 complete -c x -n "__fish_x_using_subcommand test" -s i -l incremental -d 'use incremental compilation'
 complete -c x -n "__fish_x_using_subcommand test" -l include-default-paths -d 'include default paths in addition to the provided ones'
@@ -374,8 +376,10 @@ complete -c x -n "__fish_x_using_subcommand miri" -l reproducible-artifact -d 'A
 complete -c x -n "__fish_x_using_subcommand miri" -l set -d 'override options in bootstrap.toml' -r -f
 complete -c x -n "__fish_x_using_subcommand miri" -l ci -d 'Make bootstrap to behave as it\'s running on the CI environment or not' -r -f -a "{true\t'',false\t''}"
 complete -c x -n "__fish_x_using_subcommand miri" -l no-fail-fast -d 'run all tests regardless of failure'
-complete -c x -n "__fish_x_using_subcommand miri" -l no-doc -d 'do not run doc tests'
-complete -c x -n "__fish_x_using_subcommand miri" -l doc -d 'only run doc tests'
+complete -c x -n "__fish_x_using_subcommand miri" -l all-targets -d 'Run all test targets (no doc tests)'
+complete -c x -n "__fish_x_using_subcommand miri" -l doc -d 'Only run doc tests'
+complete -c x -n "__fish_x_using_subcommand miri" -l tests -d 'Only run unit and integration tests'
+complete -c x -n "__fish_x_using_subcommand miri" -l no-doc -d 'Deprecated. Use `--all-targets` or `--tests` instead'
 complete -c x -n "__fish_x_using_subcommand miri" -s v -l verbose -d 'use verbose output (-vv for very verbose)'
 complete -c x -n "__fish_x_using_subcommand miri" -s i -l incremental -d 'use incremental compilation'
 complete -c x -n "__fish_x_using_subcommand miri" -l include-default-paths -d 'include default paths in addition to the provided ones'

src/etc/completions/x.ps1

@@ -376,14 +376,16 @@ Register-ArgumentCompleter -Native -CommandName 'x' -ScriptBlock {
             [CompletionResult]::new('--set', '--set', [CompletionResultType]::ParameterName, 'override options in bootstrap.toml')
             [CompletionResult]::new('--ci', '--ci', [CompletionResultType]::ParameterName, 'Make bootstrap to behave as it''s running on the CI environment or not')
             [CompletionResult]::new('--no-fail-fast', '--no-fail-fast', [CompletionResultType]::ParameterName, 'run all tests regardless of failure')
-            [CompletionResult]::new('--no-doc', '--no-doc', [CompletionResultType]::ParameterName, 'do not run doc tests')
-            [CompletionResult]::new('--doc', '--doc', [CompletionResultType]::ParameterName, 'only run doc tests')
+            [CompletionResult]::new('--all-targets', '--all-targets', [CompletionResultType]::ParameterName, 'Run all test targets (no doc tests)')
+            [CompletionResult]::new('--doc', '--doc', [CompletionResultType]::ParameterName, 'Only run doc tests')
+            [CompletionResult]::new('--tests', '--tests', [CompletionResultType]::ParameterName, 'Only run unit and integration tests')
             [CompletionResult]::new('--bless', '--bless', [CompletionResultType]::ParameterName, 'whether to automatically update stderr/stdout files')
             [CompletionResult]::new('--force-rerun', '--force-rerun', [CompletionResultType]::ParameterName, 'rerun tests even if the inputs are unchanged')
             [CompletionResult]::new('--only-modified', '--only-modified', [CompletionResultType]::ParameterName, 'only run tests that result has been changed')
             [CompletionResult]::new('--rustfix-coverage', '--rustfix-coverage', [CompletionResultType]::ParameterName, 'enable this to generate a Rustfix coverage file, which is saved in `/<build_base>/rustfix_missing_coverage.txt`')
             [CompletionResult]::new('--no-capture', '--no-capture', [CompletionResultType]::ParameterName, 'don''t capture stdout/stderr of tests')
             [CompletionResult]::new('--bypass-ignore-backends', '--bypass-ignore-backends', [CompletionResultType]::ParameterName, 'Ignore `//@ ignore-backends` directives')
+            [CompletionResult]::new('--no-doc', '--no-doc', [CompletionResultType]::ParameterName, 'Deprecated. Use `--all-targets` or `--tests` instead')
             [CompletionResult]::new('-v', '-v', [CompletionResultType]::ParameterName, 'use verbose output (-vv for very verbose)')
             [CompletionResult]::new('--verbose', '--verbose', [CompletionResultType]::ParameterName, 'use verbose output (-vv for very verbose)')
             [CompletionResult]::new('-i', '-i', [CompletionResultType]::ParameterName, 'use incremental compilation')
@@ -428,8 +430,10 @@ Register-ArgumentCompleter -Native -CommandName 'x' -ScriptBlock {
             [CompletionResult]::new('--set', '--set', [CompletionResultType]::ParameterName, 'override options in bootstrap.toml')
             [CompletionResult]::new('--ci', '--ci', [CompletionResultType]::ParameterName, 'Make bootstrap to behave as it''s running on the CI environment or not')
             [CompletionResult]::new('--no-fail-fast', '--no-fail-fast', [CompletionResultType]::ParameterName, 'run all tests regardless of failure')
-            [CompletionResult]::new('--no-doc', '--no-doc', [CompletionResultType]::ParameterName, 'do not run doc tests')
-            [CompletionResult]::new('--doc', '--doc', [CompletionResultType]::ParameterName, 'only run doc tests')
+            [CompletionResult]::new('--all-targets', '--all-targets', [CompletionResultType]::ParameterName, 'Run all test targets (no doc tests)')
+            [CompletionResult]::new('--doc', '--doc', [CompletionResultType]::ParameterName, 'Only run doc tests')
+            [CompletionResult]::new('--tests', '--tests', [CompletionResultType]::ParameterName, 'Only run unit and integration tests')
+            [CompletionResult]::new('--no-doc', '--no-doc', [CompletionResultType]::ParameterName, 'Deprecated. Use `--all-targets` or `--tests` instead')
             [CompletionResult]::new('-v', '-v', [CompletionResultType]::ParameterName, 'use verbose output (-vv for very verbose)')
             [CompletionResult]::new('--verbose', '--verbose', [CompletionResultType]::ParameterName, 'use verbose output (-vv for very verbose)')
             [CompletionResult]::new('-i', '-i', [CompletionResultType]::ParameterName, 'use incremental compilation')