From 5ae529b24ed7bd03f0d1dfc52e5c25a0c9adf710 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Fri, 11 Aug 2023 14:12:59 +0100 Subject: [PATCH 1/2] feat: remove `--keep-going` from `cargo test/bench` It confuses people that both `--no-fail-fast` and `--keep-going` exist on `cargo test` and `cargo bench` but with slightly different behavior. The intended use cases for `--keep-going` involve build commands like `build`/`check`/`clippy` but never `test`/`bench`. Hence, this commit removes `--keep-going` from `test`/`bench` and provides guidance of `--no-fail-fast` instead. If people really want to build as many tests as possible, they can also do it in two steps: cargo build --tests --keep-going cargo test --test --no-fail-fast --- src/bin/cargo/commands/bench.rs | 14 +++++++++++++- src/bin/cargo/commands/test.rs | 13 ++++++++++++- src/cargo/util/command_prelude.rs | 17 ++++++++++------- src/doc/man/cargo-test.md | 1 - src/doc/man/generated_txt/cargo-test.txt | 5 ----- src/doc/src/commands/cargo-test.md | 6 ------ src/etc/man/cargo-test.1 | 7 ------- tests/testsuite/bench.rs | 18 ++++++++++++++++++ tests/testsuite/cargo_bench/help/stdout.log | 1 - tests/testsuite/cargo_test/help/stdout.log | 1 - tests/testsuite/test.rs | 18 ++++++++++++++++++ 11 files changed, 71 insertions(+), 30 deletions(-) diff --git a/src/bin/cargo/commands/bench.rs b/src/bin/cargo/commands/bench.rs index 7f4c4eeafc6..bb2c193b0a1 100644 --- a/src/bin/cargo/commands/bench.rs +++ b/src/bin/cargo/commands/bench.rs @@ -42,7 +42,8 @@ pub fn cli() -> Command { "Benchmark all targets", ) .arg_features() - .arg_jobs() + .arg_jobs_without_keep_going() + .arg(flag("keep-going", "Use `--no-fail-fast` instead").hide(true)) // See rust-lang/cargo#11702 .arg_profile("Build artifacts with the specified profile") .arg_target_triple("Build for the target triple") .arg_target_dir() @@ -54,6 +55,17 @@ pub fn cli() -> Command { pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { let ws = args.workspace(config)?; + + if args.keep_going() { + return Err(anyhow::format_err!( + "\ +unexpected argument `--keep-going` found + + tip: to run as many benchmarks as possible without failing fast, use `--no-fail-fast`" + ) + .into()); + } + let mut compile_opts = args.compile_options( config, CompileMode::Bench, diff --git a/src/bin/cargo/commands/test.rs b/src/bin/cargo/commands/test.rs index e9cc4b93113..80c935d628e 100644 --- a/src/bin/cargo/commands/test.rs +++ b/src/bin/cargo/commands/test.rs @@ -48,7 +48,8 @@ pub fn cli() -> Command { "Test all targets (does not include doctests)", ) .arg_features() - .arg_jobs() + .arg_jobs_without_keep_going() + .arg(flag("keep-going", "Use `--no-fail-fast` instead").hide(true)) // See rust-lang/cargo#11702 .arg_release("Build artifacts in release mode, with optimizations") .arg_profile("Build artifacts with the specified profile") .arg_target_triple("Build for the target triple") @@ -65,6 +66,16 @@ pub fn cli() -> Command { pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { let ws = args.workspace(config)?; + if args.keep_going() { + return Err(anyhow::format_err!( + "\ +unexpected argument `--keep-going` found + + tip: to run as many tests as possible without failing fast, use `--no-fail-fast`" + ) + .into()); + } + let mut compile_opts = args.compile_options( config, CompileMode::Test, diff --git a/src/cargo/util/command_prelude.rs b/src/cargo/util/command_prelude.rs index db6fae34822..bc707ef6f65 100644 --- a/src/cargo/util/command_prelude.rs +++ b/src/cargo/util/command_prelude.rs @@ -83,6 +83,16 @@ pub trait CommandExt: Sized { } fn arg_jobs(self) -> Self { + self.arg_jobs_without_keep_going()._arg( + flag( + "keep-going", + "Do not abort the build as soon as there is an error (unstable)", + ) + .help_heading(heading::COMPILATION_OPTIONS), + ) + } + + fn arg_jobs_without_keep_going(self) -> Self { self._arg( opt("jobs", "Number of parallel jobs, defaults to # of CPUs.") .short('j') @@ -90,13 +100,6 @@ pub trait CommandExt: Sized { .allow_hyphen_values(true) .help_heading(heading::COMPILATION_OPTIONS), ) - ._arg( - flag( - "keep-going", - "Do not abort the build as soon as there is an error (unstable)", - ) - .help_heading(heading::COMPILATION_OPTIONS), - ) } fn arg_targets_all( diff --git a/src/doc/man/cargo-test.md b/src/doc/man/cargo-test.md index 75bf72b3070..cba98b20daf 100644 --- a/src/doc/man/cargo-test.md +++ b/src/doc/man/cargo-test.md @@ -186,7 +186,6 @@ includes an option to control the number of threads used: {{#options}} {{> options-jobs }} -{{> options-keep-going }} {{> options-future-incompat }} {{/options}} diff --git a/src/doc/man/generated_txt/cargo-test.txt b/src/doc/man/generated_txt/cargo-test.txt index 7955b0e3dd9..dc32bdbf7fe 100644 --- a/src/doc/man/generated_txt/cargo-test.txt +++ b/src/doc/man/generated_txt/cargo-test.txt @@ -442,11 +442,6 @@ OPTIONS If a string default is provided, it sets the value back to defaults. Should not be 0. - --keep-going - Build as many crates in the dependency graph as possible, rather - than aborting the build on the first one that fails to build. - Unstable, requires -Zunstable-options. - --future-incompat-report Displays a future-incompat report for any future-incompatible warnings produced during execution of this command diff --git a/src/doc/src/commands/cargo-test.md b/src/doc/src/commands/cargo-test.md index e38e9929e22..bcf46d60191 100644 --- a/src/doc/src/commands/cargo-test.md +++ b/src/doc/src/commands/cargo-test.md @@ -515,12 +515,6 @@ a string default is provided, it sets the value back to defaults. Should not be 0. -
--keep-going
-
Build as many crates in the dependency graph as possible, rather than aborting -the build on the first one that fails to build. Unstable, requires --Zunstable-options.
- -
--future-incompat-report
Displays a future-incompat report for any future-incompatible warnings produced during execution of this command

