Skip to content
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.

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

Tarpaulin fails to do coverage test on proc-macro repos #326

Closed
oddcoder opened this issue Jan 17, 2020 · 49 comments
Closed

Tarpaulin fails to do coverage test on proc-macro repos #326

oddcoder opened this issue Jan 17, 2020 · 49 comments
Labels

Comments

@oddcoder
Copy link

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
Copy link
Owner

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
Copy link
Author

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

@xd009642
Copy link
Owner

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

@xd009642 xd009642 added the bug label Jan 18, 2020
@oddcoder
Copy link
Author

oddcoder commented Jan 18, 2020

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
Copy link
Owner

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
Copy link
Contributor

Looks like the same issue with #336

@xd009642
Copy link
Owner

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

@pksunkara
Copy link
Contributor

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

@xd009642
Copy link
Owner

xd009642 commented Mar 4, 2020

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
Copy link
Contributor

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

@xd009642
Copy link
Owner

xd009642 commented Mar 5, 2020

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Owner

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 👍

@pksunkara
Copy link
Contributor

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

@xd009642 xd009642 reopened this Mar 14, 2020
@xd009642
Copy link
Owner

@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
Copy link
Contributor

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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Contributor

pksunkara commented Mar 14, 2020

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
Copy link
Owner

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
Copy link
Contributor

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
Copy link
Owner

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
Copy link
Contributor

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
Copy link
Owner

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
Copy link
Contributor

https://travis-ci.org/github/clap-rs/clap/jobs/662706650#L299 with clap-rs/clap@9b6eaae

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

@pksunkara
Copy link
Contributor

@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
Copy link
Owner

xd009642 commented Apr 9, 2020

Yeah sure I'll always accept reproducible examples 😊

@CreepySkeleton
Copy link

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

@CreepySkeleton
Copy link

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

@pksunkara
Copy link
Contributor

@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
Copy link

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
Copy link
Contributor

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

@pksunkara
Copy link
Contributor

pksunkara commented Apr 9, 2020

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
Copy link
Contributor

@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
Copy link
Contributor

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
Copy link

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
Copy link
Owner

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link

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
Copy link
Owner

@CreepySkeleton that's spot on 👍

@pksunkara
Copy link
Contributor

pksunkara commented Apr 13, 2020

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

@CreepySkeleton
Copy link

CreepySkeleton commented Apr 13, 2020

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
Copy link
Contributor

runtime-macros does not support derives.

@CreepySkeleton
Copy link

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
Copy link
Owner

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

@tonyfinn
Copy link
Contributor

tonyfinn commented Dec 1, 2020

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

@Javagedes
Copy link

Javagedes commented Oct 23, 2024

Hi @xd009642, are there any other known issues similar to this?

I added a proc-macro crate to a larger repository with many crates in it, and for some reason it is failing on windows-latest github action runners, but passes locally on Windows, and passes on ubuntu-latest runners. It also passes fine on all scenarios when just running cargo test.

It started failing coverage testing even before adding any tests to it. I added tests and it still failed, then I switched the test format to the one mentioned above, with runtime-macros and it is still failing.

I think its crashing the entire executable, not a particular test, but I can't tell, and I'm not sure how to get any additional information out of the logs through tarpaulin or flags to rust.

2024-10-23T23:42:17.231295Z  INFO cargo_tarpaulin::process_handling: running *******\uefi_test_macro-f2a1d8a12c5496b9.exe
2024-10-23T23:42:17.231561Z  INFO cargo_tarpaulin::process_handling: Setting LLVM_PROFILE_FILE
2024-10-23T23:42:17.240008Z ERROR cargo_tarpaulin: Test failed during run
Error while executing command, exit code: 1

I understand you can't help much since I can't provide a repo as of yet, but I was just hoping you had seen something like this before. I skimmed the issues, but I could have missed it.

Currently running:

cargo-tarpaulin 0.31.2
1.80.0 w/ RUSTC_BOOTSTRAP=1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants