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

Add an option to merge coverage of generics in the summary #43

Open
larsluthman opened this issue Aug 7, 2021 · 2 comments
Open

Add an option to merge coverage of generics in the summary #43

larsluthman opened this issue Aug 7, 2021 · 2 comments
Labels
C-enhancement Category: A new feature or an improvement for an existing one help wanted Call for participation: Help is requested to fix this issue

Comments

@larsluthman
Copy link
Contributor

First, thanks for a very useful tool!

It would be nice if there was an command line option to merge the coverage of different instantiations of generic functions in the summary of the coverge of a file, so that it would show 100% coverage if the tests covered all of a generic function, even if different regions of the function were covered by different instantiations. Consider the following source file:

fn smaller_than_default<T: Default + PartialOrd>(t: T) -> bool {
    if t < T::default() { true }
    else { false }
}

#[test]
fn test_instantiations() {
    assert!(!smaller_than_default(56.4_f32));
    assert!(smaller_than_default(-56_i32));
}

With cargo-llvm-cov 0.1.0-alpha.4, the report will show a region coverage of 88.89% (8/9), even though all regions of the generic function are marked as covered. The current rule seems to be that there needs to be at least one instantiation with 100% coverage for the generic function to be counted as 100% covered. This is probably a good choice for some situations, but sometimes you have a generic function that is difficult to cover completely with a single instantiation and for those cases it would be nice to have an option to merge the coverage of all instantiations when computing the summary.

@taiki-e
Copy link
Owner

taiki-e commented Aug 7, 2021

This seems reasonable to me.

Probably, we can implement this by adding a CLI option to disable -show-instantiations in the following code.

@taiki-e taiki-e added the C-enhancement Category: A new feature or an improvement for an existing one label Aug 7, 2021
@taiki-e
Copy link
Owner

taiki-e commented Aug 7, 2021

Probably, we can implement this by adding a CLI option to disable -show-instantiations in the following code.

Hmm, I'm not sure how this can be implemented on our side because the llvm-cov report command used to output the summary does not have a similar flag.

@taiki-e taiki-e added the help wanted Call for participation: Help is requested to fix this issue label Aug 7, 2021
bors bot added a commit that referenced this issue Aug 7, 2021
45: Add test for issue 43 r=taiki-e a=taiki-e

Add test for #43.

Co-authored-by: Taiki Endo <te316e89@gmail.com>
bors bot added a commit that referenced this issue Aug 15, 2021
61: Do not output summary with --html and --open r=taiki-e a=taiki-e

related #43

Co-authored-by: Taiki Endo <te316e89@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: A new feature or an improvement for an existing one help wanted Call for participation: Help is requested to fix this issue
Projects
None yet
Development

No branches or pull requests

2 participants