coverage: Add UI tests for values accepted by `-Cinstrument-coverage` #117040

Zalathar · 2023-10-22T06:41:54Z

I wanted to clean up the code in parse_instrument_coverage, but it occurred to me that we currently don't have any UI tests for the various stable and unstable values supported by this flag.

Normally it might be overkill to individually test all the different variants of on/off, but in this case the parsing of those values is mixed in with some other custom code, so I think it's worthwhile being thorough.

rustbot · 2023-10-22T06:42:02Z

r? @b-naber

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

Zalathar · 2023-10-22T06:43:32Z

@rustbot label +A-code-coverage

cc @Swatinem as this is relevant to #116094

Also indirectly relevant to the possible removal of -Zinstrument-coverage (#116980).

Zalathar · 2023-10-22T06:45:16Z

I would also like to change the documented default value from all to on (to permit all to possibly do something different in the future), but I'll propose and implement that separately.

Zalathar · 2023-10-22T07:08:54Z

src/tools/tidy/src/ui_tests.rs

@@ -11,7 +11,7 @@ use std::path::{Path, PathBuf};
 const ENTRY_LIMIT: usize = 900;
 // FIXME: The following limits should be reduced eventually.
 const ISSUES_ENTRY_LIMIT: usize = 1854;
-const ROOT_ENTRY_LIMIT: usize = 865;
+const ROOT_ENTRY_LIMIT: usize = 866;


Bumping this by 1 because I'm creating a new directory, and I'm not aware of any existing tests directly in ui/ that could be moved into it.

Zalathar · 2023-10-22T07:32:27Z

Ah, looks like I need to add // needs-profiler-support.

(Or I could avoid flags that avoid needing the profiler runtime in the first place, but that doesn't seem necessary here.)

Zalathar · 2023-10-22T07:40:51Z

src/tools/compiletest/src/runtest.rs

@@ -2470,7 +2470,7 @@ impl<'test> TestCx<'test> {
            }
            CoverageMap => {
                rustc.arg("-Cinstrument-coverage");
-                // These tests only compile to MIR, so they don't need the
+                // These tests only compile to LLVM IR, so they don't need the


This is a mostly-unrelated comment fix that I happened to notice at around the same time.

Zalathar · 2023-10-23T06:43:38Z

Added an explicit error-case test for -Cinstrument-coverage= with no value (diff).

cjgillot · 2023-10-23T10:55:59Z

@bors r+ rollup

bors · 2023-10-23T10:56:01Z

📌 Commit 71b7322 has been approved by cjgillot

It is now in the queue for this repository.

…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#116960 (Location-insensitive polonius: consider a loan escaping if an SCC has member constraints applied only) - rust-lang#116978 (Rewrite gdb pretty-printer registration) - rust-lang#117040 (coverage: Add UI tests for values accepted by `-Cinstrument-coverage`) - rust-lang#117064 (Eliminate rustc_attrs::builtin::handle_errors in favor of emitting errors directly) - rust-lang#117073 (Fix suggestion for renamed coroutines feature) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#117040 - Zalathar:instrument-coverage-ui, r=cjgillot coverage: Add UI tests for values accepted by `-Cinstrument-coverage` I wanted to clean up the code in `parse_instrument_coverage`, but it occurred to me that we currently don't have any UI tests for the various stable and unstable values supported by this flag. --- Normally it might be overkill to individually test all the different variants of `on`/`off`, but in this case the parsing of those values is mixed in with some other custom code, so I think it's worthwhile being thorough.

rustbot assigned b-naber Oct 22, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 22, 2023

rustbot added the A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) label Oct 22, 2023

This comment has been minimized.

Sign in to view

Zalathar force-pushed the instrument-coverage-ui branch from 22be6c0 to c06b444 Compare October 22, 2023 07:03

rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Oct 22, 2023

Zalathar commented Oct 22, 2023

View reviewed changes

This comment has been minimized.

Sign in to view

Zalathar force-pushed the instrument-coverage-ui branch from c06b444 to 2ac5ac9 Compare October 22, 2023 07:39

Zalathar commented Oct 22, 2023

View reviewed changes

Swatinem approved these changes Oct 22, 2023

View reviewed changes

Zalathar force-pushed the instrument-coverage-ui branch from 2ac5ac9 to 6a3393c Compare October 23, 2023 06:20

Zalathar added 2 commits October 23, 2023 17:41

coverage: Add UI tests for values accepted by -Cinstrument-coverage

f83f796

Fix comment: coverage-map tests compile to LLVM IR, not MIR

71b7322

Zalathar force-pushed the instrument-coverage-ui branch from 6a3393c to 71b7322 Compare October 23, 2023 06:42

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 Oct 23, 2023

matthiaskrgr mentioned this pull request Oct 23, 2023

Rollup of 5 pull requests #117087

Merged

bors merged commit 46d7038 into rust-lang:master Oct 23, 2023
11 checks passed

rustbot added this to the 1.75.0 milestone Oct 23, 2023

Zalathar mentioned this pull request Oct 23, 2023

Introduce -C instrument-coverage=branch to gate branch coverage #116094

Merged

Zalathar deleted the instrument-coverage-ui branch October 23, 2023 21:54

github-actions bot mentioned this pull request Oct 31, 2023

Automated pull from upstream master ferrocene/ferrocene#74

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

coverage: Add UI tests for values accepted by `-Cinstrument-coverage` #117040

coverage: Add UI tests for values accepted by `-Cinstrument-coverage` #117040

Zalathar commented Oct 22, 2023

rustbot commented Oct 22, 2023

Zalathar commented Oct 22, 2023

Zalathar commented Oct 22, 2023

This comment has been minimized.

Zalathar Oct 22, 2023

This comment has been minimized.

Zalathar commented Oct 22, 2023

Zalathar Oct 22, 2023

Zalathar commented Oct 23, 2023

cjgillot commented Oct 23, 2023

bors commented Oct 23, 2023

coverage: Add UI tests for values accepted by -Cinstrument-coverage #117040

coverage: Add UI tests for values accepted by -Cinstrument-coverage #117040

Conversation

Zalathar commented Oct 22, 2023

rustbot commented Oct 22, 2023

Zalathar commented Oct 22, 2023

Zalathar commented Oct 22, 2023

This comment has been minimized.

Zalathar Oct 22, 2023

Choose a reason for hiding this comment

This comment has been minimized.

Zalathar commented Oct 22, 2023

Zalathar Oct 22, 2023

Choose a reason for hiding this comment

Zalathar commented Oct 23, 2023

cjgillot commented Oct 23, 2023

bors commented Oct 23, 2023

coverage: Add UI tests for values accepted by `-Cinstrument-coverage` #117040

coverage: Add UI tests for values accepted by `-Cinstrument-coverage` #117040