Skip to content

Commit f44c227

Browse files
authored
Rollup merge of rust-lang#80109 - richkadel:llvm-coverage-counters-2.3.0, r=tmandry
Remove redundant and unreliable coverage test results The `coverage-reports` tests still generate counters and JSON reports for inspection, but these files are no longer used in Makefile diffs, to reduce complexity and confusion from unreliable or unexpected test results, especially when maintaining them (i.e., generating `--bless`ed results). The associated `expected_` files for counters and JSON reports have been removed, leaving only the files actually used for testing: the `llvm-cov show` reports. r? `@tmandry` Tyler - as we discussed offline... FYI: `@wesleywiser` `@Swatinem` Arpad, depending on the timing of this PR, it may not affect you, but I'm removing some of the files that produce slightly different results on Windows as they really aren't necessary to validate coverage results.
2 parents da89dbb + c9fab50 commit f44c227

File tree

56 files changed

+86
-3576
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

56 files changed

+86
-3576
lines changed

src/test/run-make-fulldeps/coverage-reports/Makefile

+86-49
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,13 @@
99
BASEDIR=../coverage-reports
1010
SOURCEDIR=../coverage
1111

12-
# The `llvm-cov show` flag `--debug`, used to generate the `counters` output files, is only enabled
13-
# if LLVM assertions are enabled. Requires Rust config `llvm/optimize` and not
12+
# The `llvm-cov show` flag `--debug`, used to generate the `counters` output files, is only
13+
# enabled if LLVM assertions are enabled. This requires Rust config `llvm/optimize` and not
1414
# `llvm/release_debuginfo`. Note that some CI builds disable debug assertions (by setting
15-
# `NO_LLVM_ASSERTIONS=1`), so it is not OK to fail the test, but `bless`ed test results cannot be
16-
# generated without debug assertions.
15+
# `NO_LLVM_ASSERTIONS=1`), so the tests must still pass even if the `--debug` flag is
16+
# not supported. (Note that `counters` files are only produced in the `$(TMPDIR)`
17+
# directory, for inspection and debugging support. They are *not* copied to `expected_*`
18+
# files when `--bless`ed.)
1719
LLVM_COV_DEBUG := $(shell \
1820
"$(LLVM_BIN_DIR)"/llvm-cov show --debug 2>&1 | \
1921
grep -q "Unknown command line argument '--debug'"; \
@@ -51,11 +53,10 @@ endif
5153
# Yes these `--ignore-filename-regex=` options are included in all invocations of `llvm-cov show`
5254
# for now, but it is effectively ignored for all tests that don't include this file anyway.
5355
#
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.
56+
# (Note that it's also possible the `_counters.<test>.txt` and `<test>.json` files (if generated)
57+
# may order results from multiple files inconsistently, which might also have to be accomodated
58+
# if and when we allow `llvm-cov` to produce results for multiple files. Note, the path separators
59+
# appear to be normalized to `/` in those files, thankfully.)
5960
LLVM_COV_IGNORE_FILES=\
6061
--ignore-filename-regex=uses_crate.rs
6162

@@ -77,9 +78,7 @@ endif
7778
.PHONY: clear_expected_if_blessed
7879
clear_expected_if_blessed:
7980
ifdef RUSTC_BLESS_TEST
80-
rm -f expected_export_coverage.*.json
81-
rm -f expected_show_coverage.*.txt
82-
rm -f expected_show_coverage_counters.*.txt
81+
rm -f expected_*
8382
endif
8483

8584
-include clear_expected_if_blessed
@@ -140,12 +139,8 @@ endif
140139
ifdef RUSTC_BLESS_TEST
141140
cp "$(TMPDIR)"/actual_show_coverage.$@.txt \
142141
expected_show_coverage.$@.txt
143-
cp "$(TMPDIR)"/actual_show_coverage_counters.$@.txt \
144-
expected_show_coverage_counters.$@.txt
145142
else
146-
# Compare the show coverage output (`--bless` refreshes `typical` files)
147-
# Note `llvm-cov show` output for some programs can vary, but can be ignored
148-
# by inserting `// ignore-llvm-cov-show-diffs` at the top of the source file.
143+
# Compare the show coverage output (`--bless` refreshes `typical` files).
149144
#
150145
# FIXME(richkadel): None of the Rust test source samples have the
151146
# `// ignore-llvm-cov-show-diffs` anymore. This directive exists to work around a limitation
@@ -158,8 +153,10 @@ else
158153
#
159154
# This workaround only works if the coverage counts are identical across all reported
160155
# instantiations. If there is no way to ensure this, you may need to apply the
161-
# `// ignore-llvm-cov-show-diffs` directive, and rely on the `.json` and counter
162-
# files for validating results have not changed.
156+
# `// ignore-llvm-cov-show-diffs` directive, and check for differences using the
157+
# `.json` files to validate that results have not changed. (Until then, the JSON
158+
# files are redundant, so there is no need to generate `expected_*.json` files or
159+
# compare actual JSON results.)
163160

164161
$(DIFF) --ignore-matching-lines='::<.*>.*:$$' \
165162
expected_show_coverage.$@.txt "$(TMPDIR)"/actual_show_coverage.$@.txt || \
@@ -169,37 +166,77 @@ else
169166
( >&2 echo 'diff failed, and not suppressed without `// ignore-llvm-cov-show-diffs` in $(SOURCEDIR)/$@.rs'; \
170167
false \
171168
)
172-
173-
ifdef DEBUG_FLAG
174-
$(DIFF) expected_show_coverage_counters.$@.txt "$(TMPDIR)"/actual_show_coverage_counters.$@.txt || \
175-
( grep -q '^\/\/ ignore-llvm-cov-show-diffs' $(SOURCEDIR)/$@.rs && \
176-
>&2 echo 'diff failed, but suppressed with `// ignore-llvm-cov-show-diffs` in $(SOURCEDIR)/$@.rs' \
177-
) || \
178-
( >&2 echo 'diff failed, and not suppressed without `// ignore-llvm-cov-show-diffs` in $(SOURCEDIR)/$@.rs'; \
179-
>&2 echo '(Ignore anyway until mangled function names in "counters" files are demangled.)' \
180-
)
181-
182-
# FIXME(richkadel): Apply the demangler to the `*_show_coverage_counters.*.txt` files,
183-
# so the crate disambiguator differences will be stripped away. At that point, these files
184-
# will be less likely to vary, and the last `echo` above (starting with "Ignore anyway")
185-
# can be replaced with `false` to fail the test.
186169
endif
187170

188-
endif
171+
####################################################################################################
189172

190-
# Generate a coverage report in JSON, using `llvm-cov export`, and fail if
191-
# there are differences from the expected output.
192-
"$(LLVM_BIN_DIR)"/llvm-cov export \
193-
$(LLVM_COV_IGNORE_FILES) \
194-
--summary-only \
195-
--instr-profile="$(TMPDIR)"/$@.profdata \
196-
$(call BIN,"$(TMPDIR)"/$@) \
197-
| "$(PYTHON)" $(BASEDIR)/prettify_json.py \
198-
> "$(TMPDIR)"/actual_export_coverage.$@.json
173+
# The following Makefile content was used to copy the generated `counters` files
174+
# to `expected_` files (when `--bless`ed) and to compare them via `diff`; but for
175+
# multiple reasons, these files cannot easily be used for test validation:
176+
#
177+
# * Output lines can be produced in non-deterministic order (depending on the
178+
# target platform, and sometimes on unrelated codegen changes).
179+
# * Some lines include demangled function names, making them more challenging
180+
# to interpret and compare.
181+
#
182+
# The files are still generated (in `$(TMPDIR)`) to support developers wanting
183+
# to inspect the counters, for debugging purposes.
184+
#
185+
# ifdef RUSTC_BLESS_TEST
186+
# cp "$(TMPDIR)"/actual_show_coverage_counters.$@.txt \
187+
# expected_show_coverage_counters.$@.txt
188+
# else
189+
#
190+
# ifdef DEBUG_FLAG
191+
# $(DIFF) expected_show_coverage_counters.$@.txt "$(TMPDIR)"/actual_show_coverage_counters.$@.txt || \
192+
# ( grep -q '^\/\/ ignore-llvm-cov-show-diffs' $(SOURCEDIR)/$@.rs && \
193+
# >&2 echo 'diff failed, but suppressed with `// ignore-llvm-cov-show-diffs` in $(SOURCEDIR)/$@.rs' \
194+
# ) || \
195+
# ( >&2 echo 'diff failed, and not suppressed without `// ignore-llvm-cov-show-diffs` in $(SOURCEDIR)/$@.rs'; \
196+
# >&2 echo '(Ignore anyway until mangled function names in "counters" files are demangled.)' \
197+
# )
198+
# endif
199+
#
200+
# endif
199201

200-
ifdef RUSTC_BLESS_TEST
201-
cp "$(TMPDIR)"/actual_export_coverage.$@.json expected_export_coverage.$@.json
202-
else
203-
# Check that exported JSON coverage data matches what we expect (`--bless` refreshes `expected`)
204-
$(DIFF) expected_export_coverage.$@.json "$(TMPDIR)"/actual_export_coverage.$@.json
205-
endif
202+
####################################################################################################
203+
204+
# The following Makefile content, and short JSON script, were used to generate
205+
# coverage reports in JSON when the `llvm-cov show` reports were less reliable for
206+
# testing. At the present time, however, the `llvm-cov show` results, and methods
207+
# for comparing them, are working for all tests, making the JSON reports redundant.
208+
#
209+
# If this changes in the future, the scripts are left here, commented out, but can
210+
# be resurrected if desired. This could be used to compare *only* the JSON files;
211+
# and in that case, the `llvm-cov show` reports can be ignored by inserting
212+
# `// ignore-llvm-cov-show-diffs` at the top of the source file.
213+
#
214+
# # Generate a coverage report in JSON, using `llvm-cov export`, and fail if
215+
# # there are differences from the expected output.
216+
# "$(LLVM_BIN_DIR)"/llvm-cov export \
217+
# $(LLVM_COV_IGNORE_FILES) \
218+
# --summary-only \
219+
# --instr-profile="$(TMPDIR)"/$@.profdata \
220+
# $(call BIN,"$(TMPDIR)"/$@) \
221+
# | "$(PYTHON)" $(BASEDIR)/prettify_json.py \
222+
# > "$(TMPDIR)"/actual_export_coverage.$@.json
223+
#
224+
# ifdef RUSTC_BLESS_TEST
225+
# cp "$(TMPDIR)"/actual_export_coverage.$@.json expected_export_coverage.$@.json
226+
# else
227+
# # Check that exported JSON coverage data matches what we expect (`--bless` refreshes `expected`)
228+
# $(DIFF) expected_export_coverage.$@.json "$(TMPDIR)"/actual_export_coverage.$@.json
229+
# endif
230+
#
231+
# # # If generating coverage reports in JSON, this Makefile is accompanied by
232+
# # # a Python script, `prettify_json.py`, which is defined:
233+
# #
234+
# # #!/usr/bin/env python
235+
# #
236+
# # import sys
237+
# # import json
238+
# #
239+
# # # Try to decode line in order to ensure it is a valid JSON document
240+
# # for line in sys.stdin:
241+
# # parsed = json.loads(line)
242+
# # print (json.dumps(parsed, indent=2, separators=(',', ': '), sort_keys=True))

src/test/run-make-fulldeps/coverage-reports/expected_export_coverage.abort.json

-59
This file was deleted.

src/test/run-make-fulldeps/coverage-reports/expected_export_coverage.assert.json

-59
This file was deleted.

src/test/run-make-fulldeps/coverage-reports/expected_export_coverage.async.json

-59
This file was deleted.

0 commit comments

Comments
 (0)