From 913c8d807ab4ff5ed74eb876f5f1524784fa02fd Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Fri, 11 Sep 2020 14:10:13 -0400 Subject: [PATCH 1/3] tests: attempt to gather code coverage info in the executable tests It turns out that our tests of the built `tectonic` executable weren't gathering code coverage information about the code paths exercised in those tests. I *think* that it's not so hard to hack in some support to gather that information, so let's give it a try. --- dist/azure-coverage.yml | 38 +++++++++++++++++++++++++++++--------- tests/executable.rs | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 9 deletions(-) diff --git a/dist/azure-coverage.yml b/dist/azure-coverage.yml index 43e19dfe6..21a71b720 100644 --- a/dist/azure-coverage.yml +++ b/dist/azure-coverage.yml @@ -28,17 +28,16 @@ steps: cargo install --force cargo-kcov displayName: Set up code coverage +# As of Rust 1.44, test executables land in target/debug/deps/ instead of +# target/debug/, which messes up current cargo-kcov (0.5.2) because it tries to +# search for those executables. Work around with `cp`. One of the `tectonic-*` +# binaries is the debug executable, which is hard-linked to +# `target/debug/tectonic`. kcov will erroneously try to run this as a test if we +# copy it, so we have to make not to do that, which we accomplish with a search +# based on the hardlink count. Hacky and fragile but this should get us going. +# Hopefully kcov will get fixed where this will become unneccessary anyway. - bash: | set -xeuo pipefail - # As of Rust 1.44, test executables land in target/debug/deps/ instead of - # target/debug/, which messes up current cargo-kcov (0.5.2) because it tries - # to search for those executables. Work around with `cp`. One of the - # `tectonic-*` binaries is the debug executable, which is hard-linked to - # `target/debug/tectonic`. kcov will erroneously try to run this as a test - # if we copy it, so we have to make not to do that, which we accomplish with - # a search based on the hardlink count. Hacky and fragile but this should - # get us going. Hopefully kcov will get fixed where this will become - # unneccessary anyway. cargo test --no-run find target/debug/deps/tectonic-???????????????? -links 2 -print -delete cp -vp target/debug/deps/*-???????????????? target/debug/ @@ -54,6 +53,27 @@ steps: - bash: cargo kcov --no-clean-rebuild displayName: cargo kcov +# Our "executable" test executes the actual Tectonic binary. In order to collect +# coverage information about what happens in those executions, we have special +# support in the test harness to wrap the invocations in `kcov` calls. +- bash: | + set -xeuo pipefail + export TECTONIC_EXETEST_KCOV_RUNNER="kcov --exclude-pattern=$HOME/.cargo --verify" + cargo test --test executable + displayName: Special-case executable tests + +# Now, merge all of the coverage reports. `cargo kcov` does this automatically, +# but it uses an explicit list of coverage runs, so there's no way to get it to +# include our special executable tests. We just glob everything, which means we +# have to delete the preexisting merged report. +- bash: | + set -xeou pipefail + cd target/cov + rm -rf amber.png bcov.css data glass.png index.html index.json \ + kcov-merged merged-kcov-output + kcov --merge . * + displayName: Merge coverage reports + - bash: | set -xeuo pipefail bash <(curl -s https://codecov.io/bash) diff --git a/tests/executable.rs b/tests/executable.rs index c6c925f98..3dc1358fa 100644 --- a/tests/executable.rs +++ b/tests/executable.rs @@ -23,6 +23,7 @@ lazy_static! { root.push("tests"); root }; + static ref TARGET_RUNNER_WORDS: Vec = { // compile-time environment variable from build.rs: let target = env!("TARGET").to_owned(); @@ -36,6 +37,17 @@ lazy_static! { vec![] } }; + + // Special coverage-collection mode. This implementation is quite tuned for + // the Tectonic CI/CD system, so if you're trying to use it manually, expect + // some rough edges. + static ref KCOV_WORDS: Vec = { + if let Ok(runtext) = env::var("TECTONIC_EXETEST_KCOV_RUNNER") { + runtext.split_whitespace().map(|x| x.to_owned()).collect() + } else { + vec![] + } + }; } fn get_plain_format_arg() -> String { @@ -61,10 +73,32 @@ fn prep_tectonic(cwd: &Path, args: &[&str]) -> Command { println!("using tectonic binary at {:?}", tectonic); println!("using cwd {:?}", cwd); + // We may need to wrap the Tectonic invocation. If we're cross-compiling, we + // might need to use something like QEMU to actually be able to run the + // executable. If we're collecting code coverage information with kcov, we + // need to wrap the invocation with that program. let mut command = if TARGET_RUNNER_WORDS.len() > 0 { let mut cmd = Command::new(&TARGET_RUNNER_WORDS[0]); cmd.args(&TARGET_RUNNER_WORDS[1..]).arg(tectonic); cmd + } else if KCOV_WORDS.len() > 0 { + let mut cmd = Command::new(&KCOV_WORDS[0]); + cmd.args(&KCOV_WORDS[1..]); + + // Give kcov a directory into which to put its output. We use + // mktemp-like functionality to automatically create such directories + // uniquely so that we don't have to manually bookkeep. This does mean + // that successive runs will build up new data directories indefinitely. + let mut root = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + root.push("target"); + root.push("cov"); + root.push("exetest."); + let tempdir = tempfile::Builder::new().prefix(&root).tempdir().unwrap(); + let tempdir = tempdir.into_path(); + cmd.arg(tempdir); + + cmd.arg(tectonic); + cmd } else { Command::new(tectonic) }; From 3b100e42437a7ee7c6e904f6f08ad18e1ff2c2dd Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Fri, 11 Sep 2020 14:45:14 -0400 Subject: [PATCH 2/3] dist/azure-coverage.yml: some kcovs have index.js, it seems --- dist/azure-coverage.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dist/azure-coverage.yml b/dist/azure-coverage.yml index 21a71b720..0600cb969 100644 --- a/dist/azure-coverage.yml +++ b/dist/azure-coverage.yml @@ -69,7 +69,7 @@ steps: - bash: | set -xeou pipefail cd target/cov - rm -rf amber.png bcov.css data glass.png index.html index.json \ + rm -rf amber.png bcov.css data glass.png index.html index.js* \ kcov-merged merged-kcov-output kcov --merge . * displayName: Merge coverage reports From 74d2549b1fb0b9b0e5d94c5858fe33e26ef5574b Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Sun, 13 Sep 2020 23:06:25 -0400 Subject: [PATCH 3/3] dist/azure-coverage.yml: try to only include main code in coverage stats --- dist/azure-coverage.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/dist/azure-coverage.yml b/dist/azure-coverage.yml index 0600cb969..2248afd69 100644 --- a/dist/azure-coverage.yml +++ b/dist/azure-coverage.yml @@ -50,7 +50,10 @@ steps: git show HEAD displayName: Make release commit -- bash: cargo kcov --no-clean-rebuild +- bash: | + set -xeuo pipefail + p="$(pwd)" + cargo kcov --no-clean-rebuild -- --include-pattern="$p/src,$p/tectonic,$p/xdv" displayName: cargo kcov # Our "executable" test executes the actual Tectonic binary. In order to collect @@ -58,7 +61,8 @@ steps: # support in the test harness to wrap the invocations in `kcov` calls. - bash: | set -xeuo pipefail - export TECTONIC_EXETEST_KCOV_RUNNER="kcov --exclude-pattern=$HOME/.cargo --verify" + p="$(pwd)" + export TECTONIC_EXETEST_KCOV_RUNNER="kcov --include-pattern=$p/src,$p/tectonic,$p/xdv" cargo test --test executable displayName: Special-case executable tests