Skip to content

Fix and re-enable two coverage tests on MacOS #78888

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 13, 2020
+23 −35
Merged

Fix and re-enable two coverage tests on MacOS #78888

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions src/test/run-make-fulldeps/coverage-reports-base/Makefile
View file
Original file line number Diff line number Diff line change
@@ -13,15 +13,13 @@
BASEDIR=../coverage-reports-base
SOURCEDIR=../coverage

ifeq ($(UNAME),Darwin)
# FIXME(richkadel): It appears that --debug is not available on MacOS even when not running
# under CI.
NO_LLVM_ASSERTIONS=1
endif

# The `llvm-cov show` flag `--debug`, used to generate the `counters` output files, is only enabled
# if LLVM assertions are enabled. Some CI builds disable debug assertions.
ifndef NO_LLVM_ASSERTIONS
# if LLVM assertions are enabled. Requires Rust config `llvm/optimize` and not
# `llvm/release_debuginfo`. Note that some CI builds disable debug assertions (by setting
# `NO_LLVM_ASSERTIONS=1`), so it is not OK to fail the test, but `bless`ed test results cannot be
# generated without debug assertions.
Comment on lines 16 to +20
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we can consider this like tests which only run on certain platforms, except it only runs in certain configurations. It's not ideal, because if someone makes a large change that affects these tests and isn't in the right configuration, they won't be able to update the tests.

That being said, I'm not sure what we can do differently. I wish there was an LLVM build option to enable only this flag.

One mitigation is to add a warning if someone tries to bless and we can't update all the files. (To keep it from being too noisy.. Maybe only print it if they specify bless and one of the other files doesn't match? This kind of logic is difficult in a Makefile which is another reason I think we should move away from those.)

LLVM_COV_DEBUG := $(shell "$(LLVM_BIN_DIR)"/llvm-cov show --debug 2>&1 | grep -q "Unknown command line argument '--debug'"; echo $$?)
ifeq ($(LLVM_COV_DEBUG), 1)
DEBUG_FLAG=--debug
endif

44 changes: 17 additions & 27 deletions src/test/run-make-fulldeps/coverage-spanview-base/Makefile
View file
Original file line number Diff line number Diff line change
@@ -9,9 +9,20 @@
BASEDIR=../coverage-spanview-base
SOURCEDIR=../coverage

ifeq ($(UNAME),Darwin)
SED_HAS_ISSUES=1
endif
define SPANVIEW_HEADER
<!DOCTYPE html>
<!--

Preview this file as rendered HTML from the github source at:
https://htmlpreview.github.io/?https://github.com/rust-lang/rust/blob/master/src/test/run-make-fulldeps/coverage-spanview-base/expected_mir_dump.%s/%s

For revisions in Pull Requests (PR):
* Replace "rust-lang" with the github PR author
* Replace "master" with the PR branch name

-->
endef
export SPANVIEW_HEADER

all: $(patsubst $(SOURCEDIR)/%.rs,%,$(wildcard $(SOURCEDIR)/*.rs))

@@ -33,31 +44,12 @@ endif
-Zdump-mir-spanview \
-Zdump-mir-dir="$(TMPDIR)"/mir_dump.$@

ifdef SED_HAS_ISSUES
# FIXME(richkadel): MacOS's default sed has some significant limitations. Until I've come up
# with a better workaround, I'm disabling this test for MacOS.
#
# For future reference, see if `gsed` is available as an alternative.
which gsed || echo "no gsed"
else

for path in "$(TMPDIR)"/mir_dump.$@/*; do \
echo $$path; \
file="$$(basename "$$path")"; \
echo $$file; \
urlescaped="$$("$(PYTHON)" $(BASEDIR)/escape_url.py $$file)" || exit $$?; \
echo $$urlescaped; \
sed -i -e '1a\
<!--\
\
Preview this file as rendered HTML from the github source at:\
https://htmlpreview.github.io/?https://github.com/rust-lang/rust/blob/master/src/test/run-make-fulldeps/coverage-spanview-base/expected_mir_dump.$@/'"$$urlescaped"'\
\
For revisions in Pull Requests (PR):\
* Replace "rust-lang" with the github PR author\
* Replace "master" with the PR branch name\
\
-->' "$$path"; \
printf "$$SPANVIEW_HEADER\n" "$@" "$$urlescaped" > "$$path".modified; \
tail -n +2 "$$path" >> "$$path".modified; \
mv "$$path".modified "$$path"; \
done && true # for/done ends in non-zero status

ifdef RUSTC_BLESS_TEST
@@ -70,5 +62,3 @@ else
cp "$(TMPDIR)"/mir_dump.$@/*InstrumentCoverage.0.html "$(TMPDIR)"/actual_mir_dump.$@/
$(DIFF) -r expected_mir_dump.$@/ "$(TMPDIR)"/actual_mir_dump.$@/
endif

endif