Skip to content

Commit

Permalink
Auto merge of #114875 - Zalathar:line-numbers, r=ozkanonur
Browse files Browse the repository at this point in the history
coverage: Anonymize line numbers in `run-coverage` test snapshots

LLVM's coverage reporter always prints line numbers in its coverage reports.

For testing purposes this is slightly inconvenient, because it means that adding or removing a line in a test file causes all subsequent lines in the snapshot to change. That makes it harder to see the actually meaningful changes in the re-blessed snapshot.

---

This change fixes that by adding another normalization pass that replaces all line numbers in the coverage reports with `LL`, which is similar to what UI tests tell the compiler to do when emitting line numbers in error messages.
  • Loading branch information
bors committed Aug 17, 2023
2 parents c5833f1 + bfb1654 commit aa864a7
Show file tree
Hide file tree
Showing 46 changed files with 2,672 additions and 2,655 deletions.
17 changes: 17 additions & 0 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use crate::ColorConfig;
use regex::{Captures, Regex};
use rustfix::{apply_suggestions, get_suggestions_from_json, Filter};

use std::borrow::Cow;
use std::collections::hash_map::DefaultHasher;
use std::collections::{HashMap, HashSet};
use std::env;
Expand Down Expand Up @@ -664,6 +665,7 @@ impl<'test> TestCx<'test> {

fn normalize_coverage_output(&self, coverage: &str) -> Result<String, String> {
let normalized = self.normalize_output(coverage, &[]);
let normalized = Self::anonymize_coverage_line_numbers(&normalized);

let mut lines = normalized.lines().collect::<Vec<_>>();

Expand All @@ -674,6 +676,21 @@ impl<'test> TestCx<'test> {
Ok(joined_lines)
}

/// Replace line numbers in coverage reports with the placeholder `LL`,
/// so that the tests are less sensitive to lines being added/removed.
fn anonymize_coverage_line_numbers(coverage: &str) -> Cow<'_, str> {
// The coverage reporter prints line numbers at the start of a line.
// They are truncated or left-padded to occupy exactly 5 columns.
// (`LineNumberColumnWidth` in `SourceCoverageViewText.cpp`.)
// A pipe character `|` appears immediately after the final digit.
//
// Line numbers that appear inside expansion/instantiation subviews
// have an additional prefix of ` |` for each nesting level.
static LINE_NUMBER_RE: Lazy<Regex> =
Lazy::new(|| Regex::new(r"(?m:^)(?<prefix>(?: \|)*) *[0-9]+\|").unwrap());
LINE_NUMBER_RE.replace_all(coverage, "$prefix LL|")
}

/// Coverage reports can describe multiple source files, separated by
/// blank lines. The order of these files is unpredictable (since it
/// depends on implementation details), so we need to sort the file
Expand Down
216 changes: 108 additions & 108 deletions tests/run-coverage-rustdoc/doctest.coverage
Original file line number Diff line number Diff line change
@@ -1,115 +1,115 @@
$DIR/auxiliary/doctest_crate.rs:
1| |/// A function run only from within doctests
2| 3|pub fn fn_run_in_doctests(conditional: usize) {
3| 3| match conditional {
4| 1| 1 => assert_eq!(1, 1), // this is run,
5| 1| 2 => assert_eq!(1, 1), // this,
6| 1| 3 => assert_eq!(1, 1), // and this too
7| 0| _ => assert_eq!(1, 2), // however this is not
8| | }
9| 3|}
LL| |/// A function run only from within doctests
LL| 3|pub fn fn_run_in_doctests(conditional: usize) {
LL| 3| match conditional {
LL| 1| 1 => assert_eq!(1, 1), // this is run,
LL| 1| 2 => assert_eq!(1, 1), // this,
LL| 1| 3 => assert_eq!(1, 1), // and this too
LL| 0| _ => assert_eq!(1, 2), // however this is not
LL| | }
LL| 3|}

$DIR/doctest.rs:
1| |//! This test ensures that code from doctests is properly re-mapped.
2| |//! See <https://github.com/rust-lang/rust/issues/79417> for more info.
3| |//!
4| |//! Just some random code:
5| 1|//! ```
6| 1|//! if true {
7| |//! // this is executed!
8| 1|//! assert_eq!(1, 1);
9| |//! } else {
10| |//! // this is not!
11| 0|//! assert_eq!(1, 2);
12| |//! }
13| 1|//! ```
14| |//!
15| |//! doctest testing external code:
16| |//! ```
17| 1|//! extern crate doctest_crate;
18| 1|//! doctest_crate::fn_run_in_doctests(1);
19| 1|//! ```
20| |//!
21| |//! doctest returning a result:
22| 1|//! ```
23| 2|//! #[derive(Debug, PartialEq)]
LL| |//! This test ensures that code from doctests is properly re-mapped.
LL| |//! See <https://github.com/rust-lang/rust/issues/79417> for more info.
LL| |//!
LL| |//! Just some random code:
LL| 1|//! ```
LL| 1|//! if true {
LL| |//! // this is executed!
LL| 1|//! assert_eq!(1, 1);
LL| |//! } else {
LL| |//! // this is not!
LL| 0|//! assert_eq!(1, 2);
LL| |//! }
LL| 1|//! ```
LL| |//!
LL| |//! doctest testing external code:
LL| |//! ```
LL| 1|//! extern crate doctest_crate;
LL| 1|//! doctest_crate::fn_run_in_doctests(1);
LL| 1|//! ```
LL| |//!
LL| |//! doctest returning a result:
LL| 1|//! ```
LL| 2|//! #[derive(Debug, PartialEq)]
^1
24| 1|//! struct SomeError {
25| 1|//! msg: String,
26| 1|//! }
27| 1|//! let mut res = Err(SomeError { msg: String::from("a message") });
28| 1|//! if res.is_ok() {
29| 0|//! res?;
30| |//! } else {
31| 1|//! if *res.as_ref().unwrap_err() == *res.as_ref().unwrap_err() {
32| 1|//! println!("{:?}", res);
33| 1|//! }
LL| 1|//! struct SomeError {
LL| 1|//! msg: String,
LL| 1|//! }
LL| 1|//! let mut res = Err(SomeError { msg: String::from("a message") });
LL| 1|//! if res.is_ok() {
LL| 0|//! res?;
LL| |//! } else {
LL| 1|//! if *res.as_ref().unwrap_err() == *res.as_ref().unwrap_err() {
LL| 1|//! println!("{:?}", res);
LL| 1|//! }
^0
34| 1|//! if *res.as_ref().unwrap_err() == *res.as_ref().unwrap_err() {
35| 1|//! res = Ok(1);
36| 1|//! }
LL| 1|//! if *res.as_ref().unwrap_err() == *res.as_ref().unwrap_err() {
LL| 1|//! res = Ok(1);
LL| 1|//! }
^0
37| 1|//! res = Ok(0);
38| |//! }
39| |//! // need to be explicit because rustdoc cant infer the return type
40| 1|//! Ok::<(), SomeError>(())
41| 1|//! ```
42| |//!
43| |//! doctest with custom main:
44| |//! ```
45| 1|//! fn some_func() {
46| 1|//! println!("called some_func()");
47| 1|//! }
48| |//!
49| 0|//! #[derive(Debug)]
50| |//! struct SomeError;
51| |//!
52| |//! extern crate doctest_crate;
53| |//!
54| 1|//! fn doctest_main() -> Result<(), SomeError> {
55| 1|//! some_func();
56| 1|//! doctest_crate::fn_run_in_doctests(2);
57| 1|//! Ok(())
58| 1|//! }
59| |//!
60| |//! // this `main` is not shown as covered, as it clashes with all the other
61| |//! // `main` functions that were automatically generated for doctests
62| |//! fn main() -> Result<(), SomeError> {
63| |//! doctest_main()
64| |//! }
65| |//! ```
66| |// aux-build:doctest_crate.rs
67| |/// doctest attached to fn testing external code:
68| |/// ```
69| 1|/// extern crate doctest_crate;
70| 1|/// doctest_crate::fn_run_in_doctests(3);
71| 1|/// ```
72| |///
73| 1|fn main() {
74| 1| if true {
75| 1| assert_eq!(1, 1);
76| | } else {
77| 0| assert_eq!(1, 2);
78| | }
79| 1|}
80| |
81| |// FIXME(Swatinem): Fix known issue that coverage code region columns need to be offset by the
82| |// doc comment line prefix (`///` or `//!`) and any additional indent (before or after the doc
83| |// comment characters). This test produces `llvm-cov show` results demonstrating the problem.
84| |//
85| |// One of the above tests now includes: `derive(Debug, PartialEq)`, producing an `llvm-cov show`
86| |// result with a distinct count for `Debug`, denoted by `^1`, but the caret points to the wrong
87| |// column. Similarly, the `if` blocks without `else` blocks show `^0`, which should point at, or
88| |// one character past, the `if` block's closing brace. In both cases, these are most likely off
89| |// by the number of characters stripped from the beginning of each doc comment line: indent
90| |// whitespace, if any, doc comment prefix (`//!` in this case) and (I assume) one space character
91| |// (?). Note, when viewing `llvm-cov show` results in `--color` mode, the column offset errors are
92| |// more pronounced, and show up in more places, with background color used to show some distinct
93| |// code regions with different coverage counts.
94| |//
95| |// NOTE: Since the doc comment line prefix may vary, one possible solution is to replace each
96| |// character stripped from the beginning of doc comment lines with a space. This will give coverage
97| |// results the correct column offsets, and I think it should compile correctly, but I don't know
98| |// what affect it might have on diagnostic messages from the compiler, and whether anyone would care
99| |// if the indentation changed. I don't know if there is a more viable solution.
LL| 1|//! res = Ok(0);
LL| |//! }
LL| |//! // need to be explicit because rustdoc cant infer the return type
LL| 1|//! Ok::<(), SomeError>(())
LL| 1|//! ```
LL| |//!
LL| |//! doctest with custom main:
LL| |//! ```
LL| 1|//! fn some_func() {
LL| 1|//! println!("called some_func()");
LL| 1|//! }
LL| |//!
LL| 0|//! #[derive(Debug)]
LL| |//! struct SomeError;
LL| |//!
LL| |//! extern crate doctest_crate;
LL| |//!
LL| 1|//! fn doctest_main() -> Result<(), SomeError> {
LL| 1|//! some_func();
LL| 1|//! doctest_crate::fn_run_in_doctests(2);
LL| 1|//! Ok(())
LL| 1|//! }
LL| |//!
LL| |//! // this `main` is not shown as covered, as it clashes with all the other
LL| |//! // `main` functions that were automatically generated for doctests
LL| |//! fn main() -> Result<(), SomeError> {
LL| |//! doctest_main()
LL| |//! }
LL| |//! ```
LL| |// aux-build:doctest_crate.rs
LL| |/// doctest attached to fn testing external code:
LL| |/// ```
LL| 1|/// extern crate doctest_crate;
LL| 1|/// doctest_crate::fn_run_in_doctests(3);
LL| 1|/// ```
LL| |///
LL| 1|fn main() {
LL| 1| if true {
LL| 1| assert_eq!(1, 1);
LL| | } else {
LL| 0| assert_eq!(1, 2);
LL| | }
LL| 1|}
LL| |
LL| |// FIXME(Swatinem): Fix known issue that coverage code region columns need to be offset by the
LL| |// doc comment line prefix (`///` or `//!`) and any additional indent (before or after the doc
LL| |// comment characters). This test produces `llvm-cov show` results demonstrating the problem.
LL| |//
LL| |// One of the above tests now includes: `derive(Debug, PartialEq)`, producing an `llvm-cov show`
LL| |// result with a distinct count for `Debug`, denoted by `^1`, but the caret points to the wrong
LL| |// column. Similarly, the `if` blocks without `else` blocks show `^0`, which should point at, or
LL| |// one character past, the `if` block's closing brace. In both cases, these are most likely off
LL| |// by the number of characters stripped from the beginning of each doc comment line: indent
LL| |// whitespace, if any, doc comment prefix (`//!` in this case) and (I assume) one space character
LL| |// (?). Note, when viewing `llvm-cov show` results in `--color` mode, the column offset errors are
LL| |// more pronounced, and show up in more places, with background color used to show some distinct
LL| |// code regions with different coverage counts.
LL| |//
LL| |// NOTE: Since the doc comment line prefix may vary, one possible solution is to replace each
LL| |// character stripped from the beginning of doc comment lines with a space. This will give coverage
LL| |// results the correct column offsets, and I think it should compile correctly, but I don't know
LL| |// what affect it might have on diagnostic messages from the compiler, and whether anyone would care
LL| |// if the indentation changed. I don't know if there is a more viable solution.

132 changes: 66 additions & 66 deletions tests/run-coverage/abort.coverage
Original file line number Diff line number Diff line change
@@ -1,69 +1,69 @@
1| |#![feature(c_unwind)]
2| |#![allow(unused_assignments)]
3| |
4| 12|extern "C" fn might_abort(should_abort: bool) {
5| 12| if should_abort {
6| 0| println!("aborting...");
7| 0| panic!("panics and aborts");
8| 12| } else {
9| 12| println!("Don't Panic");
10| 12| }
11| 12|}
12| |
13| 1|fn main() -> Result<(), u8> {
14| 1| let mut countdown = 10;
15| 11| while countdown > 0 {
16| 10| if countdown < 5 {
17| 4| might_abort(false);
18| 6| }
19| | // See discussion (below the `Notes` section) on coverage results for the closing brace.
20| 10| if countdown < 5 { might_abort(false); } // Counts for different regions on one line.
LL| |#![feature(c_unwind)]
LL| |#![allow(unused_assignments)]
LL| |
LL| 12|extern "C" fn might_abort(should_abort: bool) {
LL| 12| if should_abort {
LL| 0| println!("aborting...");
LL| 0| panic!("panics and aborts");
LL| 12| } else {
LL| 12| println!("Don't Panic");
LL| 12| }
LL| 12|}
LL| |
LL| 1|fn main() -> Result<(), u8> {
LL| 1| let mut countdown = 10;
LL| 11| while countdown > 0 {
LL| 10| if countdown < 5 {
LL| 4| might_abort(false);
LL| 6| }
LL| | // See discussion (below the `Notes` section) on coverage results for the closing brace.
LL| 10| if countdown < 5 { might_abort(false); } // Counts for different regions on one line.
^4 ^6
21| | // For the following example, the closing brace is the last character on the line.
22| | // This shows the character after the closing brace is highlighted, even if that next
23| | // character is a newline.
24| 10| if countdown < 5 { might_abort(false); }
LL| | // For the following example, the closing brace is the last character on the line.
LL| | // This shows the character after the closing brace is highlighted, even if that next
LL| | // character is a newline.
LL| 10| if countdown < 5 { might_abort(false); }
^4 ^6
25| 10| countdown -= 1;
26| | }
27| 1| Ok(())
28| 1|}
29| |
30| |// Notes:
31| |// 1. Compare this program and its coverage results to those of the similar tests
32| |// `panic_unwind.rs` and `try_error_result.rs`.
33| |// 2. This test confirms the coverage generated when a program includes `UnwindAction::Terminate`.
34| |// 3. The test does not invoke the abort. By executing to a successful completion, the coverage
35| |// results show where the program did and did not execute.
36| |// 4. If the program actually aborted, the coverage counters would not be saved (which "works as
37| |// intended"). Coverage results would show no executed coverage regions.
38| |// 6. If `should_abort` is `true` and the program aborts, the program exits with a `132` status
39| |// (on Linux at least).
40| |
41| |/*
42| |
43| |Expect the following coverage results:
44| |
45| |```text
46| | 16| 11| while countdown > 0 {
47| | 17| 10| if countdown < 5 {
48| | 18| 4| might_abort(false);
49| | 19| 6| }
50| |```
51| |
52| |This is actually correct.
53| |
54| |The condition `countdown < 5` executed 10 times (10 loop iterations).
55| |
56| |It evaluated to `true` 4 times, and executed the `might_abort()` call.
57| |
58| |It skipped the body of the `might_abort()` call 6 times. If an `if` does not include an explicit
59| |`else`, the coverage implementation injects a counter, at the character immediately after the `if`s
60| |closing brace, to count the "implicit" `else`. This is the only way to capture the coverage of the
61| |non-true condition.
62| |
63| |As another example of why this is important, say the condition was `countdown < 50`, which is always
64| |`true`. In that case, we wouldn't have a test for what happens if `might_abort()` is not called.
65| |The closing brace would have a count of `0`, highlighting the missed coverage.
66| |*/
LL| 10| countdown -= 1;
LL| | }
LL| 1| Ok(())
LL| 1|}
LL| |
LL| |// Notes:
LL| |// 1. Compare this program and its coverage results to those of the similar tests
LL| |// `panic_unwind.rs` and `try_error_result.rs`.
LL| |// 2. This test confirms the coverage generated when a program includes `UnwindAction::Terminate`.
LL| |// 3. The test does not invoke the abort. By executing to a successful completion, the coverage
LL| |// results show where the program did and did not execute.
LL| |// 4. If the program actually aborted, the coverage counters would not be saved (which "works as
LL| |// intended"). Coverage results would show no executed coverage regions.
LL| |// 6. If `should_abort` is `true` and the program aborts, the program exits with a `132` status
LL| |// (on Linux at least).
LL| |
LL| |/*
LL| |
LL| |Expect the following coverage results:
LL| |
LL| |```text
LL| | 16| 11| while countdown > 0 {
LL| | 17| 10| if countdown < 5 {
LL| | 18| 4| might_abort(false);
LL| | 19| 6| }
LL| |```
LL| |
LL| |This is actually correct.
LL| |
LL| |The condition `countdown < 5` executed 10 times (10 loop iterations).
LL| |
LL| |It evaluated to `true` 4 times, and executed the `might_abort()` call.
LL| |
LL| |It skipped the body of the `might_abort()` call 6 times. If an `if` does not include an explicit
LL| |`else`, the coverage implementation injects a counter, at the character immediately after the `if`s
LL| |closing brace, to count the "implicit" `else`. This is the only way to capture the coverage of the
LL| |non-true condition.
LL| |
LL| |As another example of why this is important, say the condition was `countdown < 50`, which is always
LL| |`true`. In that case, we wouldn't have a test for what happens if `might_abort()` is not called.
LL| |The closing brace would have a count of `0`, highlighting the missed coverage.
LL| |*/

Loading

0 comments on commit aa864a7

Please sign in to comment.