tarpaulin icon indicating copy to clipboard operation
tarpaulin copied to clipboard

Line break in source code leads to uncovered lines

Open vmiklos opened this issue 2 years ago • 8 comments

Hi,

Describe the bug rustfmt likes to wrap long lines to multiple shorter ones. At the same time, sometimes such wrapping causes tarpaulin to report false uncovered lines.

To Reproduce

Create Cargo.toml like this:

[package]
name = "mypackage"
version = "1.0.0"
edition = "2018"

[dependencies]

and src/lib.rs like this:

pub struct S { 
}

impl S { 
    pub fn f(&self) -> Result<Vec<i32>, String> { 
        Ok(vec![42])
    } 
}

pub fn g(
    s: &S,
) -> Result<(), String> { 
    for _i in s
        .f()? {
    } 

    Ok(())
}

#[cfg(test)]
mod tests { 
    use super::*;

    #[test]
    fn t() { 
        g(&S{}).unwrap();
    } 
}

Finally run cargo tarpaulin -v.

Expected behavior

Current output:

Dec 03 23:34:04.882 DEBUG cargo_tarpaulin: set up logging
Dec 03 23:34:04.882  INFO cargo_tarpaulin::config: Creating config
Dec 03 23:34:04.887  INFO cargo_tarpaulin: Running Tarpaulin
Dec 03 23:34:04.887  INFO cargo_tarpaulin: Building project
Dec 03 23:34:04.887  INFO cargo_tarpaulin::cargo: Cleaning project
   Compiling mypackage v1.0.0 (/home/vmiklos/t)
     Running `rustc --crate-name mypackage --edition=2018 src/lib.rs --error-format=json --json=diagnostic-rendered-ansi --emit=dep-info,link -C embed-bitcode=no -C debuginfo=2 --test -C metadata=3c53dd0beed7c685 -C extra-filename=-3c53dd0beed7c685 --out-dir /home/vmiklos/t/target/debug/deps -C incremental=/home/vmiklos/t/target/debug/incremental -L dependency=/home/vmiklos/t/target/debug/deps -C debuginfo=2 --cfg=tarpaulin -C link-dead-code`
    Finished test [unoptimized + debuginfo] target(s) in 0.48s
Dec 03 23:34:05.486  INFO cargo_tarpaulin::process_handling::linux: Launching test
Dec 03 23:34:05.486  INFO cargo_tarpaulin::process_handling: running /home/vmiklos/t/target/debug/deps/mypackage-3c53dd0beed7c685

running 1 test
test tests::t ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.03s

Dec 03 23:34:05.702  INFO cargo_tarpaulin::report: Coverage Results:
|| Uncovered Lines:
|| src/lib.rs: 14
|| Tested/Total Lines:
|| src/lib.rs: 7/8 +0.00%
|| 
87.50% coverage, 7/8 lines covered, +0% change in coverage

Expected output: no uncovered lines. Note that in case

    for _i in s
        .f()? {

is folded into a single line, then tarpaulin correctly reports no uncovered lines.

Version:

$ cargo tarpaulin --version
cargo-tarpaulin version: 0.18.5

Thanks!

vmiklos avatar Dec 03 '21 22:12 vmiklos

An observation I had when looking into a similar issue in my code is that sometimes adding a // ... comment at the end of the false uncovered line magically gets it removed from cobertura.xml (instead of having it exist with hits=0). I immediately tried it out on this minimal example but no, this doesn't happen here. However I swear do see this effect it in my non-trivial, owned-by-employer code. I just tested and re-tested this. Some strange butterfly effect on the compiler?

orenbenkiki avatar Dec 29 '21 16:12 orenbenkiki

Rather than raise a new issue, I'm pretty sure I've got the same type of problem occurring to me. I wrote it up on StackOverflow on the assumption that I was at fault.

jmacadie avatar Feb 19 '22 09:02 jmacadie

Running code in the debugger it seems like this might not be tarpaulin's fault as such, but rather that the line information emitted by the Rust compiler itself is inaccurate. If this is the case the issue should be opened in rustc, I guess?

orenbenkiki avatar Feb 19 '22 09:02 orenbenkiki

I think a large aspect of the compilation stage involving this is likely in LLVM. Either way it's not something I think could provide a stable interface, and the rustc advice might be use just use the llvm coverage built in. There are some approaches for most of the issues though:

  1. Update source_analysis module to filter out things that are unreliable (I currently do)
  2. Look at the lines we get from debug info and the assembly instructions and see if it's clear if a line is instrumentable and if not filter it out (I've not tried this).
  3. Use the llvm coverage (I am working on a tarpaulin backend for this)

For think problem I think the best solution is just to add to the source analysis module to get the for expressions to be registered as a single logical line which I will make a note of to do before the next release! (I could try milestone issues but I forget about them)

xd009642 avatar Feb 19 '22 15:02 xd009642

I assume that llvm coverage will solve this: if I understand correctly, then the instrument-coverage switch will be stabilized in the next rust release and cargo llvm-cov does not show uncovered lines for the problematic input from the top of this issue.

vmiklos avatar Feb 20 '22 14:02 vmiklos

I experienced what I believe is this same issue when a tuple was broken across lines. Private repo; apologies for not being able to link directly. Image below:

Screen Shot 2022-06-20 at 5 56 39 PM

Here, the tuple (Status::Unauthorized, AuthorizationError::OBONotAllowed,) is passed to Outcome::Failure (in the Rocket framework fwiw), and Tarpaulin marks only one line -- the first element of the tuple -- as uncovered.

jblachly avatar Jun 23 '22 00:06 jblachly

My immediate need for this is gone. Do you prefer to keep the issue open (since it's still a bug) or OK to close this, so you spend your time on issues which are useful to fix in practice?

vmiklos avatar Dec 02 '22 08:12 vmiklos

Well, I for one would still like to see more stable output, if possible.

orenbenkiki avatar Dec 02 '22 08:12 orenbenkiki