--codecov: change how regions are calculated

[andrewgazelka]

The definition of a region is now very conservative. A region is now defined by

/// The location of a region
#[derive(Debug, Copy, Clone, Hash, Eq, PartialEq)]
pub(crate) struct RegionLocation {
    start_line: u64,
    end_line: u64,
    start_column: u64,
    end_column: u64,
}

If that location is hit even once, it is marked as covered.

Quickly looking at the results, it appears to work properly. However, this should probably be tested more before it is merged. https://app.codecov.io/gh/getcollective-ai/collective/tree/codecov-better-region?displayType=list

TODO

cargo-llvm-cov/src/json.rs

Line 89 in 876abc0

*covered = *covered || region.execution_count() > 0; // TODO: maybe this should be compared to func_count?

[geeklint]

Out of curiosity, do you know why I might run into the issue if I'm not testing generic code? My report shows over 97% locally vs less than 28% on codecov (seemingly because of untested instantations) but I'm not sure why functions would get instantiated multiple times if they're not generic?

[andrewgazelka]

Out of curiosity, do you know why I might run into the issue if I'm not testing generic code? My report shows over 97% locally vs less than 28% on codecov (seemingly because of untested instantations) but I'm not sure why functions would get instantiated multiple times if they're not generic?

It's difficult for me to pinpoint the exact cause without a deeper understanding of the raw JSON format, which is later processed into the codecov format. However, I can still offer some insights that might be helpful.

Even though your code might not involve generics, the changes in this PR could still impact it. For instance, under the new code, a region is considered hit as long as it's executed once, whereas the old code required a region to be executed at least a specific number of times (defined by func_count) to be considered valid.

I'm not entirely sure if func_count represents the number of regions between lines or simply how many times a function was instantiated, but it's worth looking into. If you could provide more context or perhaps share the JSON format and relevant code, I'd be happy to help you dive deeper into this issue and explore potential solutions.

[andrewgazelka]

In addition, does this PR fix the coverage?

[geeklint]

The repo is here, I hadn't had a chance to test this PR myself yet.

https://github.com/geeklint/typeid-set

[andrewgazelka]

The repo is here, I hadn't had a chance to test this PR myself yet.

geeklint/typeid-set

Hey there @geeklint,

Whenever you have some time, I'd be grateful if you could give it a go and share your experience with me. It's as simple as installing this custom cargo-llvm-cov in your GitHub actions, just like I did in this example:

https://github.com/getcollective-ai/collective/blob/main/.github/workflows/rust-test.yml#L61-L62

Feel free to reach out if it doesn't solve your problem, and I'll be more than happy to help! Fingers crossed it does the trick, though. 🙂

[geeklint]

Alright, I had a chance to check it out. It seems the data is at least consistent now, with the only difference being how the respective services count lines.

The region I am missing covers two "real" lines of code, but the braces on the surrounding lines get excluded as "partially" covered. I can't imagine there's a consistent way to adjust a region to only the meaningful bits though, so I would suggest this is working as can be expected.

I wonder if the documentation for cargo-llvm-cov should be adjusted, since reading the line "By using --codecov flag instead of --lcov flag, you can use region coverage on Codecov:" in the README gave me the impression codecov.io would let me report the actual region coverage, whereas it seems it's more like it uses the collected region coverage data to determine a final line coverage, unless I'm missing something?

[andrewgazelka]

@geeklint Unfortunately, CodeCov does not support region coverage and only offers branch coverage, while Rust LLVM coverage only works with region coverage. Branch and region coverage are quite similar, so it would be interesting to explore creating a mapping between the two in the future. As you mentioned, accurately highlighting the relevant part of a line would be more helpful than displaying the entire line in red, yellow, or green. @taiki-e, do you think creating such a mapping would be valuable?

Regarding the term "line coverage," using it may lead to confusion, as it typically does not imply partial coverage support. However, if you have examples that suggest otherwise, please feel free to share them.

[andrewgazelka]

ok, so I looked into this a little bit and a branch basically is the same as a region except for the FalseExecutionCount field

https://github.com/llvm/llvm-project/blob/llvmorg-16.0.0/llvm/tools/llvm-cov/CoverageExporterJson.cpp#L92-L98

maybe we could just set ExecutionCount = FalseExecutionCount?

[andrewgazelka]

Working on fake branch here: https://github.com/andrewgazelka/cargo-llvm-cov/tree/fake-branch-coverage

[andrewgazelka]

I looked into this more, and about 30 minutes to try to get fake branches to work, but couldn't. I think we should just merge this for now.

[andrewgazelka]

@taiki-e, what are your thoughts on merging this? Please let me know if you require further testing or changes.

[taiki-e]

@geeklint

whereas it seems it's more like it uses the collected region coverage data to determine a final line coverage

AFAIK, this is how codecov handles lines that are partially covered in branch coverage, etc. (see also #20 (comment) and later 3 comments). Since LLVM's region coverage is a subset of branch coverage (see this LLVM patch for more), the --codecov flag maps region coverage like branch coverage.

To be honest, I don't think the way codecov shows branch coverage is great, but some people indeed prefer it (#86), and I don't expect codecov to change it in the near future.

If you want to get decent region coverage reports with CI, consider generating HTML reports and deploying them to github pages (e.g., by using actions/deploy-pages), github artifacts (e.g., by using actions/upload-artifact), netlify, etc. -- This approch is what my colleagues (and I) use.

---

@andrewgazelka

As you mentioned, accurately highlighting the relevant part of a line would be more helpful than displaying the entire line in red, yellow, or green. @taiki-e, do you think creating such a mapping would be valuable?

I believe HTML reports and text reports already do that.

[taiki-e]

LGTM, thanks @andrewgazelka!

[taiki-e]

Published in 0.5.16.