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

Improve support for doctests #2

Open
taiki-e opened this issue Jan 22, 2021 · 18 comments
Open

Improve support for doctests #2

taiki-e opened this issue Jan 22, 2021 · 18 comments
Labels
C-enhancement Category: A new feature or an improvement for an existing one C-upstream-bug Category: This is a bug of compiler or dependencies (the fix may require action in the upstream)

Comments

@taiki-e
Copy link
Owner

taiki-e commented Jan 22, 2021

STATUS:

#40 fixed all of the errors and many of the warnings

The remaining warnings are probably the column offset errors mentioned in rust-lang/rust#79417 (comment), and it's not clear whether we can fix them on our side at this time.


This is currently available via --doctests flag as an unstable feature.

cargo llvm-cov --doctests

Refs:

@taiki-e taiki-e added the C-enhancement Category: A new feature or an improvement for an existing one label Jan 22, 2021
@taiki-e
Copy link
Owner Author

taiki-e commented Jun 5, 2021

STATUS: I think it's better than before, but there are still a few problems.

warning: 4 functions have mismatched data

@cloudhead
Copy link

It would be great to have this working better. This tool is super handy.

Currently, running with --doctests I get some error output at the end, and it seems like the files that contain doctests get included twice in the coverage output:

error: src/block.rs: No such file or directory
warning: The file 'src/block.rs' isn't covered.
error: src/block/filter.rs: No such file or directory
warning: The file 'src/block/filter.rs' isn't covered.
error: src/block/iter.rs: No such file or directory
warning: The file 'src/block/iter.rs' isn't covered.
error: src/network.rs: No such file or directory
warning: The file 'src/network.rs' isn't covered.
error: src/protocol/addrmgr.rs: No such file or directory
warning: The file 'src/protocol/addrmgr.rs' isn't covered.
error: src/time.rs: No such file or directory
warning: The file 'src/time.rs' isn't covered.

@taiki-e
Copy link
Owner Author

taiki-e commented Jul 24, 2021

@cloudhead Is there an example that can reproduce the error you encountered?

@cloudhead
Copy link

cloudhead commented Jul 25, 2021

Yeah, though after reproducing this I realize some of these errors come from the --html flag, and on its own, --doctests doesn't output these errors, only in combination with --html.

The repo is here:
https://github.com/cloudhead/nakamoto

I ran: cargo llvm-cov --all --doctests --html

The issue with coverage output for files included twice appears with only --doctests though. Basically the paths are not matched, eg.:

net/poll/src/time.rs                     45                22    51.11%          14                 5    64.29%          78                30    61.54%           0                 0         -
src/time.rs                              22                 2    90.91%           4                 0   100.00%          89                 0   100.00%           0                 0         -

These are actually the same file, but I'm guessing the second one is from the doctest run, while the first is from the unit test run, I'm not sure.

Note that there is no src/time.rs, that path doesn't exist.

@taiki-e
Copy link
Owner Author

taiki-e commented Jul 27, 2021

Thanks. I'll take a look.

bors bot added a commit that referenced this issue Jul 27, 2021
40: Use -Z doctest-in-workspace for doctests r=taiki-e a=taiki-e

cc #2 

Co-authored-by: Taiki Endo <[email protected]>
@taiki-e
Copy link
Owner Author

taiki-e commented Jul 27, 2021

#40 fixed all of the errors and many of the warnings that happen in cloudhead/nakamoto.

Before #40:

warning: 5 functions have mismatched data
error: src/block.rs: No such file or directory
warning: The file 'src/block.rs' isn't covered.
error: src/block/filter.rs: No such file or directory
warning: The file 'src/block/filter.rs' isn't covered.
error: src/block/iter.rs: No such file or directory
warning: The file 'src/block/iter.rs' isn't covered.
error: src/network.rs: No such file or directory
warning: The file 'src/network.rs' isn't covered.
error: src/protocol/addrmgr.rs: No such file or directory
warning: The file 'src/protocol/addrmgr.rs' isn't covered.
error: src/time.rs: No such file or directory
warning: The file 'src/time.rs' isn't covered.
warning: 5 functions have mismatched data

After #40:

warning: 5 functions have mismatched data
warning: 5 functions have mismatched data

The remaining warnings are probably the column offset errors mentioned in rust-lang/rust#79417 (comment), and it's not clear whether we can fix them on our side at this time.

@cloudhead
Copy link

Awesome, thanks a bunch!

lpenz added a commit to lpenz/ghaction-rust-coverage that referenced this issue Jan 24, 2022
lpenz added a commit to lpenz/ghaction-rust-coverage that referenced this issue Jan 26, 2022
lpenz added a commit to lpenz/ghaction-rust-coverage that referenced this issue Jan 27, 2022
@adamreichold
Copy link

I am not sure whether I have missed any of the prerequisites, but I have tried to use the --doctests option in the rust-numpy crate, c.f. PyO3/rust-numpy#286, but it appears that while the doctests are executed, they are not included in the coverage computation (whereas both unit tests and integration tests work). There are no error messages and no warnings besides the initial warning that the feature is unstable. I can also reproduce this locally, e.g. if I run cargo llvm-cov --doc --html I get an empty report.

Can anybody advise if am missing some dependency or step? Thank you!

@taiki-e
Copy link
Owner Author

taiki-e commented Mar 8, 2022

My understanding is that the coverage is broken due to a bug of rustc's --remap-path-prefix (#140, #122).
Could you try #141?

@adamreichold
Copy link

Could you try #141?

I installed cargo-llvm-cov using

