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

Misses in coverage #351

Open
3 of 15 tasks
xd009642 opened this issue Feb 23, 2020 · 71 comments
Open
3 of 15 tasks

Misses in coverage #351

xd009642 opened this issue Feb 23, 2020 · 71 comments

Comments

@xd009642
Copy link
Owner

xd009642 commented Feb 23, 2020

Mention any false positives or false negatives in code coverage in this issue with an example where it occurred and it will be added to the list!

@jamestwebber
Copy link

It looks like my coverage report is counting the lines in tests as part of coverage (including one miss, somehow). See e.g. here if you scroll down to the lines 500+. Not sure whether that's intentional or not but seems a bit odd (and the miss is a false negative I guess)

@xd009642
Copy link
Owner Author

There's some weirdness with macros, I need to figure out the best way to handle them. Also if you want to exclude test code you can use --exclude-tests and all lines in anything with the #[test] attribute or #[cfg(test)] will be removed from the results

@pwoolcoc
Copy link

Just curious, why is --ignore-tests not the default? Are people really writing tests for their tests? I can't imagine a situation where I'd want my test code to be counted as part of the coverable source code

@xd009642
Copy link
Owner Author

Honestly just because as a feature it came later and no ones asked for the default to be switched around, I've got no opposition to switching the default behaviour. I also definitely don't think people are writing tests for their tests (or at least I hope not...)

The main use I can see is for integration tests that involve things like file or network IO some people might want to make sure their tests are actually executing the code as thoroughly as they expect

@mathstuf
Copy link
Contributor

I think this is another instance of the "split logical lines", but the attached report for serde.rs shows some more uncovered chain calls and unexpected (inlined?) not-to-be-covered lines at the very bottom.

tarpaulin-report.html.gz

@Kixiron
Copy link

Kixiron commented Mar 30, 2020

A variable declaration with the left-hand expression on a different line is counted as untested, see this report and this one. Additionally, conditionally compiled code that is not part of the current feature set seems to flag a false negative as well, as shown here

@xd009642
Copy link
Owner Author

xd009642 commented Apr 1, 2020

oh the feature flag is something I've definitely missed! Although I suppose I wonder if that should be filtered out by default, as ideally you should test all your features thoroughly and it is possible to organise different runs in a batch with different feature settings... Maybe I'll add a flag to add/remove them.

Also for the assign expressions thinking of something that maps to logical lines of code for solving that class of issues once and for all

@jtempest
Copy link

jtempest commented Apr 7, 2020

It looks like the inline, associated functions returning &'static str in this report only have the function signature marked and not the body that returns the string.

Also, thanks for tarpaulin, it's really handy and easy to set up :)

@recmo
Copy link

recmo commented Apr 12, 2020

Are people really writing tests for their tests?

I also definitely don't think people are writing tests for their tests (or at least I hope not...)

I can think of two examples:

  • Implementations of Quickcheck / Proptest test case generation. These implementations are sometimes complicated enough to warrant some testing.
  • Reference functions. Sometimes I write a slow-but-simple textbook test version of an algorithm (for example an FFT). I then use this to compare with the fast-but-complicated real implementation. But I first add a few tests to make sure the reference is correct.

But I don't care much about coverage in both cases, --ignore-test seems like a good default.

@jonhoo
Copy link
Sponsor

jonhoo commented Apr 12, 2020

Here's another coverage result that seems to have some odd lines marked as not covered:
https://codecov.io/gh/jonhoo/openssh-rs/src/9c82a1b11668033e2361d04f821786a41ce46615/src/lib.rs

The repository is over here: https://github.com/jonhoo/openssh-rs/

@jonhoo
Copy link
Sponsor

jonhoo commented Apr 21, 2020

I believe all of the misses here are erroneous:
https://codecov.io/gh/jonhoo/cliff/src/811bace3d7b1d7c64bdd316ab59d4e355e3d163c/src/lib.rs

@xd009642
Copy link
Owner Author

xd009642 commented Apr 21, 2020

GitHub API: Forbidden

😞

@jamestwebber
Copy link

^ I think that's just GitHub having issues today

@FeuRenard
Copy link

I have 2 covered lines containing doc comments using tarpaulin 0.11.1:
https://codecov.io/gh/FeuRenard/mygpoclient-rs/src/c5e4d61f21b548f2efd03429a4d35f62441c42c2/src/episode.rs

@kristiannotari
Copy link

kristiannotari commented May 9, 2020

False negatives here.

I have setup tarpaulin recently and tested it on my code and uploaded test coverage to Codecov. This is the actual report on Codecov (no custom formatting).

