Skip to content

Commit b0c140a

Browse files
committed
Workaround for inconsistent order of llvm-cov results on Windows
1 parent d96f351 commit b0c140a

File tree

4 files changed

+49
-54
lines changed

4 files changed

+49
-54
lines changed

Diff for: src/test/run-make-fulldeps/coverage-reports/Makefile

+39
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,43 @@ ifeq ($(LLVM_COV_DEBUG), 1)
2222
DEBUG_FLAG=--debug
2323
endif
2424

25+
# FIXME(richkadel): I'm adding `--ignore-filename-regex=` line(s) for specific test(s) that produce
26+
# `llvm-cov` results for multiple files (for example `uses_crate.rs` and `used_crate/mod.rs`) as a
27+
# workaround for two problems causing tests to fail on Windows:
28+
#
29+
# 1. When multiple files appear in the `llvm-cov show` results, each file's coverage results can
30+
# appear in different a different order. Whether this is random or, somehow, platform-specific,
31+
# the Windows output flips the order of the files, compared to Linux. In the `uses_crate.rs`
32+
# test, the only test-unique (interesting) results we care about are the results for only one
33+
# of the two files, `mod/uses_crate.rs`, so the workaround is to ignore all but this one file.
34+
# In the future, we may want a more sophisticated solution that splits apart `llvm-cov show`
35+
# results into separate results files for each result (taking care not to create new file
36+
# paths that might be too long for Windows MAX_PATH limits when creating these new sub-results,
37+
# as well).
38+
# 2. When multiple files appear in the `llvm-cov show` results, the results for each file are
39+
# prefixed with their filename, including platform-specific path separators (`\` for Windows,
40+
# and `/` everywhere else). This could be filtered or normalized of course, but by ignoring
41+
# coverage results for all but one of the file, the filenames are no longer included anyway.
42+
# If this changes (if/when we decide to support `llvm-cov show` results for multiple files),
43+
# the file path separator differences may need to be addressed.
44+
#
45+
# Since this is only a workaround, I decided to implement the override by adding an option for
46+
# each file to be ignored, using a `--ignore-filename-regex=` entry for each one, rather than
47+
# implement some more sophisticated solution with a new custom test directive in the test file
48+
# itself (similar to `expect-exit-status`) because that would add a lot of complexity and still
49+
# be a workaround, with the same result, with no benefit.
50+
#
51+
# Yes these `--ignore-filename-regex=` options are included in all invocations of `llvm-cov show`
52+
# for now, but it is effectively ignored for all tests that don't include this file anyway.
53+
#
54+
# Note that it's also possible the `_counters.<test>.txt` and `<test>.json` files may order
55+
# results from multiple files inconsistently, which might also have to be accomodated if and when
56+
# we allow `llvm-cov` to produce results for multiple files. (The path separators appear to be
57+
# normalized to `/` in those files, thankfully.) But since we are ignoring results for all but one
58+
# file, this workaround addresses those potential issues as well.
59+
LLVM_COV_IGNORE_FILES=\
60+
--ignore-filename-regex=uses_crate.rs
61+
2562
# When generating `expected_*` results (using `x.py test --bless`), the `--debug` flag is forced.
2663
# If assertions are disabled, the command will fail with an error, rather than attempt to generate
2764
# only partial results.
@@ -76,6 +113,7 @@ endif
76113
# Generate a coverage report using `llvm-cov show`.
77114
"$(LLVM_BIN_DIR)"/llvm-cov show \
78115
$(DEBUG_FLAG) \
116+
$(LLVM_COV_IGNORE_FILES) \
79117
--Xdemangler="$(RUST_DEMANGLER)" \
80118
--show-line-counts-or-regions \
81119
--instr-profile="$(TMPDIR)"/[email protected] \
@@ -133,6 +171,7 @@ endif
133171
# Generate a coverage report in JSON, using `llvm-cov export`, and fail if
134172
# there are differences from the expected output.
135173
"$(LLVM_BIN_DIR)"/llvm-cov export \
174+
$(LLVM_COV_IGNORE_FILES) \
136175
--summary-only \
137176
--instr-profile="$(TMPDIR)"/[email protected] \
138177
$(call BIN,"$(TMPDIR)"/$@) \

Diff for: src/test/run-make-fulldeps/coverage-reports/expected_export_coverage.uses_crate.json

+10-36
Original file line numberDiff line numberDiff line change
@@ -27,55 +27,29 @@
2727
"percent": 37.5
2828
}
2929
}
30-
},
31-
{
32-
"filename": "../coverage/uses_crate.rs",
33-
"summary": {
34-
"functions": {
35-
"count": 1,
36-
"covered": 1,
37-
"percent": 100
38-
},
39-
"instantiations": {
40-
"count": 1,
41-
"covered": 1,
42-
"percent": 100
43-
},
44-
"lines": {
45-
"count": 6,
46-
"covered": 6,
47-
"percent": 100
48-
},
49-
"regions": {
50-
"count": 1,
51-
"covered": 1,
52-
"notcovered": 0,
53-
"percent": 100
54-
}
55-
}
5630
}
5731
],
5832
"totals": {
5933
"functions": {
60-
"count": 4,
61-
"covered": 4,
34+
"count": 3,
35+
"covered": 3,
6236
"percent": 100
6337
},
6438
"instantiations": {
65-
"count": 5,
66-
"covered": 5,
39+
"count": 4,
40+
"covered": 4,
6741
"percent": 100
6842
},
6943
"lines": {
70-
"count": 37,
71-
"covered": 20,
72-
"percent": 54.054054054054056
44+
"count": 31,
45+
"covered": 14,
46+
"percent": 45.16129032258064
7347
},
7448
"regions": {
75-
"count": 17,
76-
"covered": 7,
49+
"count": 16,
50+
"covered": 6,
7751
"notcovered": 10,
78-
"percent": 41.17647058823529
52+
"percent": 37.5
7953
}
8054
}
8155
}

Diff for: src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt

-13
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
../coverage/used_crate/mod.rs:
21
1| |#![allow(unused_assignments, unused_variables)]
32
2| |
43
3| |use std::fmt::Debug;
@@ -55,15 +54,3 @@
5554
42| 0| }
5655
43| 0|}
5756

58-
../coverage/uses_crate.rs:
59-
1| |#![allow(unused_assignments, unused_variables)]
60-
2| |
61-
3| |mod used_crate;
62-
4| |
63-
5| 1|fn main() {
64-
6| 1| used_crate::used_function();
65-
7| 1| let some_vec = vec![1, 2, 3, 4];
66-
8| 1| used_crate::used_generic_function(&some_vec);
67-
9| 1| used_crate::used_twice_generic_function(some_vec);
68-
10| 1|}
69-

Diff for: src/test/run-make-fulldeps/coverage-reports/expected_show_coverage_counters.uses_crate.txt

-5
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,3 @@ Combined regions:
7373
21:1 -> 23:2 (count=1)
7474
Segment at 21:1 (count = 1), RegionEntry
7575
Segment at 23:2 (count = 0), Skipped
76-
Emitting segments for file: ../coverage/uses_crate.rs
77-
Combined regions:
78-
5:1 -> 10:2 (count=1)
79-
Segment at 5:1 (count = 1), RegionEntry
80-
Segment at 10:2 (count = 0), Skipped

0 commit comments

Comments
 (0)