Warn when not having a profiler runtime means that coverage tests won't be run/blessed #120083
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @wesleywiser (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
A-testsuite
wesleywiser
approved these changes
Jan 18, 2024
|
Thanks! This is a pretty easy footgun to trip over 👍 @bors r+ rollup |
bors
added
S-waiting-on-bors
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 19, 2024
…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#118665 (Consolidate all associated items on the NonZero integer types into a single impl block per type) - rust-lang#118798 (Use AtomicU8 instead of AtomicUsize in backtrace.rs) - rust-lang#119062 (Deny braced macro invocations in let-else) - rust-lang#119138 (Docs: Use non-SeqCst in module example of atomics) - rust-lang#119907 (Update `fn()` trait implementation docs) - rust-lang#120083 (Warn when not having a profiler runtime means that coverage tests won't be run/blessed) - rust-lang#120107 (dead_code treats #[repr(transparent)] the same as #[repr(C)]) - rust-lang#120110 (Update documentation for Vec::into_boxed_slice to be more clear about excess capacity) - rust-lang#120113 (Remove myself from review rotation) - rust-lang#120118 (Fix typo in documentation in base.rs) r? `@ghost` `@rustbot` modify labels: rollup
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 19, 2024
…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#118665 (Consolidate all associated items on the NonZero integer types into a single impl block per type) - rust-lang#118798 (Use AtomicU8 instead of AtomicUsize in backtrace.rs) - rust-lang#119062 (Deny braced macro invocations in let-else) - rust-lang#119138 (Docs: Use non-SeqCst in module example of atomics) - rust-lang#119907 (Update `fn()` trait implementation docs) - rust-lang#120083 (Warn when not having a profiler runtime means that coverage tests won't be run/blessed) - rust-lang#120107 (dead_code treats #[repr(transparent)] the same as #[repr(C)]) - rust-lang#120110 (Update documentation for Vec::into_boxed_slice to be more clear about excess capacity) - rust-lang#120113 (Remove myself from review rotation) - rust-lang#120118 (Fix typo in documentation in base.rs) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 19, 2024
Rollup merge of rust-lang#120083 - Zalathar:no-profiler, r=wesleywiser Warn when not having a profiler runtime means that coverage tests won't be run/blessed On a few occasions (e.g. rust-lang#118036, rust-lang#119984) people have been tripped up by the fact that half of the coverage test suite is skipped by default, because it `// needs-profiler-support` and the profiler runtime is not actually built in any of the default config profiles. (This is made worse by the fact that it isn't enabled in any of the PR CI jobs either. So people think that they've successfully blessed the test suite, and then get a rude surprise when their merge only fails in the full CI job suite.) This PR adds a simple warning to compiletest that should alert the user in some cases. It's not foolproof, but it should increase the chances of catching this problem earlier in the PR process.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
On a few occasions (e.g. #118036, #119984) people have been tripped up by the fact that half of the coverage test suite is skipped by default, because it
// needs-profiler-supportand the profiler runtime is not actually built in any of the default config profiles.(This is made worse by the fact that it isn't enabled in any of the PR CI jobs either. So people think that they've successfully blessed the test suite, and then get a rude surprise when their merge only fails in the full CI job suite.)
This PR adds a simple warning to compiletest that should alert the user in some cases. It's not foolproof, but it should increase the chances of catching this problem earlier in the PR process.