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

Incorrect coverage reporting 🐛 #7

Closed
atifaziz opened this issue Mar 22, 2018 · 11 comments · Fixed by #68
Closed

Incorrect coverage reporting 🐛 #7

atifaziz opened this issue Mar 22, 2018 · 11 comments · Fixed by #68

Comments

@atifaziz
Copy link

The coverage report for 87135a5 claims that DictionaryExtensions.cs has 75% coverage. It only has one method:

https://github.com/tonerdo/coverlet/blob/87135a5d0c3c5a27db83509fba01774dd20201d9/src/coverlet.core/Extensions/DictionaryExtensions.cs#L7-L14

According to the report, lines 9, 10 and 13 are covered but not line 12 (thus the 75%). How can 13 be covered but not 12? Is this a bug or am I misreading something? The only test for this method…

https://github.com/tonerdo/coverlet/blob/87135a5d0c3c5a27db83509fba01774dd20201d9/test/coverlet.core.tests/Extensions/DictionaryExtensionsTests.cs#L10-L17

…seems to cover it 100%.

@tonerdo
Copy link
Collaborator

tonerdo commented Mar 22, 2018

@atifaziz I have noticed this little kinks here and there. Currently trying to find the root cause, intend to have it fixed by this weekend

@atifaziz atifaziz changed the title Incorrect coverage reporting? 🐛 Incorrect coverage reporting 🐛 Mar 22, 2018
@OneCyrus
Copy link
Contributor

we saw similar issues here. some lines between covered ones where missing where it wasn't possible to branch.

@tonerdo
Copy link
Collaborator

tonerdo commented Mar 22, 2018

@OneCyrus @atifaziz This issue has been fixed in #10 and you'll see that the coverage for this repo increased as a result.

It was actually a pretty straightforward fix and a good side effect is it sets the foundation for branch coverage support. This fix will be in a new NuGet package I'll release this weekend, in the future I'll setup a nightly CI package publish so improvements can get out faster.

Cheers

@atifaziz
Copy link
Author

@tonerdo Thanks and looking forward to trying out the new package soon after it's up.

@tonerdo
Copy link
Collaborator

tonerdo commented Mar 26, 2018

@atifaziz New NuGet package release: https://www.nuget.org/packages/coverlet.msbuild/1.0.1

@tonerdo
Copy link
Collaborator

tonerdo commented Mar 26, 2018

@OneCyrus lemme know if the new NuGet package also fixes the issues you were seeing regarding coverage accuracy

@tonerdo tonerdo reopened this Mar 26, 2018
@OneCyrus
Copy link
Contributor

it improved here but there are still some missing parts. e.g.:

image

@tonerdo
Copy link
Collaborator

tonerdo commented Mar 26, 2018

Thanks for pointing that out. Any other examples you can show? A larger sample size is always helpful

@OneCyrus
Copy link
Contributor

here's another example:

image

it looks like it doesn't like LINQ. all the missing things I saw are related to LINQ.

@tonerdo
Copy link
Collaborator

tonerdo commented Mar 27, 2018

I'm not surprised, the IL the compiler generates for LINQ is horrendous. I'll take a closer look, thanks

@tonerdo
Copy link
Collaborator

tonerdo commented Apr 16, 2018

@OneCyrus kindly use the latest NuGet bits which has branch coverage support and see if this helps fix this issue. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants