-
Notifications
You must be signed in to change notification settings - Fork 12.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add more comments to the bootstrap code that handles tests/coverage
#122982
Conversation
rustbot has assigned @albertlarsan68. Use |
@@ -1319,6 +1319,7 @@ macro_rules! coverage_test_alias { | |||
} | |||
|
|||
fn run(self, builder: &Builder<'_>) { | |||
// Actually running the tests goes through a common helper method. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment can be improved I think. It doesn't say enough about the context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, as think about how to explain this better, I’m realising that going through Coverage
is unnecessarily complicated anyway.
If I change run_unified_suite
to be a top-level helper function (with a better name), it should be easier to write a good comment. I’ll try it out tomorrow and see how it looks.
633ca8a
to
b8834fa
Compare
b8834fa
to
1bbab71
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
@bors r+ rollup |
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#122858 (Tweak `parse_dot_suffix_expr`) - rust-lang#122982 (Add more comments to the bootstrap code that handles `tests/coverage`) - rust-lang#122990 (Clarify transmute example) - rust-lang#122995 (Clean up unnecessary headers/flags in coverage mir-opt tests) - rust-lang#123003 (CFI: Handle dyn with no principal) - rust-lang#123005 (CFI: Support complex receivers) - rust-lang#123020 (Temporarily remove nnethercote from the review rotation.) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#122982 - Zalathar:bootstrap-coverage-docs, r=onur-ozkan Add more comments to the bootstrap code that handles `tests/coverage` At the bootstrap level, coverage tests are a bit more complicated than other test suites, because we want to run the same set of test files in multiple different modes, in a way that's convenient and flexible when invoked manually. This PR adds a few more comments to explain what's going on.
At the bootstrap level, coverage tests are a bit more complicated than other test suites, because we want to run the same set of test files in multiple different modes, in a way that's convenient and flexible when invoked manually.
This PR adds a few more comments to explain what's going on.