Skip to content

feat: remove --keep-going from cargo test/bench #12478

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Aug 13, 2023

Conversation

weihanglo
Copy link
Member

@weihanglo weihanglo commented Aug 11, 2023

What does this PR try to resolve?

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

How should we test and review this PR?

Only one commit containing two new tests verifying the error message.

To test the completion, simply run:

zsh

fpath+=$PWD/src/etc
autoload -Uz compinit
compinit

cargo t <tab>

bash

. ./src/etc/cargo.bashcomp.sh

cargo t <tab>

Additional information

cc #10496 (comment)

How long should we wait after this gets merged for stabilizing --keep-going?

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
@rustbot
Copy link
Collaborator

rustbot commented Aug 11, 2023

r? @ehuss

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-cli Area: Command-line interface, option parsing, etc. A-cli-help Area: built-in command-line help A-documenting-cargo-itself Area: Cargo's documentation Command-bench Command-test S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 11, 2023
@weihanglo
Copy link
Member Author

Wish we could have something from #11702 or clap-rs/clap#4706 for suggestions.

To test, simply run:

zsh

```zsh
fpath+=$PWD/src/etc
autoload -Uz compinit
compinit

cargo t <tab>
```

bash:

```bash

. ./src/etc/cargo.bashcomp.sh

cargo t <tab>
```
@rustbot rustbot added the A-completions Area: shell completions label Aug 11, 2023
@epage
Copy link
Contributor

epage commented Aug 13, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Aug 13, 2023

📌 Commit de8b913 has been approved by epage

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 13, 2023
@bors
Copy link
Contributor

bors commented Aug 13, 2023

⌛ Testing commit de8b913 with merge 7e9de3f...

@bors
Copy link
Contributor

bors commented Aug 13, 2023

☀️ Test successful - checks-actions
Approved by: epage
Pushing 7e9de3f to master...

@bors bors merged commit 7e9de3f into rust-lang:master Aug 13, 2023
@weihanglo weihanglo deleted the keep-going branch August 13, 2023 05:11
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 13, 2023
Update cargo

21 commits in d78bbf4bde3c6b95caca7512f537c6f9721426ff..7e9de3f4ec3708f500bec142317895b96131e47c
2023-08-03 12:58:25 +0000 to 2023-08-13 00:47:32 +0000
- feat: remove `--keep-going` from `cargo test/bench` (rust-lang/cargo#12478)
- chore: window-sys should be a platform-specific dependency (rust-lang/cargo#12483)
- docs: make the env var source of rerun-if-env-changed clearer (rust-lang/cargo#12482)
- doc: note the backward compatible `.cargo/credential` file exists (rust-lang/cargo#12479)
- Fix elided lifetime in associated const (rust-lang/cargo#12475)
- prompt the use of `--nocapture` flag if `cargo test` process is terminated via a signal. (rust-lang/cargo#12463)
- cargo-credential: reset stdin & stdout to the Console (rust-lang/cargo#12469)
- Fix cargo remove incorrectly removing used patches (rust-lang/cargo#12454)
- chore(gh): Expand update window (rust-lang/cargo#12466)
- Fix panic when enabling http.debug for certain strings (rust-lang/cargo#12468)
- fix(cli): Make `--help` easier to browse (rust-lang/cargo#11905)
- fix: preserve jobserver file descriptors on rustc invocation to get `TargetInfo` (rust-lang/cargo#12447)
- refactor: migrate to `tracing` (rust-lang/cargo#12458)
- docs: add example for cargo-credential (rust-lang/cargo#12461)
- Bail out an error when using cargo:: in custom build script (rust-lang/cargo#12332)
- Fix printing multiple warning messages for unused fields in [registries] table (rust-lang/cargo#12439)
- Update windows dependencies (rust-lang/cargo#12453)
- Rustfmt a let-else statement (rust-lang/cargo#12451)
- Add allow(internal_features) (rust-lang/cargo#12450)
- Update pretty_env_logger to 0.5 (rust-lang/cargo#12445)
- Remove build metadata from libgit2-sys dependency (rust-lang/cargo#12444)

r? `@ghost`
@ehuss ehuss added this to the 1.73.0 milestone Aug 22, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-cli Area: Command-line interface, option parsing, etc. A-cli-help Area: built-in command-line help A-completions Area: shell completions A-documenting-cargo-itself Area: Cargo's documentation Command-bench Command-test S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants