Skip to content
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

Merged
merged 1 commit into from
Mar 25, 2024

Conversation

Zalathar
Copy link
Contributor

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.

@rustbot
Copy link
Collaborator

rustbot commented Mar 24, 2024

r? @albertlarsan68

rustbot has assigned @albertlarsan68.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot 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 Mar 24, 2024
@@ -1319,6 +1319,7 @@ macro_rules! coverage_test_alias {
}

fn run(self, builder: &Builder<'_>) {
// Actually running the tests goes through a common helper method.
Copy link
Member

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.

Copy link
Contributor Author

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.

@Zalathar Zalathar force-pushed the bootstrap-coverage-docs branch 2 times, most recently from 633ca8a to b8834fa Compare March 25, 2024 04:30
@Zalathar Zalathar force-pushed the bootstrap-coverage-docs branch from b8834fa to 1bbab71 Compare March 25, 2024 04:31
Copy link
Member

@onur-ozkan onur-ozkan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@onur-ozkan
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 25, 2024

📌 Commit 1bbab71 has been approved by onur-ozkan

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 Mar 25, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 25, 2024
…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
@bors bors merged commit fbb81fa into rust-lang:master Mar 25, 2024
11 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Mar 25, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 25, 2024
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.
@Zalathar Zalathar deleted the bootstrap-coverage-docs branch March 25, 2024 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants