Src file mapping not working properly with `trybuild` and proc-macro workspace member

taiki-e / cargo-llvm-cov

Cargo subcommand to easily use LLVM source-based code coverage (-C instrument-coverage).

Apache License 2.0

928 stars 57 forks source link

Src file mapping not working properly with `trybuild` and proc-macro workspace member #140

Closed teoxoy closed 2 years ago

teoxoy commented 2 years ago

When running integration tests (via trybuild) for a proc-macro crate that is a workspace member, the following warning is output:

error: dep/dep/src/lib.rs: No such file or directory
warning: The file 'dep/dep/src/lib.rs' isn't covered.

cmd run:

cargo llvm-cov --workspace --html

The report ends up showing 2 entries for the same src file as seen below

You can find a small reproducible sample here: test.tar.gz

This seems to be happening even if trybuild::TestCases::new() is the only thing in the test fn (no calls to pass or compile_fail). It also seems to work fine if the crate is a normal lib instead of a proc-macro lib.

I wasn't sure if this is an issue with cargo-llvm-cov or trybuild but having tried tarpaulin with success (same setup), I thought it might be an issue with cargo-llvm-cov.

taiki-e commented 2 years ago

Thanks for the report!

This appears to be a bug in rustc's --remap-path-prefix. If I disable it, it works correctly (https://github.com/taiki-e/cargo-llvm-cov/pull/141).

teoxoy commented 2 years ago

I tested the PR and it seems to solve the issue. Thanks for the quick response and fix!

Do you have any concerns with the change in the PR? Also, should we report the bug upstream?

taiki-e commented 2 years ago

The problem I am currently aware of is that our test suite does not work well because the reports show absolute paths.

https://github.com/taiki-e/cargo-llvm-cov/blob/20d5da7cab721cdd3d140beb86d48e8076f291a5/tests/fixtures/coverage-reports/no_test/link_dead_code.txt#L1

Another concern is that this may be a problem when merging coverage between different platforms. (I have not looked into this yet.)

So, for now, I'm starting to think it is reasonable to provide an option to disable --remap-path-prefix.

taiki-e commented 2 years ago

According to https://github.com/ruifengx/rowantlr/pull/3, this seems to be a regression introduced around 2021-11-30.

teoxoy commented 2 years ago

Maybe something to do with https://github.com/rust-lang/rust/pull/83846 (merged on 2021-11-11)?

taiki-e commented 2 years ago

Published 0.3.0 which includes the fix for this.