From b0c140a55b589680bbcd76d18fae9f411a7af0c5 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Wed, 2 Dec 2020 09:43:46 -0800 Subject: [PATCH] Workaround for inconsistent order of llvm-cov results on Windows --- .../coverage-reports/Makefile | 39 +++++++++++++++ .../expected_export_coverage.uses_crate.json | 48 +++++-------------- .../expected_show_coverage.uses_crate.txt | 13 ----- ...cted_show_coverage_counters.uses_crate.txt | 5 -- 4 files changed, 50 insertions(+), 55 deletions(-) diff --git a/src/test/run-make-fulldeps/coverage-reports/Makefile b/src/test/run-make-fulldeps/coverage-reports/Makefile index 0902ad74051f..91390f6cc02e 100644 --- a/src/test/run-make-fulldeps/coverage-reports/Makefile +++ b/src/test/run-make-fulldeps/coverage-reports/Makefile @@ -22,6 +22,43 @@ ifeq ($(LLVM_COV_DEBUG), 1) DEBUG_FLAG=--debug endif +# FIXME(richkadel): I'm adding `--ignore-filename-regex=` line(s) for specific test(s) that produce +# `llvm-cov` results for multiple files (for example `uses_crate.rs` and `used_crate/mod.rs`) as a +# workaround for two problems causing tests to fail on Windows: +# +# 1. When multiple files appear in the `llvm-cov show` results, each file's coverage results can +# appear in different a different order. Whether this is random or, somehow, platform-specific, +# the Windows output flips the order of the files, compared to Linux. In the `uses_crate.rs` +# test, the only test-unique (interesting) results we care about are the results for only one +# of the two files, `mod/uses_crate.rs`, so the workaround is to ignore all but this one file. +# In the future, we may want a more sophisticated solution that splits apart `llvm-cov show` +# results into separate results files for each result (taking care not to create new file +# paths that might be too long for Windows MAX_PATH limits when creating these new sub-results, +# as well). +# 2. When multiple files appear in the `llvm-cov show` results, the results for each file are +# prefixed with their filename, including platform-specific path separators (`\` for Windows, +# and `/` everywhere else). This could be filtered or normalized of course, but by ignoring +# coverage results for all but one of the file, the filenames are no longer included anyway. +# If this changes (if/when we decide to support `llvm-cov show` results for multiple files), +# the file path separator differences may need to be addressed. +# +# Since this is only a workaround, I decided to implement the override by adding an option for +# each file to be ignored, using a `--ignore-filename-regex=` entry for each one, rather than +# implement some more sophisticated solution with a new custom test directive in the test file +# itself (similar to `expect-exit-status`) because that would add a lot of complexity and still +# be a workaround, with the same result, with no benefit. +# +# Yes these `--ignore-filename-regex=` options are included in all invocations of `llvm-cov show` +# for now, but it is effectively ignored for all tests that don't include this file anyway. +# +# Note that it's also possible the `_counters..txt` and `.json` files may order +# results from multiple files inconsistently, which might also have to be accomodated if and when +# we allow `llvm-cov` to produce results for multiple files. (The path separators appear to be +# normalized to `/` in those files, thankfully.) But since we are ignoring results for all but one +# file, this workaround addresses those potential issues as well. +LLVM_COV_IGNORE_FILES=\ + --ignore-filename-regex=uses_crate.rs + # When generating `expected_*` results (using `x.py test --bless`), the `--debug` flag is forced. # If assertions are disabled, the command will fail with an error, rather than attempt to generate # only partial results. @@ -76,6 +113,7 @@ endif # Generate a coverage report using `llvm-cov show`. "$(LLVM_BIN_DIR)"/llvm-cov show \ $(DEBUG_FLAG) \ + $(LLVM_COV_IGNORE_FILES) \ --Xdemangler="$(RUST_DEMANGLER)" \ --show-line-counts-or-regions \ --instr-profile="$(TMPDIR)"/$@.profdata \ @@ -133,6 +171,7 @@ endif # Generate a coverage report in JSON, using `llvm-cov export`, and fail if # there are differences from the expected output. "$(LLVM_BIN_DIR)"/llvm-cov export \ + $(LLVM_COV_IGNORE_FILES) \ --summary-only \ --instr-profile="$(TMPDIR)"/$@.profdata \ $(call BIN,"$(TMPDIR)"/$@) \ diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_export_coverage.uses_crate.json b/src/test/run-make-fulldeps/coverage-reports/expected_export_coverage.uses_crate.json index 6189c7b7b641..fc0d7db7c299 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_export_coverage.uses_crate.json +++ b/src/test/run-make-fulldeps/coverage-reports/expected_export_coverage.uses_crate.json @@ -27,55 +27,29 @@ "percent": 37.5 } } - }, - { - "filename": "../coverage/uses_crate.rs", - "summary": { - "functions": { - "count": 1, - "covered": 1, - "percent": 100 - }, - "instantiations": { - "count": 1, - "covered": 1, - "percent": 100 - }, - "lines": { - "count": 6, - "covered": 6, - "percent": 100 - }, - "regions": { - "count": 1, - "covered": 1, - "notcovered": 0, - "percent": 100 - } - } } ], "totals": { "functions": { + "count": 3, + "covered": 3, + "percent": 100 + }, + "instantiations": { "count": 4, "covered": 4, "percent": 100 }, - "instantiations": { - "count": 5, - "covered": 5, - "percent": 100 - }, "lines": { - "count": 37, - "covered": 20, - "percent": 54.054054054054056 + "count": 31, + "covered": 14, + "percent": 45.16129032258064 }, "regions": { - "count": 17, - "covered": 7, + "count": 16, + "covered": 6, "notcovered": 10, - "percent": 41.17647058823529 + "percent": 37.5 } } } diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt index 81b355a29acf..ad1bdb5d6a54 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt @@ -1,4 +1,3 @@ -../coverage/used_crate/mod.rs: 1| |#![allow(unused_assignments, unused_variables)] 2| | 3| |use std::fmt::Debug; @@ -55,15 +54,3 @@ 42| 0| } 43| 0|} -../coverage/uses_crate.rs: - 1| |#![allow(unused_assignments, unused_variables)] - 2| | - 3| |mod used_crate; - 4| | - 5| 1|fn main() { - 6| 1| used_crate::used_function(); - 7| 1| let some_vec = vec![1, 2, 3, 4]; - 8| 1| used_crate::used_generic_function(&some_vec); - 9| 1| used_crate::used_twice_generic_function(some_vec); - 10| 1|} - diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage_counters.uses_crate.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage_counters.uses_crate.txt index 8901019f1910..e8be1e685e65 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage_counters.uses_crate.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage_counters.uses_crate.txt @@ -73,8 +73,3 @@ Combined regions: 21:1 -> 23:2 (count=1) Segment at 21:1 (count = 1), RegionEntry Segment at 23:2 (count = 0), Skipped -Emitting segments for file: ../coverage/uses_crate.rs -Combined regions: - 5:1 -> 10:2 (count=1) -Segment at 5:1 (count = 1), RegionEntry -Segment at 10:2 (count = 0), Skipped