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

feat: Make pycobertura branch-aware and count partially hit branches as missed #168

Open
wants to merge 27 commits into
base: master
Choose a base branch
from

Conversation

aconrad
Copy link
Owner

@aconrad aconrad commented May 21, 2023

Addresses #167.

Now that partially hit branches are marked as missed, pycobertura might pick up more missing lines if the XML report contains branching data. Here is an example of the same XML file with branching data before and after the change:

Before

Filename Stmts Miss Cover Missing
Main.java 15 0 100.00%
search/BinarySearch.java 12 1 91.67% 24
search/ISortedArraySearch.java 0 0 100.00%
search/LinearSearch.java 7 2 71.43% 19-24
TOTAL 34 3 90.00%

After

Filename Stmts Miss Cover Missing
Main.java 15 0 100.00%
search/BinarySearch.java 12 2 91.67% 23-24
search/ISortedArraySearch.java 0 0 100.00%
search/LinearSearch.java 7 4 71.43% 13, 17, 19-24
TOTAL 34 6 90.00%

Note that the number of missed lines also goes up. If the branch is covered at 50%, then we just say it's missing.

Copy link

@kinow kinow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @aconrad

Thanks heaps for working on a prototype so quickly ⭐

I had a look at your example of the Java report, then checked out this branch and tested on my current Python project. It appears to be showing a lot more lines missing 🙂

Are you planning to add both line & branch coverage? I think the pytest-cov HTML report shows both the old stmts/missing, as well as branches/covered.

Thanks!

@aconrad
Copy link
Owner Author

aconrad commented May 21, 2023

Are you planning to add both line & branch coverage? I think the pytest-cov HTML report shows both the old stmts/missing, as well as branches/covered.

I'm not convinced I will. What is valuable information from the coverage.py html report that cannot be inferred from the pycobertura report as it currently is? The extra columns in the coverage.py (excluded, branches, partial) don't feel useful to me, it's additional statistics that get in the way of the important data.

Here are the pycovertura and coverage.py reports side-by-side:

pycobertura show in HTML

Screen Shot 2023-05-21 at 11 56 45 AM

(I omitted the source output)

coverage.py html

Screen Shot 2023-05-21 at 11 57 30 AM

When reviewing both, it's likely that the engineer will go look at one of the "Missing" lines and understand what needs to be covered in their next test. What do you think?

Side note: Currently, the percentages are slightly off because coverage.py calculates branch coverage differently, whereas pycobertura takes the percentage value straight from the XML file. So while pycobertura/cli.py shows 100% coverage in the pycobertura report (since all lines were executed), the coverage of coverage.py shows the ratio adjusted for the partially covered branches. I guess we could override the percentage value in the XML report and calculate "Miss" / "Stmts".

I'll likely make the partially covered lines in the "Missing" column yellow but I'll have to do some refactoring for this to happen. Maybe in a subsequent PR.

`coverage.xml` is listed in `.gitignore`
@kinow
Copy link

kinow commented May 21, 2023

When reviewing both, it's likely that the engineer will go look at one of the "Missing" lines and understand what needs to be covered in their next test. What do you think?

I agree some may use it that way. For me, I would like to know among the missing/not-covered code if there is any that's missing branch coverage.

I'll likely make the partially covered lines in the "Missing" column yellow but I'll have to do some refactoring for this to happen. Maybe in a subsequent PR.

That won't be helpful in my case, I think. At the moment I get the Markdown report of pycobertura and post it to the GitHub REST API to create a comment on pull requests in this private repository. I don't believe the yellow color will be displayed in these comments.

So without having the different color, if I understand correctly, the only difference after upgrading to this version of pycobertura would be a change in the missing column, but engineers wouldn't be able to tell (in our case) whether that's because of missing branch coverage or not.

I hope that makes sense.

Side note: Currently, the percentages are slightly off because coverage.py calculates branch coverage differently, whereas pycobertura takes the percentage value straight from the XML file.

👍

Thanks!

@aconrad
Copy link
Owner Author

aconrad commented May 21, 2023

That won't be helpful in my case, I think. At the moment I get the Markdown report of pycobertura and post it to the GitHub REST API to create a comment on pull requests in this private repository. I don't believe the yellow color will be displayed in these comments.

I agree that the color doesn't work everywhere since not all reports produce color. We should have another indicator to let users know that a line is partially covered. What if we annotated the missing line with a special symbol? In English, the tilde is informally used to mean "approximate", or "about", or "around" (e.g. it will take ~5 minutes to cook). Since the coverage is only approximate for partially covered lines, we could have ~183. What do you think?

@aconrad
Copy link
Owner Author

aconrad commented May 22, 2023

@kinow Here's a sample output in markdown using the tilde ~ as the partial marker, what do you think?

Filename Stmts Miss Cover Missing
pycobertura/init.py 2 0 100.00%
pycobertura/cli.py 90 1 98.89% ~133
pycobertura/cobertura.py 214 7 96.73% ~336, ~450, 451-452, ~458, 459-460
pycobertura/filesystem.py 87 0 100.00%
pycobertura/reporters.py 234 4 98.29% ~337, ~364, ~377, ~398
pycobertura/utils.py 147 3 97.96% ~46, 47, ~183
pycobertura/templates/init.py 0 0 100.00%
pycobertura/templates/filters.py 14 0 100.00%
TOTAL 788 15 98.10%

@kinow
Copy link

kinow commented May 22, 2023

@kinow Here's a sample output in markdown using the tilde ~ as the partial marker, what do you think?

I think I still miss being able to see how much is branch-covered or not. I can see the numbers for line coverage (stmts / miss), but not branch (branch / partial).

For others that might not be an issue, so if you prefer to keep it that way I think it's OK. For our setup I will probably keep using htmlq + pandoc to have the same information we have in pytest-cov (which I think might be fine, you don't need to modify pycobertura to be 100% equal to pytest-cov 🙂 )

Thanks

@aconrad
Copy link
Owner Author

aconrad commented May 22, 2023

I think I still miss being able to see how much is branch-covered or not. I can see the numbers for line coverage (stmts / miss), but not branch (branch / partial).

Okay. Can you elaborate more on how the counts of branches and partials are useful to you and your team? I might not fully understand the context in which you use this information.

@nbauma109
Copy link
Contributor

nbauma109 commented Mar 31, 2024

I've tested this PR on one of my projects and it looks good to me.
The yellow highlights appear in the right locations.
Only the yellow line numbers weren't visible on screen so I suggest changing the color to .yellow {color: #FFD700}.
The tooltip "X of Y branches missed" should be done in a separate PR, I think.

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 this pull request may close these issues.

3 participants