I have found that:

  • some write!( ... ) macros arguments, when those are split on multiple lines are flagged as misses
  • some assert!( ... ) macros arguments, when those are split on multiple lines are flagged as misses
  • some right hand sides of let ... assignments, when on other lines (even if self is on the same line of the let ...) are flagged as misses

Here's my travis-ci build configuration, if needed:

configuration
language: rust
sudo: required # required for some configurations
# tarpaulin has only been tested on bionic and trusty other distros may have issues
dist: bionic
addons:
  apt:
      packages:
          - libssl-dev
rust:
- stable
- beta
- nightly
jobs:
allow_failures:
  - rust: nightly
fast_finish: true
cache: cargo
before_script: |
if [[ "$TRAVIS_RUST_VERSION" == stable ]]; then
  cargo install cargo-tarpaulin
fi
after_success: |
if [[ "$TRAVIS_RUST_VERSION" == stable ]]; then
  # Get coverage report and upload it for codecov.io.
  cargo tarpaulin --out Xml
  bash <(curl -s https://codecov.io/bash)
fi

@khoek
Copy link

khoek commented May 9, 2020

I have a whole bunch of match self.blah { <newline> }s where the line containing the match itself is marked not-covered.

@CAD97
Copy link

CAD97 commented May 10, 2020

In a fn foo(&mut self, ...) -> &mut Self { ...; self }, the final line returning self is always listed as uncovered.

It seems to be any trailing expression that is just a borrow from self: report

@CAD97
Copy link

CAD97 commented May 11, 2020

The item headers for impls in statement position are incorrectly marked as uncovered. Lifting the impl blocks to item position properly does not consider these lines as uncovered. [example]

@mathstuf
Copy link
Contributor

@CAD97 That link is behind a login wall. Not everyone has already given access to codecov.io.

@CAD97
Copy link

CAD97 commented May 11, 2020

@mathstuf ah, sorry, I was unaware that that link required a login. (It should be public, the repository in question is public.) (If it was just that the second link was blank, it's because I had forgotten to pin it to a specific commit, which is fixed now.) Here's the linked snippets inline:

+   pub fn builder(&mut self) -> &mut Builder {
-       &mut self.cache
    }

+    pub fn add(&mut self, element: impl Into<NodeOrToken<Arc<Node>, Arc<Token>>) -> &mut Self {
+       self.children.push(element.into());
-       self
    }
+   fn visit_seq<Seq>(self, mut seq: Seq) -> Result<Self::Value, Seq::Error>
    where
        Seq: SeqAccess<'de>,
    {
+       if seq.size_hint().is_some() {
            struct SeqAccessExactSizeIterator<'a, 'de, Seq: SeqAccess<'de>>(
                &'a mut Builder,
                Seq,
                PhantomData<&'de ()>,
            );
-           impl<'de, Seq: SeqAccess<'de>> Iterator for SeqAccessExactSizeIterator<'_, 'de, Seq> {
-               type Item = Result<NodeOrToken<Arc<Node>, Arc<Token>>, Seq::Error>;
+               fn next(&mut self) -> Option<Self::Item> {
+                   self.1.next_element_seed(ElementSeed(self.0)).transpose()
                }

rraval added a commit to rraval/git-nomad that referenced this issue Dec 26, 2021
They don't really work well:
- `act` ships an ancient version of `git` that predates support for
  `--initial-branch`.
- Ran into misses in multiline coverage:
  xd009642/tarpaulin#351
@tokcum
Copy link

tokcum commented Nov 8, 2022

I tried tarpaulin on the code base of kodiak-taxonomy, a library I published recently and got fairly decent results. Thank you so much for this tool!

Sometimes tarpaulin missed "None" match arms, some of them being empty ( None => {} ), some of them having code in the None block ( None => { ... } ). I was wondering why and tried to create a minimalist example. At first I didn't succeed. In my example code tarpaulin reported the None match arm as covered.

It turned out that tarpaulin behaves differently as soon as any generics are involved. When I replace K with String in the following example the tarpaulin report is correct: all covered.

Anything I can do about this? Am I missing something? Thanks.

use std::fmt::Display;

pub struct User<K> {
    name: K,
}

impl<K> User<K>
where
    K: Display + Clone,
{
    pub fn set_name(&mut self, user: Option<User<K>>) {
        match user {
            None => {}
            Some(user) => self.name = user.name,
        }
    }
}

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

    #[test]
    fn set_name_some() {
        let mut me = User {
            name: "Tobias".to_string(),
        };
        let new_name = User {
            name: "Valentin".to_string(),
        };

        me.set_name(Some(new_name));
    }

    #[test]
    fn set_name_none() {
        let mut me = User {
            name: "Tobias".to_string(),
        };

        me.set_name(None);
    }
}

I also attach the tarpaulin report here.

tarpaulin-report.html.zip

@xd009642
Copy link
Owner Author

@tokcum, if you're using the ptrace engine (default on linux) it's just an unfortunate side-effect of instructions being in different locations and it being more sensitive to that. One potential solution is using --engine llvm and seeing if that improves things

@tokcum
Copy link

tokcum commented Nov 15, 2022

@xd009642, thank you. Changing the engine to LLVM did the trick. Great! I was not aware of the implications changing the engine.

@blacktemplar
Copy link

I have a similar issue as @tokcum but in my case --engine llvm doesnt help. Basically it is a generic function with a match where the blocks are multiline, minimal example:

pub fn is_some<T>(option: Option<T>) -> bool {
    match option {
        Some(_) => {
            return true;
        }
        None => {
            return false;
        }
    }
}

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

    #[test]
    fn test_none() {
        assert_eq!(is_some::<()>(None), false);
    }

    #[test]
    fn test_some() {
        assert_eq!(is_some(Some(())), true);
    }
}

If I run tarpaulin on this (regardless if with --engine llvm I get only 4 / 6 matched lines and the non-matching lines are the Some and the None lines:

image

dargor added a commit to dargor/dotfiles that referenced this issue Jan 11, 2023
Force LLVM engine to workaround some misses.
    xd009642/tarpaulin#351 (comment)

Signed-off-by: Gabriel Linder <gabriel@numberly.com>
@Nilstrieb
Copy link

It sometimes misses inlined functions: #1192

@jplatte
Copy link
Sponsor

jplatte commented Feb 15, 2023

Here's a few inaccurracies in one screenshot (code that's being shown there):

Screenshot_2023-02-15_122715

  • multi-line match arms can be partially covered
  • field access / method chains without any divergence partially covered
  • return not covered despite the preceding statement that doesn't diverge being covered

@LeTurt333
Copy link

LeTurt333 commented Feb 21, 2023

Is there a flag or config setting to ignore lines with normal, non-doc comments? I'm getting a large amount of false hits on lines with // comments

It seems arbitrary as well, I have comments containing plain text getting false hits, while I also have comments containing multiple "punctuation" type symbols which are ignored

False hit
// Pull incrementor ID

Ignored
//BUCKETS.save(deps.storage, (sender.clone(), &bucket_id), &new_bucket)?;

@j-tai
Copy link

j-tai commented Apr 29, 2023

Variable declarations like let x; are sometimes marked as not covered.

Reproduce:

#[inline(never)]
fn foo() {
    let x;
    x = 1;
}

#[test]
fn test() {
    foo();
}

Result:
image

@Razican
Copy link

Razican commented May 1, 2023

Not sure if this is one of the cases mentioned above:
https://app.codecov.io/gh/boa-dev/boa/pull/2885/blob/boa_unicode/src/lib.rs

Screenshot 2023-05-01 at 18 35 00

The matches!() is sometimes covered, sometimes not, then, the function call (first argument) is covered, but the pattern matching is not.

@kierankasha
Copy link

I noticed a little bug in a small crate (~100 lines) I'm working on. Comments without a newline before them are getting counted towards coverage. Adding a newline gave me +2.88% coverage. This is the command used:
cargo tarpaulin -v --release --engine ptrace --all-features --out html

This is an excerpt from the html report showing with and without the newline:
image
image

Not sure if this has been brought up before.

@clechasseur
Copy link

clechasseur commented Sep 16, 2023

Found this yesterday, not sure it has already been reported, but I couldn't find any reference to it:

The code: https://github.com/clechasseur/mini_exercism/blob/c4d0075a4dad0b6c9d1eb5efdcd10f36d560d7fa/src/api/detail.rs

Resulting coverage:
https://app.codecov.io/gh/clechasseur/mini_exercism/commit/c4d0075a4dad0b6c9d1eb5efdcd10f36d560d7fa/blob/src/api/detail.rs#L117

image

This is inside a macro_rules! as well as inside a paste! block.

It also comes up when invoking the macro, because the macro forwards the attributes:

The code:
https://github.com/clechasseur/mini_exercism/blob/c4d0075a4dad0b6c9d1eb5efdcd10f36d560d7fa/src/api/v1.rs#L17-L23

Resulting coverage:
https://app.codecov.io/gh/clechasseur/mini_exercism/commit/c4d0075a4dad0b6c9d1eb5efdcd10f36d560d7fa/blob/src/api/v1.rs#L21

image

Seems like tarpaulin believes the #[derive(Debug)] is a line to be covered.

EDIT: I tried adding a test that actually uses the implementation of Debug being generated and it does solve the issue (e.g. the line is now considered covered). It's weird however because I have lots of other #[derive(Debug)] on custom types not defined through macros that do not need to be covered.

@kemitix
Copy link

kemitix commented Nov 18, 2023

Fails to record coverage properly when functions are annotated with tracing::instrumentation from tokio's tracing crate.

image

and

image

@jplatte
Copy link
Sponsor

jplatte commented Nov 18, 2023

For tracing, we found that it helps a lot to set RUST_LOG=trace (assuming tracing-subscriber's EnvFilter is used), otherwise expressions inside of event! / trace! / info! and so on don't get evaluated, and tarpaulin correctly shows them as not covered.

@qsantos
Copy link

qsantos commented Nov 22, 2023

I'm also seeing this behaviour. Also break statements and enum match cases are uncovered.

@eloff It might be a bit late, but was this in generic code by any chance? I have both break statements and match cases not covered in generic code.

@jplatte Is your example from generic code?

I concur with @tokcum and @blacktemplar and I think that all these might have the same root cause. Maybe discussion about this case should be regrouped in #1078.

@jplatte
Copy link
Sponsor

jplatte commented Nov 22, 2023

I included a link with my example. It's in an impl block with lifetime generics, no type generics are involved.

@qsantos
Copy link

qsantos commented Nov 22, 2023

I included a link with my example. It's in an impl block with lifetime generics, no type generics are involved.

Sorry, I had missed that. It seems like lifetime generics are enough to trigger the behavior I have observed:

struct S1 { v: Option<i32>, }
fn f1<'a>() {
    let s = S1 { v: Some(0) };
    Box::new(S1 {
        v: s
            .v
            .map(|v| 42),
    });
}
#[test]
fn test1() { f1(); }

struct S2 { u: i32, }
fn f2<'a>() {
    Box::new(S2 {
        u: 0,
    });
}
#[test]
fn test2() { f2(); }

fn f3<'a>() {
    Some(0)
    .map(
        |
        v
        |
        42
    );
}
#[test]
fn test3() { f3(); }