> cargo install --force --git https://github.com/taiki-e/cargo-llvm-cov.git --branch disable-remap-path-prefix cargo-llvm-cov

but did not observe any changes in the output. I guess I need to wait for #122 and therefore rust-lang/rust#92648 to land?

@dominikwilkowski
Copy link

dominikwilkowski commented Jun 5, 2022

I don't know if this is related to this issue here but adding the below lines to a test will give me warning: 4 functions have mismatched data.

let candy_color = [
	String::from("#ea3223"),
	String::from("#377d22"),
	String::from("#fffd54"),
	String::from("#ea3df7"),
	String::from("#74fbfd"),
	String::from("#ee776d"),
	String::from("#8cf57b"),
	String::from("#fffb7f"),
	String::from("#6974f6"),
	String::from("#ee82f8"),
	String::from("#8dfafd"),
];
assert!(candy_color.contains(&color2hex(&Colors::Candy, &options)));

These lines are within a *_test.rs file inside the test folder and inside a #[cfg(test)] and #[test] block.
They test the output of a function that randomizes it's output.

And I'm running the test without --doctests via cargo llvm-cov --html on stable

@taiki-e
Copy link
Owner Author

taiki-e commented Jul 7, 2022

@dominikwilkowski I believe that's also a rustc bug. (maybe related to code generated by #[test] attr)

@taiki-e taiki-e added the C-upstream-bug Category: This is a bug of compiler or dependencies (the fix may require action in the upstream) label Nov 27, 2022
@jonhoo
Copy link

jonhoo commented Jan 12, 2023

fwiw, now that instrument-coverage is stabilized, https://doc.rust-lang.org/rustc/instrument-coverage.html#including-doc-tests is the link to the relevant paragraph in the upstream docs

@taiki-e
Copy link
Owner Author

taiki-e commented Jan 12, 2023

@jonhoo Thanks. I updated the link.

saroh added a commit to saroh/tantivy that referenced this issue Jan 31, 2023
:warn: Marked as [unstable](taiki-e/cargo-llvm-cov#2)
also blindly removes `ci` directory which looks obsolete
refs quickwit-oss#1761
saroh added a commit to saroh/tantivy that referenced this issue Jan 31, 2023
:warn: Marked as [unstable](taiki-e/cargo-llvm-cov#2)
also blindly removes `ci` directory which looks obsolete
refs quickwit-oss#1761
saroh added a commit to saroh/tantivy that referenced this issue Jan 31, 2023
saroh added a commit to saroh/tantivy that referenced this issue Feb 11, 2023
PSeitz pushed a commit to quickwit-oss/tantivy that referenced this issue Feb 14, 2023
* fix(CI) enable coverage on doctest
:warning: Marked as [unstable](taiki-e/cargo-llvm-cov#2)
refs #1761

* remove obsolete CI directory
@taiki-e
Copy link
Owner Author

taiki-e commented Feb 22, 2023

@dominikwilkowski If an issue you have encountered has not yet been fixed, would you mind submitting an issue about it at https://github.com/rust-lang/rust?

@dominikwilkowski
Copy link

@taiki-e what's a good way to re-produce this bug without llvm-cov? Once I know how to re-produce this I'll happily submit a ticket to rust

@taiki-e
Copy link
Owner Author

taiki-e commented Feb 26, 2023

I think we can get the commands that cargo-llvm-cov internally calls and adjust them. For example, the output of the following command should be able to be run as-is:

cargo llvm-cov --html -vv 2>&1 | grep CARGO_INCREMENTAL | sed -E -e 's/^ *Running `//g; s/`$//g; s|-sparse -f .*-o|-sparse target/llvm-cov-target/*.profraw -o|; s|.*/bin/llvm-|"$(rustc --print target-libdir)"/../bin/llvm-|g' -e "s|$(pwd)/||g"

@jcape
Copy link

jcape commented Mar 23, 2023

Here's the output from mobilecoinfoundation/from-random

CARGO_INCREMENTAL=0 LLVM_PROFILE_FILE='target/llvm-cov-target/from-random-%p-%m.profraw' RUSTFLAGS='-C instrument-coverage --cfg=coverage --cfg=trybuild_no_target' RUST_TEST_THREADS=1 /home/user/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/cargo test --tests --manifest-path Cargo.toml --target-dir target/llvm-cov-target -v

"$(rustc --print target-libdir)"/../bin/llvm-profdata merge -sparse target/llvm-cov-target/*.profraw -o target/llvm-cov-target/from-random.profdata

"$(rustc --print target-libdir)"/../bin/llvm-cov show -format=html -instr-profile=target/llvm-cov-target/from-random.profdata -object target/llvm-cov-target/debug/deps/mc_from_random-0b047475ff4e3b36 -ignore-filename-regex '/rustc/[0-9a-f]+/|^/home/user/src/mobilecoinfoundation/from\-random(/.*)?/(tests|examples|benches)/|^/home/user/src/mobilecoinfoundation/from\-random/target/llvm\-cov\-target($|/)|^/home/user/\.cargo/(registry|git)/|^/home/user/\.rustup/toolchains($|/)' -show-instantiations=true -show-line-counts-or-regions -show-expansions -Xdemangler=/home/user/.cargo/bin/cargo-llvm-cov -Xdemangler=llvm-cov -Xdemangler=demangle -output-dir=target/llvm-cov/html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: A new feature or an improvement for an existing one C-upstream-bug Category: This is a bug of compiler or dependencies (the fix may require action in the upstream)
Projects
None yet
Development

No branches or pull requests

6 participants