Tarpaulin: Inaccurate coverage estimates

[jonhoo]

This issue is mostly intended as a way to track things that are not yet implemented.

Consider the following coverage report:
https://codecov.io/gh/jonhoo/faktory-rs/src/1a0e356a4c785cb392d5b9d2ecab20415257cef4/src/consumer/mod.rs

There are a number of lines marked as not covered, despite that not being the case. The general cases I see:

• Constants in struct constructors: https://codecov.io/gh/jonhoo/faktory-rs/src/1a0e356a4c785cb392d5b9d2ecab20415257cef4/src/consumer/mod.rs#L148
• where keyword for generic functions: https://codecov.io/gh/jonhoo/faktory-rs/src/1a0e356a4c785cb392d5b9d2ecab20415257cef4/src/consumer/mod.rs#L197
• match on a variable: https://codecov.io/gh/jonhoo/faktory-rs/src/1a0e356a4c785cb392d5b9d2ecab20415257cef4/src/consumer/mod.rs#L301
• Chained iterator methods (#49 I assume): https://codecov.io/gh/jonhoo/faktory-rs/src/1a0e356a4c785cb392d5b9d2ecab20415257cef4/src/consumer/mod.rs#L419
• #[ignore] tests (unclear if that's correct): https://codecov.io/gh/jonhoo/faktory-rs/src/1a0e356a4c785cb392d5b9d2ecab20415257cef4/src/consumer/mod.rs#L577
• use inside closures: https://codecov.io/gh/jonhoo/faktory-rs/src/9ecfa6180ee7c3c7a59617fe7aabff19a47c7c19/src/proto/mod.rs#L168
• a random else: https://codecov.io/gh/jonhoo/faktory-rs/src/1a0e356a4c785cb392d5b9d2ecab20415257cef4/src/proto/single/resp.rs#L189
• Empty match arms: https://codecov.io/gh/jonhoo/faktory-rs/tree/dc97405642a6dde651d600e4a23032f721d91037#L54
• assert_eq! when always equal (presumably all macros where part of macro isn't executed): https://codecov.io/gh/jonhoo/faktory-rs/src/1a0e356a4c785cb392d5b9d2ecab20415257cef4/src/proto/single/resp.rs#L267
  
  
  
  • This one is actually pretty tricky. I think the right thing to do here is to mark any macros from other crates as covered if they are ever executed. Not clear what to do about crate-local macros, but that's probably a question for #61.
  
  
• A whole host of comments:
  
  
  
  • https://codecov.io/gh/jonhoo/faktory-rs/src/1a0e356a4c785cb392d5b9d2ecab20415257cef4/src/consumer/mod.rs#L285
  
  
  • https://codecov.io/gh/jonhoo/faktory-rs/src/1a0e356a4c785cb392d5b9d2ecab20415257cef4/src/consumer/mod.rs#L294
  
  
  • https://codecov.io/gh/jonhoo/faktory-rs/src/1a0e356a4c785cb392d5b9d2ecab20415257cef4/src/consumer/mod.rs#L335
  
  
  • https://codecov.io/gh/jonhoo/faktory-rs/src/1a0e356a4c785cb392d5b9d2ecab20415257cef4/src/consumer/mod.rs#L386
  
  
  • https://codecov.io/gh/jonhoo/faktory-rs/src/1a0e356a4c785cb392d5b9d2ecab20415257cef4/src/consumer/mod.rs#L469
  
  
• First line of //! comment: https://codecov.io/gh/jonhoo/faktory-rs/src/1a0e356a4c785cb392d5b9d2ecab20415257cef4/src/lib.rs#L1

There are also some lines that should be marked as not covered, but aren't:

• The try!-like ? operator: https://codecov.io/gh/jonhoo/faktory-rs/src/1a0e356a4c785cb392d5b9d2ecab20415257cef4/src/consumer/mod.rs#L221
• Chained iterator methods (also #49?): https://codecov.io/gh/jonhoo/faktory-rs/src/1a0e356a4c785cb392d5b9d2ecab20415257cef4/src/consumer/mod.rs#L545

[xd009642]

Nice comprehensive list! For the ignored tests the -i --ignored flag runs ignored tests. They're disabled by default to match cargo test behaviour.

Some of these are covered by #49 (though that may be because I use it as a mental catch up for chained method issues).

The ? operator I deliberately excluded because it either wasn't reliably hit or wasn't always recognised as an instruction (likely because of how it translates to assembly). I figured as it's essentially defined by the preceding command coverage of that is coverage of the ? as well. I may be wrong on that and change it but only if I can get the behaviour consistent.

Others like the where, good catch! I'll probably be on this issue a while, some of the work I'm doing for branch and condition coverage involves improving the instrumentation which should fix some of these issues. As that's a massive change, I'm going to prioritise that and hopefully get a bunch of these for free as a result.

p.s. the use inside closures link doesn't work, but I can recreate that on my own machine so don't worry too much. Just thought you might want to know for your own codecov use :+1:

[jonhoo]

Ah, for ignored tests, my question was more whether or not the code inside ignored tests themselves should be considered not covered in the resulting report. Currently they are considered uncovered code, but I'm not sure they should be?

Mmm, ? is tricky. I guess one question to try to answer is whether try! on its own line would be marked as covered? ? does the same thing. The reason I'd want it to be considered a "real" line is that it is an early return from the function, which marks what is likely a fairly different code execution.

Hehe, that's fine. I just figured it'd be useful to have a list of known shortcomings to refer to. No rush at all. I can go back and update this when you push the big change.

Updated the use inside closures link. I must have accidentally deleted a character...

[xd009642]

Ah makes sense on the ignored tests, that behaviour I reckon should be the preferred behaviour.

I'll have to think more about ?, it did just occur to me that it needs covering in branch coverage as it is a branch in the code. So I guess an answer may also emerge from my work on #61.

[xd009642]

Right making this into a todo list!

• [x] Constants inside struct initialisation
• [x] where keyword for generic functions
• [x] match on a variable (fixed but I didn't deliberately do anything to fix it...)
• [x] Chained iterator methods (seems to be all good!)
• [x] Code in #[ignore] tests
• [x] Random else being missed (didn't deliberately do anything..)
• [x] Empty match arms (example doesn't link to a file.. Can't find instances where it doesn't work)
• [ ] assert_eq! when always equal find out what causes this issue because assert_eq!(5,5) gets hit
• [x] Comments showing up as coverable
• [x] Another incorrect lib.rs line 1 covered
• [x] Decision on ?
• [x] Ignore all use statements
• [x] Ignore include! macro - used to include auto-genned files from build.rs for use by file
• [ ] fix collect::<_>()? and other issues with ? on function calls
• [ ] inline asm! - potentially just ignore this and ensure the block is hit.
• [x] the odd functions where body isn't hit link
• [x] unsafe on it's own line isn't covered
• [x] #[cfg(test)] on trait or impl methods
• [ ] unsafe blocks can still be weird, look into let x = unsafe {\n
• [ ] match arms with constants in

[xd009642]

As an update 2 more things ticked off! Though I've started seeing constants inside struct initialisation disappear as an issue with the previous release of tarpaulin so it may be a newer version of rustc actually fixes it! Either way I have a syntax rule for it in case that functionality disappears from rustc.

I've also decided ? on isn't a statement in it's own right but will be considered when I get branch coverage implemented!

5 more functional changes left. Though I'm going to try out the compiler and see if any others appear to now work!

Some of these have "solved themselves" it seems. This could either be me changing the rules for instrumenting the code during my bigger data structure refactoring or compiler updates. I'm marking them off until I can see evidence they're still issues though. Also, the empty match arm issue the example you linked doesn't seem to go to a file? Either way I haven't seen any examples that aren't covered so I'm checking it off

[xd009642]

@jonhoo I've forked faktory so you can see the progress, things looking a lot better :smile: https://coveralls.io/builds/16064460

[jonhoo]

Hmm, that doesn't look all that different from the coverage I saw on https://codecov.io/gh/jonhoo/faktory-rs/tree/master/src about two months ago though? Or am I missing something?

[xd009642]

It's only ~1.2% different, the change is bigger from 4 months ago when the issue was first opened though! And you can check in on the coveralls for my faktory fork to see future progress and any other lines that should be covered that aren't. After all, 2 more boxes to check then this issue gets closed.

[jonhoo]

While looking through the new run, I came across this:

https://coveralls.io/builds/16064460/source?filename=src/consumer/mod.rs#L259

I don't think this was on my original list, but should arguably also be marked as a bug. Specifically, the line in the function body, rather than the function signature, should be marked as not hit. Same here:

https://coveralls.io/builds/16064460/source?filename=src/consumer/mod.rs#L556

[xd009642]

That's now fixed, which has lead to a much bigger change in coverage on my coveralls of faktory! Turns out it was all over the place.. 2 left

[xd009642]

Well I've added 2 tickboxes and ticked one of them off. I'm pausing work on this issue for now as more pressing issues have been raised!

[TheDan64]

Hi! Thank you for all of your hard work improving coverage!

I just wanted to mention it seems to miss coverage of the first line of a (unsafe?) scope returning something like so:

let foo = unsafe { // Marked as not covered
    Foo() // Sometimes covered, not always
}; // Covered

See: https://codecov.io/gh/TheDan64/inkwell/src/ee1e70c7ccb2dd0374bddc2c0c4b43239a0d2569/src/context.rs

There's also a match in the above link on an option which states the None arm isn't covered even though it is. Also a rogue uncovered .collect() (Though this one is maybe related to your existing collect checkbox?)

[xd009642]

Yeah the collect is probably related to the existing checkbox. I've added two checkboxes for your existing examples. Thanks for pointing them out, I'm going to try and compile these real world examples into a set of programs covering all the coverage requirements I can derive.

Then hopefully that makes it easy to spot regressions or changes in behaviour that come with compiler updates (which have affected me in the past).

[onelson]

As per @TheDan64's report, I'm noticing this same "covered" line being reported for any block assignment. Example:

fn main() {                                                                       
    let name = {                                                                  
        "world"                                                                   
    };                                                                            
    println!("Hello, {}!", name);                                                 
}

Given a lack of tests, this would report "3, 5" for missed lines, but I would have expected the entire block to be counted as missed, not just the interior lines.

Easy to understand and shrug off, but when you're looking at a large body of code with many block assignments in it, it can lead to the report showing a long series of broken up ranges which could otherwise be seen as contiguous.

Somewhat related, I wonder how blank lines contribute to breaking up line ranges in a similar way. Not sure if there's a nice way to "roll up" uncovered line ranges that are broken by only newlines.

[xd009642]

Well with the ranges those are contiguous sequences of uncovered statements. To bundle things like empty lines on would make the uncovered lines seem larger than they are and would also mean the verbose terminal report doesn't match the coverage summary which I think could cause confusion.

Also, take into account the code snippet you posted there are only two instrumentable statements.

fn main() {     // Entry point - likely represented differently to other functions                                             
    let name = {      // This line and the two after it are a single statement                                                          
        "world"                                                                   
    };                                                                            
    println!("Hello, {}!", name);          // this is a single statement
}      // Exit of function can't be reliably instrumented.

So with this in mind I'm not convinced rolling this up is the best solution. Maybe there is scope to let the user know which functions are completely uncovered, however with things like inlined functions and templated functions this does get trickier.

Ultimately, the terminal reports are harder to parse but things like xml output and services like codecov.io and coveralls.io provide more user friendly interfaces so there are other options.

The reporting module is getting a big overhaul right now so after that's done I may ask your opinion of some of the new reporting formats and see if that helps solve the usability issue some. Also, if given the above justifications for why it's been done like this is you have any better alternatives I'm open to anything :smile:

[xd009642]

So a lot of good works been done on this issue but I'm moving coverage misses like this to #136 as it's a smaller issue and has some newer things. I've moved the things not covered in this one to that issue and I'm going to close this one as a duplicate in the meantime. :+1: