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

Doctests aren't taken into account #13

Closed
Marwes opened this issue Jul 23, 2017 · 29 comments
Closed

Doctests aren't taken into account #13

Marwes opened this issue Jul 23, 2017 · 29 comments

Comments

@Marwes
Copy link

Marwes commented Jul 23, 2017

I saw you had doc tests on the roadmap which is something I could really use in combine as a they should handle most of the coverage (https://coveralls.io/builds/12513027/source?filename=src%2Fbyte.rs#L47).

Opening this issue to inquire/track when this would be implemented.

@xd009642
Copy link
Owner

Doctests are on the roadmap, mildly difficult as when you run the doctest what cargo does is create a new folder and auto generate a doctest executable. Then once the doc test is finished it deletes the directory. Now I need to after generating that executable instrument it and then collect coverage, so rustc is working counter to my aims in that respect.

I can't really give a firm date, I need to inspect the cargo and rustc source (and libs derived from rustc modules) to see if any offer a good solution.

@gnzlbg
Copy link

gnzlbg commented Jul 26, 2017

This would be great, in some libraries most of the code is covered by doc tests. Having to duplicate those just to get coverage information is a pain :(

@xd009642
Copy link
Owner

I've had a look again, librustdoc isn't available as a separate crate as of yet. I may have to create and publish the fork myself. This one will be a bit more of a slow burner though.

@malbarbo
Copy link

I was able to run kcov on doctests in the past. My quick and dirty solution was to use LD_PRELOAD and inject a execvp function.

Here is the code that I used to create a .so file:

#[macro_use]
extern crate redhook;
extern crate libc;

use libc::c_char;
use std::ffi::CString;
use std::slice;
use std::env;

hook! {
    unsafe fn execvp(filename: *const c_char, argv: *const *const c_char) => my_execvp {
        let file = slice::from_raw_parts(filename as *const u8, libc::strlen(filename));
        if let Ok(run) = env::var("TEST_WRAPPER") {
            if let Ok(cmd) = run.split_whitespace()
                                .map(|s| CString::new(s))
                                .collect::<Result<Vec<_>, _>>() {
                if !cmd.is_empty() &&
                    file.starts_with(b"/tmp/rustdoctest") &&
                    file.ends_with(b"rust_out")
                {
                    let mut args: Vec<_> = cmd.iter().map(|s| s.as_ptr()).collect();
                    let mut x = argv;
                    while !(*x).is_null() {
                        args.push(*x);
                        x = x.offset(1);
                    }
                    args.push(*x);
                    real!(execvp)(cmd[0].as_ptr(), args.as_ptr());
                    return;
                }
            }
        }

        real!(execvp)(filename, argv);
    }
}

To run doctess with kcov, execute LD_PRELOAD=path-to-lib.so TEST_WRAPPER=kcov cargo test --doc

Although this is not a beautiful solution, it works. @xd009642 Maybe you can integrate this in tarpaulin.

@rask
Copy link

rask commented Sep 19, 2018

Hey, I was thinking if this is a thing where we could raise an issue against rustc or Cargo to add a flag where the doc-tests are not removed automatically after test runs?

I've seen this same issue in a few separate places and the main issue seems to be the core test framework removing the doc-tests directory before anyone can act upon the test results.

@xd009642
Copy link
Owner

Yeah that sounds like a good shout. I did have a look at this in the past and I think the code that deletes the doctests after run is in rustc. In fact looking into the cargo source I can't see any sign in cargo that it's done there.

@rask
Copy link

rask commented Sep 19, 2018

So rustc should be our target for making a plea. I wonder how this would be received there.

EDIT: oh I found and issue, and you (@xd009642 ) have already chimed in on it. rust-lang/rust#37048

@xd009642
Copy link
Owner

Ah forgot about that! Such a long time ago I feel this won't be added to the compiler in the near time unless someone does a PR or there's some more requests

@repnop
Copy link
Sponsor Contributor

repnop commented Oct 4, 2018

Currently working to try to get a PR to the main Rust repo to make this possible, I'll try to keep things updated here

@repnop
Copy link
Sponsor Contributor

repnop commented Nov 24, 2018

PR is open, hopefully this should be pulled in soon :) unfortunately will require nightly to actually generate the doctest execs once its merged in, but its progress!

@gnzlbg
Copy link

gnzlbg commented Nov 24, 2018

Awesome thank you! Please keep us updated!

@repnop
Copy link
Sponsor Contributor

repnop commented Dec 8, 2018

PR is getting very close to being merged! 👍

bors added a commit to rust-lang/rust that referenced this issue Jan 18, 2019
…eavus

rustdoc: Add option to persist doc test executables

Fixes #37048.

This is the initial version of the code so the doctest executables can be used for stuff like code coverage (specifically xd009642/tarpaulin#13) the folders it goes into were just a first idea, so any better ones are welcome.

Right now it creates a directory structure like:
```
  given_path/
          |_____ <filename>_rs_<linenum>/
          |_____ ...
          |_____ <filename>_rs_<linenum>/
                 |_____ rust_out

```
I couldn't figure out where it actually outputs the file w/ the name, I suspect its somewhere deeper in the compiler.

It also adds the unstable `--persist-doctests` flag to `rustdoc` that enables this behavior.
@TimDiekmann
Copy link

PR was merged 👍

@repnop
Copy link
Sponsor Contributor

repnop commented Jan 18, 2019

yep! the pull request actually is laying out a bigger foundation for more doctest improvements, e.g. the plan is to eventually have doctests be a single executable like regular tests, but there's a fair amount of work to do on that stuff, however this should hopefully allow tarpaulin to start taking them into account!

@TimDiekmann
Copy link

This would be awesome! Let's hope the nightly compiles today ;)

xd009642 added a commit that referenced this issue Mar 10, 2019
@xd009642
Copy link
Owner

It's finally here everyone! I've implemented optional coverage for doc-tests and tarpaulin now has a new --run-type flag where you can get coverage for just your tests (default), just doc-tests or both!

My only bug-bear is that --no-run currently isn't supported for doc-tests so I have to run them when I compile them and then again to get coverage results. But I've opened an issue to add in this behaviour rust-lang/rust#59053.

As always, changes have been merged into develop so feel free to try it there or wait until the next release (should be this week).

@TimDiekmann
Copy link

Doctests seems not be supported on workspaces. Is this a bug or a limitation by rustc?

@xd009642
Copy link
Owner

It might be a rustc limitation. Can you link me an example I can test with?

@TimDiekmann
Copy link

TimDiekmann commented Mar 11, 2019

Sure:

# New project
cargo new tarpaulin-test
cd tarpaulin-test/

# Create inner crate
cargo new inner --lib
echo -e "inner = { path = \"inner\" }\n\n[workspace]\nmembers = [\"inner\"]" >> Cargo.toml

# fill inner/src/lib.rs
echo -e '/// ```\n/// inner::test_func()\n/// ```\npub fn test_func() {\n   println!("hello world!");\n}' > inner/src/lib.rs


### Test with `-p inner`

# [ERROR tarpaulin] Failed to report coverage! Error: No coverage results collected.
cargo tarpaulin --run-types Doctests -p inner

# Works
cargo tarpaulin --run-types Tests -p inner

# 0% coverage
cargo tarpaulin --run-types Doctests Tests -p inner


### Test with `--all`

# [ERROR tarpaulin] Failed to report coverage! Error: No coverage results collected.
cargo tarpaulin --run-types Doctests --all

# Works
cargo tarpaulin --run-types Tests --all

# 0% coverage
cargo tarpaulin --run-types Doctests Tests --all


### Test with default members:

# Add default member:
echo 'default-members = ["inner"]' >> Cargo.toml

# [ERROR tarpaulin] Failed to report coverage! Error: No coverage results collected.
cargo tarpaulin --run-types Doctests

# Works (expected)
cargo tarpaulin --run-types Tests

# 0% coverage
cargo tarpaulin --run-types Doctests Tests

### Test from inner directory works fine:

# 100% coverage
cargo tarpaulin --run-types Doctests

# 0% coverage (expected)
cargo tarpaulin --run-types Tests

# 100% coverage
cargo tarpaulin --run-types Doctests Tests

@xd009642
Copy link
Owner

I've got it, it's down to how I've implemented it. I need to crawl packages for the doctest folders as well! Should be a simple fix just seeing if there are any niceties in the cargo API to make it easier

@xd009642
Copy link
Owner

Fixed and pushed to develop! Good catch 👍

@TimDiekmann
Copy link

TimDiekmann commented Mar 11, 2019 via email

@jonasbb
Copy link

jonasbb commented Mar 11, 2019

I have the same problem as TimDiekmann, but without a workspace. I have identical results with the nightly from 2019-02-21 and 2019-03-10.

Version of tarpaulin: https://github.com/xd009642/tarpaulin?branch=develop#3bae9fcb

I tested it on misc_utils and serde_with.

For misc_utils I get the error [ERROR tarpaulin] Failed to report coverage! Error: No coverage results collected..
For serde_with I get 0% coverage, even though around 70% is expected.

Results for misc_utils
$ cargo +nightly-2019-03-10 tarpaulin --out Html --run-types Doctests
[INFO tarpaulin] Running Tarpaulin
[INFO tarpaulin] Building project                                                                     
[INFO tarpaulin] Running doctests
                                                                                                      
running 2 tests                
test src/fs.rs - fs (line 12) ... ok                                                                  
test src/fs.rs - fs (line 30) ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

[ERROR tarpaulin] Failed to report coverage! Error: No coverage results collected.
Results for serde_with
$ cargo +nightly-2019-03-10 tarpaulin --out Html --run-types Doctests                                   
[INFO tarpaulin] Running Tarpaulin
[INFO tarpaulin] Building project
[INFO tarpaulin] Running doctests

running 19 tests                                                                                      
test src/lib.rs -  (line 58) ... ok
test src/lib.rs -  (line 41) ... ok                                                                   
test src/rust.rs - rust::StringWithSeparator (line 233) ... ok                                        
test src/flatten_maybe.rs - flattened_maybe (line 14) ... ok                                          
test src/rust.rs - rust::btreemap_as_tuple_list (line 1097) ... ok                                    
test src/rust.rs - rust::display_fromstr (line 26) ... ok                                             
test src/rust.rs - rust::double_option (line 327) ... ok                                              
test src/rust.rs - rust::hashmap_as_tuple_list (line 966) ... ok                                      
test src/rust.rs - rust::hashmap_as_tuple_list (line 997) ... ok                                      
test src/rust.rs - rust::maps_duplicate_key_is_error (line 592) ... ok                                
test src/rust.rs - rust::maps_first_key_wins (line 752) ... ok                                        
test src/rust.rs - rust::sets_duplicate_value_is_error (line 495) ... ok                              
test src/rust.rs - rust::string_empty_as_none (line 850) ... ok                                       
test src/rust.rs - rust::seq_display_fromstr (line 109) ... ok                                        
test src/with_prefix.rs - with_prefix (line 31) ... ok                                                
test src/rust.rs - rust::tuple_list_as_map (line 1192) ... ok                                         
test src/rust.rs - rust::tuple_list_as_map (line 1235) ... ok                                         
test src/rust.rs - rust::unwrap_or_skip (line 405) ... ok                                             
test src/with_prefix.rs - with_prefix (line 47) ... ok                                                

test result: ok. 19 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out                           

[INFO tarpaulin] Launching test
[INFO tarpaulin] running /home/jbushart/projects/serde_with/target/doctests/rust_rs_405/rust_out      
[INFO tarpaulin] Launching test
[INFO tarpaulin] running /home/jbushart/projects/serde_with/target/doctests/rust_rs_592/rust_out      
[INFO tarpaulin] Launching test
[INFO tarpaulin] running /home/jbushart/projects/serde_with/target/doctests/flatten_maybe_rs_14/rust_ou
t
[INFO tarpaulin] Launching test
[INFO tarpaulin] running /home/jbushart/projects/serde_with/target/doctests/rust_rs_850/rust_out
[INFO tarpaulin] Launching test
[INFO tarpaulin] running /home/jbushart/projects/serde_with/target/doctests/with_prefix_rs_31/rust_out
[INFO tarpaulin] Launching test
[INFO tarpaulin] running /home/jbushart/projects/serde_with/target/doctests/rust_rs_1192/rust_out
[INFO tarpaulin] Launching test
[INFO tarpaulin] running /home/jbushart/projects/serde_with/target/doctests/rust_rs_1235/rust_out
[INFO tarpaulin] Launching test
[INFO tarpaulin] running /home/jbushart/projects/serde_with/target/doctests/rust_rs_26/rust_out
[INFO tarpaulin] Launching test
[INFO tarpaulin] running /home/jbushart/projects/serde_with/target/doctests/rust_rs_966/rust_out
[INFO tarpaulin] Launching test
[INFO tarpaulin] running /home/jbushart/projects/serde_with/target/doctests/lib_rs_58/rust_out
[INFO tarpaulin] Launching test
[INFO tarpaulin] running /home/jbushart/projects/serde_with/target/doctests/with_prefix_rs_47/rust_out
{
  "player1_name": "name1",
  "player1_votes": 1,
  "player2_name": "name2",
  "player2_votes": 2
}
[INFO tarpaulin] Launching test
[INFO tarpaulin] running /home/jbushart/projects/serde_with/target/doctests/rust_rs_109/rust_out
[INFO tarpaulin] Launching test
[INFO tarpaulin] running /home/jbushart/projects/serde_with/target/doctests/rust_rs_495/rust_out
[INFO tarpaulin] Launching test
[INFO tarpaulin] running /home/jbushart/projects/serde_with/target/doctests/rust_rs_1097/rust_out
[INFO tarpaulin] Launching test                                                                 
[INFO tarpaulin] running /home/jbushart/projects/serde_with/target/doctests/rust_rs_327/rust_out
[INFO tarpaulin] Launching test                                                                 
[INFO tarpaulin] running /home/jbushart/projects/serde_with/target/doctests/rust_rs_997/rust_out
[INFO tarpaulin] Launching test                                                                  
[INFO tarpaulin] running /home/jbushart/projects/serde_with/target/doctests/rust_rs_233/rust_out
[INFO tarpaulin] Launching test                                                                 
[INFO tarpaulin] running /home/jbushart/projects/serde_with/target/doctests/lib_rs_41/rust_out
[INFO tarpaulin] Launching test                                                                 
[INFO tarpaulin] running /home/jbushart/projects/serde_with/target/doctests/rust_rs_752/rust_out
[INFO tarpaulin] Coverage Results:                                                              
|| Tested/Total Lines:         
|| src/chrono.rs: 0/63                                                                        
|| src/duplicate_key_impls/error_on_duplicate.rs: 0/20
|| src/duplicate_key_impls/first_value_wins.rs: 0/26                                            
|| src/json.rs: 0/4               
|| src/lib.rs: 0/4    
|| src/rust.rs: 0/97  
|| src/with_prefix.rs: 0/143                          
||                                                  
0.00% coverage, 0/357 lines covered

@xd009642
Copy link
Owner

Hmmm, with serde_with I wonder if that's due to the macros etc meaning the code shows up as external to the crate in dwarf and tarpaulin doesn't instrument it as a result. misc_utils looks like it should work though... I'll reopen this issue and give it a proper look tomorrow as it's late here!

@xd009642 xd009642 reopened this Mar 11, 2019
@jonasbb
Copy link

jonasbb commented Mar 12, 2019

@xd009642 Thanks so much for looking into it. For serde_with I use tarpaulin during CI on the regular tests and it works splendidly there. I only have external tests (in the tests-folder) and there work well. So the 0% coverage seem to be related to the doctest nature.

@xd009642
Copy link
Owner

Interestingly enough it looks like for some of the normal tests in serde_with it can't find instrumentation points and does nothing and for the rest of the tests it finds them. I'd hazard a guess that as a result there may be some misses in coverage and for whatever reason the normal tests don't pick up any lines might be the same issue the doc tests experience...

@xd009642
Copy link
Owner

@jonasbb I'm going to open another issue for serde_with and misc_utils as it seems to be something new and strange!

@jonhoo
Copy link
Sponsor

jonhoo commented Feb 6, 2020

@xd009642 Hmm, I think I'm missing something... https://github.com/jonhoo/hashbag has only doctests, and I believe they should give pretty much full coverage, yet reported coverage is 0%?

@xd009642
Copy link
Owner

xd009642 commented Feb 6, 2020

@jonhoo hmm I'll have to take a look at that. Hopefully I'll have some time this weekend. I'll tag you in another doctest issue on the off chance there could be some relevance (that one has coverage on weird lines)

kulp added a commit to kulp/lightning-sys that referenced this issue May 11, 2020
Ideally this would not be necessary, but gettin Tarpaulin to compute coverage
for doctests (xd009642/tarpaulin#13) is supported, but trickier than it first
appears. For me, the following command appears to hang during the compilation
of a transitively-required crate:

    cargo tarpaulin --verbose --run-types Tests,Doctests

so as a workaround to institute coverage, the doctests are duplicated as
explicit tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants