Skip to content

no-fail-fast support for tool testsuites - #108264

Merged
bors merged 1 commit into
rust-lang:masterfrom
jchecahi:tool-testsuite-ignores-no-fail-fast
Feb 22, 2023
Merged

no-fail-fast support for tool testsuites#108264
bors merged 1 commit into
rust-lang:masterfrom
jchecahi:tool-testsuite-ignores-no-fail-fast

Conversation

@jchecahi

@jchecahijchecahi commented Feb 20, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

This commit adds a change to pass "--no-fail-fast" flag to cargo test inside tool::prepare_tool_cargo() so there is no need to do it manually in each Step trait implementation in src/bootstrap/test.rs.

Also, removes the flag from test.rs where prepare_tool_cargo() is called so cargo doesn't complain because the flag has been passed twice.

This commit adds --no-fail-fast flag to each cargo test
command in each tool Step trait implementation (miri, rustfmt and clippy).

Fixes#108261

@rustbot

rustbot commented Feb 20, 2023

Copy link
Copy Markdown
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ozkanonur (or someone else) soon.

Please see the contribution instructions for more information.

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Feb 20, 2023
@jchecahi

jchecahi commented Feb 20, 2023

Copy link
Copy Markdown
ContributorAuthor

I have tested this by enforcing some tests to fail by adding an assert!(1==2); statement in clippy, rustfmt and miri tests. Without the commit, the testsuite stops running after a failure, with the commit, it keeps going until the end.

@onur-ozkan

onur-ozkan commented Feb 21, 2023

Copy link
Copy Markdown
Contributor

Changing behaviour of test suite in tool module doesn't seem right. It also duplicates the control testing to multiple modules which isn't really good. You can pass --no-fail-fast for Miri, Rustfmt and Clippy same as in the other Step implementations in test module.

@onur-ozkan

onur-ozkan commented Feb 21, 2023

Copy link
Copy Markdown
Contributor

@rustbot author

@rustbotrustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 21, 2023
@jchecahi
jchecahiforce-pushed the tool-testsuite-ignores-no-fail-fast branch from 5db2939 to 83ae6efCompareFebruary 22, 2023 07:29
Jesus Checa Hidalgo
Add no-fail-fast support to miri, rustfmt and clippy testsuites
2f16355
This commit adds `--no-fail-fast` flag to each `cargo test`
command in each tool Step trait implementation.
Fixesrust-lang#108261
@jchecahi
jchecahiforce-pushed the tool-testsuite-ignores-no-fail-fast branch from 83ae6ef to 2f16355CompareFebruary 22, 2023 07:30
@jchecahi

jchecahi commented Feb 22, 2023

Copy link
Copy Markdown
ContributorAuthor

@ozkanonur thanks for the input, I've made the changes you recommended. Let me make sure I understood the reasoning behind your advice. IIUIC the changes related to the testsuites should be placed always in src/bootstrap/test.rs. Following that reasoning, if the goal is to gather this kind of duplicated logic (as suggested in #104198), the proper way to do it would be to create a function in src/bootstrap/test.rs that takes care of that. Did I understand it correctly?

@onur-ozkan

onur-ozkan commented Feb 22, 2023

Copy link
Copy Markdown
Contributor

We already pass the test flags in test module, if we do it in other modules as well, it would be not good for us to work/debug on it.

And yes, creating fn could be quite nice.

@onur-ozkan

onur-ozkan commented Feb 22, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

This looks good for now, we can do the code unification in seperated PR pointing the related issue. Thank you for the contribution.

@onur-ozkan

onur-ozkan commented Feb 22, 2023

Copy link
Copy Markdown
Contributor

@bors r+ rollup

@bors

bors commented Feb 22, 2023

Copy link
Copy Markdown
Collaborator

📌 Commit 2f16355 has been approved by ozkanonur

It is now in the queue for this repository.

@borsbors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 22, 2023
@GuillaumeGomezGuillaumeGomez mentioned this pull request Feb 22, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 22, 2023
@bors
Auto merge ofrust-lang#108339- GuillaumeGomez:rollup-4z02kas, r=Gui…
3b4d6e0
…llaumeGomez
Rollup of 8 pull requests
Successful merges:
- rust-lang#108110 (Move some `InferCtxt` methods to `EvalCtxt` in new solver)
- rust-lang#108168 (Fix ICE on type alias in recursion)
- rust-lang#108230 (Convert a hard-warning about named static lifetimes into lint "unused_lifetimes")
- rust-lang#108239 (Fix overlapping spans in removing extra arguments)
- rust-lang#108246 (Add an InstCombine for redundant casts)
- rust-lang#108264 (no-fail-fast support for tool testsuites)
- rust-lang#108310 (rustdoc: Fix duplicated attributes for first reexport)
- rust-lang#108318 (Remove unused FileDesc::get_cloexec)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit 8fd47a6 into rust-lang:masterFeb 22, 2023
@rustbotrustbot added this to the 1.69.0 milestone Feb 22, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@onur-ozkanonur-ozkan

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-bootstrapRelevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Projects

None yet

Milestone

1.69.0

Development

Successfully merging this pull request may close these issues.

x.py test ignores --no-fail-fast in some tool testsuites.

4 participants

@jchecahi@rustbot@onur-ozkan@bors