gets me

image

@Sajjon
Copy link

Sajjon commented Dec 1, 2023

One silly, yet super important gotcha regarding accuracy: make sure you have not added:

[lib] test = false

In your Cargo.toml (and forgot about it). I had accidentally forgot about adding this and it resulted in my code coverage from 90% -> 60% (in a multi-crate-repo).

@Sajjon
Copy link

Sajjon commented Feb 21, 2024

@xd009642 I use UniFFI and it is heavily macro based, and a lot of its macros usages results in Tarpaulin flagging the line where I use the macro as uncovered (they are covered, but I guess Tarpaulin does not know about it)

See https://app.codecov.io/gh/radixdlt/sargon/blob/develop/src%2Fhierarchical_deterministic%2Fbip39%2Fbip39_passphrase.rs

Screenshot 2024-02-21 at 20 06 57

Also when I have declared my own macros often Eq derivation is missed:
https://app.codecov.io/gh/radixdlt/sargon/blob/develop/src%2Fprofile%2Fv100%2Fnetworks%2Fnetwork%2Fauthorized_dapp%2Fshared_with_dapp.rs#L16

@KGrewal1
Copy link

KGrewal1 commented Feb 28, 2024

seem to get missing coverage when a chain of methods is called: may be because they're being optimised out before the final code? https://app.codecov.io/gh/KGrewal1/optimisers/blob/master/src%2Flbfgs.rs

missing coverage on struct field instantiation on lines 146 definitely doesnt seem correct however?

Also some issues where every line is covered bar a final call to collect https://app.codecov.io/gh/KGrewal1/optimisers/blob/master/src%2Fadam.rs

@tyrone-wu
Copy link
Contributor

tyrone-wu commented Apr 4, 2024

Hi 👋 , nice work with this project! 😸

I believe there's missing coverage when using include-tests on #[tokio::test]. One example of it not being covered is here, in which our GHA workflow uses include-tests = true here.
The #[test] are covered fine, it just appears to be #[tokio::test] that's not being included.

Edit: resolved #1503. tyty! 😸

@tyrone-wu
Copy link
Contributor

tyrone-wu commented Apr 4, 2024

I went through the codebase and it looks like this line https://github.com/xd009642/tarpaulin/blob/develop/src/source_analysis/items.rs#L106 doesn't return true for #[tokio::test] since it's not considered and indent when number of path segments is greater than 1.

Edit: resolved #1503. tyty! 😸

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