Tarpaulin: Tarpaulin fails to do coverage test on proc-macro repos

[oddcoder]

I am not sure what to look or how to debug the issue but the error message is

ERROR: failed to read cargo metadata: EOF while parsing a value at line 1 column 0

It only appears in proc-macro repos.

Travis CI log for failing job with tarpaulin: https://travis-ci.org/Rair-Project/rair-core/jobs/638586153
Travis CI log for same job passing tests without tarpaulin https://travis-ci.org/Rair-Project/rair-core/builds/638586152

[xd009642]

So cargo based issues can be caused by tarpaulin using an outdated version of cargo. The latest release 0.10.2 updated the cargo version and I can see your Travis run used 0.10.0. can you clear your cargo cache and try again to see if it works?

[oddcoder]

I cleared cargo cache and rebuilt, the same issue persists
https://travis-ci.org/Rair-Project/rair-core/builds/638824073

[xd009642]

Just tried it out on my machine and it worked fine, I also see there are some runs in your CI where it worked fine. Is there some set of feature flags etc I should be using to recreate the error?

|| Tested/Total Lines:
|| rair-env/src/environment.rs: 190/319
|| rair-env/src/err.rs: 3/6
|| rair-env/src/metadata.rs: 34/40
|| rair-io/src/desc.rs: 30/34
|| rair-io/src/descquery.rs: 79/109
|| rair-io/src/io.rs: 104/112
|| rair-io/src/mapsquery.rs: 92/101
|| rair-io/src/plugin.rs: 1/6
|| rair-io/src/plugins/base64.rs: 13/131
|| rair-io/src/plugins/defaultplugin.rs: 33/60
|| rair-io/src/plugins/dummy.rs: 0/4
|| rair-io/src/plugins/ihex.rs: 8/216
|| rair-io/src/plugins/malloc.rs: 37/50
|| rair-io/src/plugins/mod.rs: 6/6
|| rair-io/src/plugins/srec.rs: 4/256
|| rair-io/src/utils.rs: 8/37
|| rtrees/src/bktree/tree.rs: 47/50
|| rtrees/src/ist/interval.rs: 10/10
|| rtrees/src/ist/iter.rs: 0/12
|| rtrees/src/ist/iter_ref.rs: 11/12
|| rtrees/src/ist/rb_helpers.rs: 24/36
|| rtrees/src/ist/serialize.rs: 20/22
|| rtrees/src/ist/tree.rs: 46/118
|| rtrees/src/rbtree/color.rs: 4/4
|| rtrees/src/rbtree/iter.rs: 0/23
|| rtrees/src/rbtree/iter_ref.rs: 23/25
|| rtrees/src/rbtree/node.rs: 44/55
|| rtrees/src/rbtree/rbtree_wrapper.rs: 119/170
|| rtrees/src/rbtree/serialize.rs: 0/19
|| src/commands.rs: 10/12
|| src/core.rs: 112/114
|| src/helper.rs: 101/113
|| src/io/files.rs: 139/154
|| src/io/map.rs: 179/179
|| src/io/mod.rs: 16/16
|| src/io/print.rs: 445/946
|| src/io/write.rs: 148/157
|| src/loc/history.rs: 48/48
|| src/loc/mod.rs: 4/4
|| src/loc/mode.rs: 73/74
|| src/loc/seek.rs: 192/192
|| src/utils/env.rs: 322/329
|| src/utils/mod.rs: 8/8
|| src/utils/project.rs: 66/78
|| src/utils/quit.rs: 12/13
|| src/writer.rs: 44/50
|| test_file/src/lib.rs: 4/17
|| 
64.06% coverage, 2913/4547 lines covered

[oddcoder]

Yes! From what I see, the problem is with only procedure macro repos, with a proc-macro = true entry like this one https://github.com/Rair-Project/rair-core/pull/82/files#diff-3dfe6f205a9e299a17487b33833a7b1a and/or tesing with trybuild crate. Not sure which one is triggering the bug as I am not aware of other ways to test proc-macros.

[xd009642]

Ah so everything in the crate is a proc macro? I'll work on this my end but you might want to look at https://crates.io/crates/runtime-macros it turns proc macros evaluated at compile time into runtime expressions which means tarpaulin actually has something to cover. Though that would require tarpaulin getting past the Cargo.toml itself...

[pksunkara]

Looks like the same issue with #336

[xd009642]

