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

coverage: Tidy up run-coverage tests in several small ways #114924

Merged
merged 3 commits into from
Aug 27, 2023

Conversation

Zalathar
Copy link
Contributor

@Zalathar Zalathar commented Aug 17, 2023

Prior to #114875 (anonymized line numbers), these tests were very sensitive to lines being added/removed, since doing so would change the line numbers in every subsequent line of output. Therefore they ended up with a few small style issues that weren't worth the hassle of fixing.

Now that we can freely rearrange lines without needing to re-bless the entire output, we can tidy up the tests in various trivial ways.

@rustbot
Copy link
Collaborator

rustbot commented Aug 17, 2023

r? @Mark-Simulacrum

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) labels Aug 17, 2023
@Zalathar Zalathar force-pushed the tidy-tests branch 6 times, most recently from 3d56459 to 6d8832e Compare August 24, 2023 03:23
When one of these tests fails, any compiler warnings will be printed to the
console, which makes it harder to track down the actual reason for failure.

(The outstanding warnings were found by temporarily adding `-Dwarnings` to the
compiler arguments for `RunCoverage` in `src/tools/compiletest/src/runtest.rs`.)
Prior to rust-lang#114875, these tests were very sensitive to lines being added/removed,
so the migration to `run-coverage` in rust-lang#112300 tried hard to avoid disturbing
the existing line numbers. That resulted in some awkward reshuffling when
certain comments/directives needed to be added or moved.

Now that we don't have to worry about preserving line numbers, we can rearrange
those comments into a more conventional layout.
These changes were made by manually running `rustfmt` on all of the test files,
and then manually undoing all cases where the original formatting appeared to
have been deliberate.

  `rustfmt +nightly --config-path=/dev/null --edition=2021 tests/run-coverage*/**/*.rs`
@Mark-Simulacrum
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Aug 26, 2023

📌 Commit 8d91e71 has been approved by Mark-Simulacrum

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 26, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 27, 2023
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#114924 (coverage: Tidy up `run-coverage` tests in several small ways)
 - rust-lang#114927 (CI: add more debug logging to Docker caching)
 - rust-lang#114957 (tests: Fix tests for LoongArch64)
 - rust-lang#115007 (Correct and expand documentation of `handle_alloc_error` and `set_alloc_error_hook`.)
 - rust-lang#115098 (rust-gdbgui: remove excessive quotes)
 - rust-lang#115111 (compile rust-anaylzer with `x check` if it's enabled)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit be416b9 into rust-lang:master Aug 27, 2023
11 checks passed
@rustbot rustbot added this to the 1.74.0 milestone Aug 27, 2023
@Zalathar Zalathar deleted the tidy-tests branch August 27, 2023 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) 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