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

Code coverage is adding extra lines that are not part of the diff #15

Open
adilansar opened this issue Aug 2, 2021 · 2 comments
Open

Comments

@adilansar
Copy link

Hi Jan,

First of all thanks for this gem, I was just trying to integrate it in my private repo to improve code coverage.
While pronto-undercover works well for new files it is causing some issues when some code is added to existing method.
The runner lists few extra lines that are not part of the diff.

I tried to do some research and found out that when patch_to_undercover_message(patch) is called it calls offending_line_numbers per patch which internally calculates uncovered lines and returns first line of that uncovered patch.

Then in the loop of the object returned by method call offending_line_numbers(patch) we are again checking uncovered lines but this time the uncovered lines returned are simply all the lines from first and last line of that particular patch. I can see that undercover's coverage method is called for this. This seems unnecessary and incorrect, as the correct line numbers (which are only part of the diff) can be returned using offending_line_numbers(patch) this call which can instead of returning [warning, first_line_no] return [warning, uncovered_lines] and which further can be used to prepare the output.

Let me know if you wan't to discuss this.

@grodowski
Copy link
Owner

Hi! I'm glad you found my creation useful and want to use it in your project 😄

The runner lists few extra lines that are not part of the diff.

This is possible with Undercover and the answer may be hidden in what's provided by Undercover::Report, without pronto-undercover. undercover_warnings variable contains flagged chunks of code (Undercover::Result which represent methods, blocks, classes, modules) that were parsed and matched against the coverage report. This is desired behaviour, so that e.g. methods that were changed, but miss coverage slightly outside of the diff will still be brought to your attention in the pull request.

Moving to this gem, it has to conform to the Pronto::Runner and Pronto::Message interfaces, which is why I used PatchChangeset instead of undercover's own git interface in the first place. This results in pronto-undercover pinning comments to the final line of each flagged method/class/block.

Please tell me if that makes sense and let's discuss further! I am welcoming contributions to this and other undercover gems

@adilansar
Copy link
Author

I think in that case undercover should have an option which can be passed where I can specify if I need exact report or a rough report which includes coverage outside of the diff.
What do you think is the right thing to do here?

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

No branches or pull requests

2 participants