have you tried clap_derive with the runtime-macros crate? That could be worth a shot

[pksunkara]

Unfortunately, it doesn't test enough to be changed to. I could repeat the tests, but that's not DRY.

[xd009642]

So I moved my cargo dependency to the latest on master and it seems to solve the issue. You can try it out by installing from https://github.com/xd009642/tarpaulin/tree/issue_339

[pksunkara]

Could you do a pre-release so we can test it? I can't run it locally because I am on Mac.

[xd009642]

I'm at work now but I can do a docker build later and push that or you could always spin up a CI job to try it out in the meantime.

I have a bigger change to the project that should solve this better by removing the cargo dependency so if you're more patient you can wait for that (I hope to have it done end of the weekend)

[pksunkara]

I can wait. I had some issues with the docker build IIRC. Also, it was huge. Maybe you can look into https://github.com/docker-slim/docker-slim for the docker build. (I might even look into this today for you 😄)

[pksunkara]

Still has issues.

test ui ... FAILED

failures:

---- ui stdout ----
thread 'ui' panicked at 'tests failed', /usr/local/cargo/registry/src/github.com-1ecc6299db9ec823/trybuild-1.0.23/src/run.rs:38:13
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.40/src/backtrace/libunwind.rs:88
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.40/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print_fmt
             at src/libstd/sys_common/backtrace.rs:84
   3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
             at src/libstd/sys_common/backtrace.rs:61
   4: core::fmt::write
             at src/libcore/fmt/mod.rs:1025
   5: std::io::Write::write_fmt
             at /rustc/f3e1a954d2ead4e2fc197c7da7d71e6c61bad196/src/libstd/io/mod.rs:1426
   6: std::io::impls::<impl std::io::Write for alloc::boxed::Box<W>>::write_fmt
             at src/libstd/io/impls.rs:156
   7: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:65
   8: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:50
   9: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:193
  10: std::panicking::default_hook
             at src/libstd/panicking.rs:207
  11: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:471
  12: std::panicking::begin_panic
             at /rustc/f3e1a954d2ead4e2fc197c7da7d71e6c61bad196/src/libstd/panicking.rs:404
  13: trybuild::run::<impl trybuild::Runner>::run::{{closure}}
             at /usr/local/cargo/registry/src/github.com-1ecc6299db9ec823/trybuild-1.0.23/src/run.rs:38
  14: core::result::Result<T,E>::unwrap_or_else
             at /rustc/f3e1a954d2ead4e2fc197c7da7d71e6c61bad196/src/libcore/result.rs:841
  15: trybuild::run::<impl trybuild::Runner>::run
             at /usr/local/cargo/registry/src/github.com-1ecc6299db9ec823/trybuild-1.0.23/src/run.rs:36
  16: <trybuild::TestCases as core::ops::drop::Drop>::drop
             at /usr/local/cargo/registry/src/github.com-1ecc6299db9ec823/trybuild-1.0.23/src/lib.rs:274
  17: core::ptr::real_drop_in_place
             at /rustc/f3e1a954d2ead4e2fc197c7da7d71e6c61bad196/src/libcore/ptr/mod.rs:182
  18: macro_errors::ui
             at clap_derive/tests/macro-errors.rs:13
  19: macro_errors::ui::{{closure}}
             at clap_derive/tests/macro-errors.rs:10
  20: core::ops::function::FnOnce::call_once
             at /rustc/f3e1a954d2ead4e2fc197c7da7d71e6c61bad196/src/libcore/ops/function.rs:232
  21: <alloc::boxed::Box<F> as core::ops::function::FnOnce<A>>::call_once
             at /rustc/f3e1a954d2ead4e2fc197c7da7d71e6c61bad196/src/liballoc/boxed.rs:1022
  22: __rust_maybe_catch_panic
             at src/libpanic_unwind/lib.rs:78
  23: std::panicking::try
             at /rustc/f3e1a954d2ead4e2fc197c7da7d71e6c61bad196/src/libstd/panicking.rs:270
  24: std::panic::catch_unwind
             at /rustc/f3e1a954d2ead4e2fc197c7da7d71e6c61bad196/src/libstd/panic.rs:394
  25: test::run_test_in_process
             at src/libtest/lib.rs:567
  26: test::run_test::run_test_inner::{{closure}}
             at src/libtest/lib.rs:474
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.


failures:
    ui

[xd009642]

This should be fixed in the 0.11.1 release, so I'm going to close this issue. If it's not sorted feel free to reopen and comment :+1:

[pksunkara]

The trybuild tests are still failing. https://travis-ci.org/github/clap-rs/clap/jobs/662128329#L709

[xd009642]

@pksunkara if you try the latest docker image for develop (or just do cargo install cargo-tarpaulin --git https://github.com/xd009642/tarpaulin --branch develop) you should hopefully find it works a lot better. I ran it locally and the only test failures were to do with some version number checking thing in HTML docs which I'm looking into

[pksunkara]

I don't see any changes in the develop branch that would fix this. But I still tried anyway at https://github.com/clap-rs/clap/pull/1747/files. It failed with the same thing, https://travis-ci.org/github/clap-rs/clap/jobs/662526678#L475.

And running locally using docker is stuck on compiling step. (I waited for a long time).

docker run --security-opt seccomp=unconfined -v ~/.cargo/registry:/usr/local/cargo/registry -v (pwd):/volume xd009642/tarpaulin:develop sh -c "RUST_BACKTRACE=1 cargo tarpaulin --workspace --features yaml -v"
[INFO tarpaulin] Creating config
[INFO tarpaulin] Running Tarpaulin
[INFO tarpaulin] Building project
    Updating git repository `git://github.com/pksunkara/criterion.rs`
   Compiling clap v3.0.0-beta.1 (/volume)

Do you think you could forward the verbose flag to cargo? Maybe I will have a better idea if something is happening or not.

[xd009642]

That's because I'm on a different PC and it asked me for my git password to push and I didn't notice! Pushing the change now

[xd009642]

I'll add something so if you give --debug it will send --verbose to cargo, don't want to fill everyones screens with messages. If you're running on mac you might be experiencing docker issues because mac runs docker in a linux VM and the VM limitations are quite low. Most Mac docker issues I've seen have boiled down to that

[pksunkara]

Thanks. Still failing, https://travis-ci.org/github/clap-rs/clap/jobs/662526678#L544. I just restarted the job which installed the new develop HEAD

[xd009642]

So one of the issues is caused because the env var CARGO_PKG_NAME isn't available. Looking at the rust docs https://doc.rust-lang.org/cargo/reference/environment-variables.html these are available when you use cargo (run|build|test) but that doesn't means they're available if you run a test executable directly.

I think If you want the tests to run the same via cargo test or launching the test executable directly like tarpaulin does you should set the env vars to replicate the cargo test environment or alternatively use the var! or try_var! macros instead

[pksunkara]

You are talking about the non trybuild test failing? Yes, it's an issue with cargo env vars. Or are you saying this is a fix for the trybuild tests themselves?

[xd009642]

Yeah I'm talking about the trybuild test failures and this is how you could solve them without needing a change in tarpaulin. Personally for tests like that I'd prefer the compile time macros myself as it means the test runs the same each time

[pksunkara]

Could you give more details on how to fix it? I am not sure I have the complete idea on how the fix works.

[xd009642]

I got the macro names wrong, found the right ones after a google.

So in a test instead of

let pkg_name = env::var("CARGO_PKG_NAME");
match pkg_name {

}

Use https://doc.rust-lang.org/std/macro.option_env.html so it gets the var at compile time not run time

let pkg_name = option_env!("CARGO_PKG_NAME");
match pkg_name {

}

[pksunkara]

https://travis-ci.org/github/clap-rs/clap/jobs/662706650#L299 with https://github.com/clap-rs/clap/pull/1747/commits/9b6eaae7c6795ee26698467767852c5def32d88f

I even tried to do some intelligent stuff. Do you think this is an issue with trybuild itself?

[pksunkara]

@xd009642 Do you want a small reproducible repo with trybuild tests so you can work on it and fix it? (Either by fixing this crate or by sending PR to trybuild or by changing something in the project)

[xd009642]

Yeah sure I'll always accept reproducible examples 😊

[CreepySkeleton]

I think this is due to trybuild requires CARGO_PKG_NAME to be set up at runtime, see https://github.com/dtolnay/trybuild/issues/14. I'll try to fix the issue in few hours.

[CreepySkeleton]

Alternatively, tarpaulin could set CARGO_PKG_NAME to something like <nameless crate>.

[pksunkara]

@CreepySkeleton Thanks for pointing out. I started working on a preliminary probe to see if it actually fixes the issues and looks like it might.

First attempt: Use option_env! instead of env::var* (commit)

https://travis-ci.org/github/clap-rs/clap/jobs/673138109

Some progress that now trybuild tests are actually running. But still some issues

test tests/ui/bool_default_value.rs ... mismatch
EXPECTED:
┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈
error: default_value is meaningless for bool
  --> $DIR/bool_default_value.rs:14:19
   |
14 |     #[clap(short, default_value = true)]
   |                   ^^^^^^^^^^^^^
┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈
ACTUAL OUTPUT:
┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈
error: couldn't read $DIR/tests/ui/bool_default_value.rs: No such file or directory (os error 2)
┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈

[CreepySkeleton]

I suspect this is because the name/dir you get though option_env! is not the one trybuild needs. @pksunkara Could you please add a debugging println!(name, dir) to your fork? Probably will be clap ./ instead of clap_derive ./clap_derive

[pksunkara]

So, my earlier commit is completely wrong because we need those variables from runtime.

[pksunkara]

Thanks, I would accept a PR to support non-Cargo test runs as long as it is not too disruptive to the existing implementation. Really I think the coverage runner should define these env vars.

@xd009642 Can you try setting these environment variables in execute_test please? I think that would solve this issue. CARGO_PKG_NAME and CARGO_MANIFEST_DIR.

[pksunkara]

@xd009642 Got it working here. I forked tarpaulin and added this commit which got it working. Unfortunately, I can't contribute a PR because I couldn't get tarpaulin working locally since I am on MacOS. But please, go ahead and take the commit and make a release fast.

The only issue the commit might have is that I haven't ran it with doctests runtype. You should probably test that situation before finalising it.

[pksunkara]

Also, the coverage is 0 for the proc-macro crates as you can see here, https://coveralls.io/builds/29999944. The tests are passing now but coverage is not being correct.

[CreepySkeleton]

Yeah, we would need something like runtime-macros for that, but it doesn't support derive macros and looks dead.

Can we just exclude the derive from coverage entirely?

[xd009642]

There's the --exclude-files flag which you can use to exclude all the files in a folder i.e. --exclude-files clap_derive/*. But yeah in terms of coverage code has to be runtime executable which a lot of procmacro code isn't without something like runtime-macros

[pksunkara]

I am okay with leaving them at 0 right now. But I am thinking of the long term future here where tarpaulin is the go to coverage tool for Rust.

[pksunkara]

Now that the tests running is fixed, can someone give me a small bit of context on exactly why we can't get coverage of the proc-macros?

[CreepySkeleton]

To be fair, the issue is not exactly fixed, it's just ignored.

Context (not a pro, feel free to correct me): this is what the process looks like:

1. A tests gets built. This is where rustc does all the work with the source: expands macros (including proc-macros), parses source code, builds AST, lowers it to HIR, checking, lowers it further, makes use of LLVM, yadda yadda. As the result, we get an executable to run.
2. Tarpaulin runs the executable. This is where the coverage is being accounted: tarpaulin loads the executable, runs it, keeps track which branches were taken and which weren't (probably via some sort of software interrupts), maps the data to the source file somehow (does it?), et cetera.

The problem is that macro expansion only exists at the step one. Tarpaulin couldn't count it simply because the macros had already been expanded. They are not present in the binary, only the result of the expansion is, which cannot be mapped back to source.

[xd009642]

@CreepySkeleton that's spot on 👍

[pksunkara]

Tarpaulin would know if a package is proc-macro or not. Can we calculate coverage using interrupts for compilation (step one) itself?

[CreepySkeleton]

I doubt it. This would require squeezing the tarpaulin run between "the code have been parsed into a stream of tokens" and "macros have been expanded"... effectively someplace inside rustc. And it would also be very unclear where to put the interrupts: the code of rustc itself would probably be inter-tangled with code of the macro.

I think runtime-macros is the only way.

[pksunkara]

runtime-macros does not support derives.

[CreepySkeleton]

There's a PR awaiting to be landed 😉 But anyway, I don't understand your obsession with coverage. It is telling for clap thanks to very extensive testing but not so useful for the derive where number of possible inputs is not so large.

[xd009642]

I'm going to close this issue as the base issue is solved and the proc-macro stuff seems to be answered.

[tonyfinn]

I ran into the issue with testing proc-macro derives myself a while back and published runtime-macros-derive as a fork of runtime-macros with derive support, after the pull request to add it to runtime-macros was unanswered