diff --git a/src/etc/man/cargo-test.1 b/src/etc/man/cargo-test.1 index 802169815b6..4ca150dbcb1 100644 --- a/src/etc/man/cargo-test.1 +++ b/src/etc/man/cargo-test.1 @@ -535,13 +535,6 @@ a string \fBdefault\fR is provided, it sets the value back to defaults. Should not be 0. .RE .sp -\fB\-\-keep\-going\fR -.RS 4 -Build as many crates in the dependency graph as possible, rather than aborting -the build on the first one that fails to build. Unstable, requires -\fB\-Zunstable\-options\fR\&. -.RE -.sp \fB\-\-future\-incompat\-report\fR .RS 4 Displays a future\-incompat report for any future\-incompatible warnings diff --git a/tests/testsuite/bench.rs b/tests/testsuite/bench.rs index 581acbe1505..64b13f48b65 100644 --- a/tests/testsuite/bench.rs +++ b/tests/testsuite/bench.rs @@ -1670,3 +1670,21 @@ fn json_artifact_includes_executable_for_benchmark() { ) .run(); } + +#[cargo_test] +fn cargo_bench_no_keep_going() { + let p = project() + .file("Cargo.toml", &basic_bin_manifest("foo")) + .file("src/main.rs", "") + .build(); + + p.cargo("bench --keep-going") + .with_stderr( + "\ +error: unexpected argument `--keep-going` found + + tip: to run as many benchmarks as possible without failing fast, use `--no-fail-fast`", + ) + .with_status(101) + .run(); +} diff --git a/tests/testsuite/cargo_bench/help/stdout.log b/tests/testsuite/cargo_bench/help/stdout.log index 4fe8cb5e263..5d9484df9ee 100644 --- a/tests/testsuite/cargo_bench/help/stdout.log +++ b/tests/testsuite/cargo_bench/help/stdout.log @@ -44,7 +44,6 @@ Feature Selection: Compilation Options: -j, --jobs Number of parallel jobs, defaults to # of CPUs. - --keep-going Do not abort the build as soon as there is an error (unstable) --profile Build artifacts with the specified profile --target Build for the target triple --target-dir Directory for all generated artifacts diff --git a/tests/testsuite/cargo_test/help/stdout.log b/tests/testsuite/cargo_test/help/stdout.log index a0c842bb5b8..d693dc3c92d 100644 --- a/tests/testsuite/cargo_test/help/stdout.log +++ b/tests/testsuite/cargo_test/help/stdout.log @@ -46,7 +46,6 @@ Feature Selection: Compilation Options: -j, --jobs Number of parallel jobs, defaults to # of CPUs. - --keep-going Do not abort the build as soon as there is an error (unstable) -r, --release Build artifacts in release mode, with optimizations --profile Build artifacts with the specified profile --target Build for the target triple diff --git a/tests/testsuite/test.rs b/tests/testsuite/test.rs index d93ea5ac8f7..4820c7dcd78 100644 --- a/tests/testsuite/test.rs +++ b/tests/testsuite/test.rs @@ -4843,3 +4843,21 @@ error: 2 targets failed: .with_status(101) .run(); } + +#[cargo_test] +fn cargo_test_no_keep_going() { + let p = project() + .file("Cargo.toml", &basic_bin_manifest("foo")) + .file("src/main.rs", "") + .build(); + + p.cargo("test --keep-going") + .with_stderr( + "\ +error: unexpected argument `--keep-going` found + + tip: to run as many tests as possible without failing fast, use `--no-fail-fast`", + ) + .with_status(101) + .run(); +} From de8b9139ba77500929aea35835fd81d4b0212755 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Fri, 11 Aug 2023 15:39:46 +0100 Subject: [PATCH 2/2] feat: remove completions of `--keep-going` for bench/test To test, simply run: zsh ```zsh fpath+=$PWD/src/etc autoload -Uz compinit compinit cargo t ``` bash: ```bash . ./src/etc/cargo.bashcomp.sh cargo t ``` --- src/etc/_cargo | 12 ++++++++---- src/etc/cargo.bashcomp.sh | 7 ++++--- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/etc/_cargo b/src/etc/_cargo index bdceb10c97f..164d7679f2b 100644 --- a/src/etc/_cargo +++ b/src/etc/_cargo @@ -45,9 +45,13 @@ _cargo() { '(--bench --bin --example --lib)--test=[specify test name]:test name' ) - parallel=( + jobs=( '(-j --jobs)'{-j+,--jobs=}'[specify number of parallel jobs]:jobs [# of CPUs]' - '--keep-going[do not abort build on first error]' + ) + + parallel=( + "${jobs[@]}" + '--keep-going[do not abort build on first build error]' ) features=( @@ -87,7 +91,7 @@ _cargo() { '*:args:_default' ;; bench) - _arguments -s -A "^--" $common $parallel $features $msgfmt $triple $target $manifest \ + _arguments -s -A "^--" $common $jobs $features $msgfmt $triple $target $manifest \ "${command_scope_spec[@]}" \ '--all-targets[benchmark all targets]' \ "--no-run[compile but don't run]" \ @@ -297,7 +301,7 @@ _cargo() { ;; test | t) - _arguments -s -S $common $parallel $features $msgfmt $triple $target $manifest \ + _arguments -s -S $common $jobs $features $msgfmt $triple $target $manifest \ '--test=[test name]: :_cargo_test_names' \ '--no-fail-fast[run all tests regardless of failure]' \ '--no-run[compile but do not run]' \ diff --git a/src/etc/cargo.bashcomp.sh b/src/etc/cargo.bashcomp.sh index 2867ec56dd5..33f225ebf39 100644 --- a/src/etc/cargo.bashcomp.sh +++ b/src/etc/cargo.bashcomp.sh @@ -41,7 +41,8 @@ _cargo() local opt_pkg='-p --package' local opt_feat='-F --features --all-features --no-default-features' local opt_mani='--manifest-path' - local opt_parallel='-j --jobs --keep-going' + local opt_jobs='-j --jobs' + local opt_parallel="$opt_jobs --keep-going" local opt_force='-f --force' local opt_sync='-s --sync' local opt_lock='--frozen --locked --offline' @@ -49,7 +50,7 @@ _cargo() local opt___nocmd="$opt_common -V --version --list --explain" local opt__add="$opt_common -p --package --features --default-features --no-default-features $opt_mani --optional --no-optional --rename --dry-run --path --git --branch --tag --rev --registry --dev --build --target" - local opt__bench="$opt_common $opt_pkg_spec $opt_feat $opt_mani $opt_lock $opt_parallel $opt_targets --message-format --target --no-run --no-fail-fast --target-dir --ignore-rust-version" + local opt__bench="$opt_common $opt_pkg_spec $opt_feat $opt_mani $opt_lock $opt_jobs $opt_targets --message-format --target --no-run --no-fail-fast --target-dir --ignore-rust-version" local opt__build="$opt_common $opt_pkg_spec $opt_feat $opt_mani $opt_lock $opt_parallel $opt_targets --message-format --target --release --profile --target-dir --ignore-rust-version" local opt__b="$opt__build" local opt__check="$opt_common $opt_pkg_spec $opt_feat $opt_mani $opt_lock $opt_parallel $opt_targets --message-format --target --release --profile --target-dir --ignore-rust-version" @@ -82,7 +83,7 @@ _cargo() local opt__rustc="$opt_common $opt_pkg $opt_feat $opt_mani $opt_lock $opt_parallel $opt_targets -L --crate-type --extern --message-format --profile --target --release --target-dir --ignore-rust-version" local opt__rustdoc="$opt_common $opt_pkg $opt_feat $opt_mani $opt_lock $opt_parallel $opt_targets --message-format --target --release --open --target-dir --profile --ignore-rust-version" local opt__search="$opt_common $opt_lock --limit --index --registry" - local opt__test="$opt_common $opt_pkg_spec $opt_feat $opt_mani $opt_lock $opt_parallel $opt_targets --message-format --doc --target --no-run --release --no-fail-fast --target-dir --profile --ignore-rust-version" + local opt__test="$opt_common $opt_pkg_spec $opt_feat $opt_mani $opt_lock $opt_jobs $opt_targets --message-format --doc --target --no-run --release --no-fail-fast --target-dir --profile --ignore-rust-version" local opt__t="$opt__test" local opt__tree="$opt_common $opt_pkg_spec $opt_feat $opt_mani $opt_lock --target -i --invert --prefix --no-dedupe --duplicates -d --charset -f --format -e --edges" local opt__uninstall="$opt_common $opt_lock $opt_pkg --bin --root"