aconrad / pycobertura

A code coverage diff tool for Cobertura reports
MIT License
114 stars 39 forks source link

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

Open aconrad opened 1 year ago

aconrad commented 1 year ago

Addresses https://github.com/aconrad/pycobertura/issues/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.

aconrad commented 1 year ago

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.

kinow commented 1 year ago

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.

:+1:

Thanks!

aconrad commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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 :slightly_smiling_face: )

Thanks

aconrad commented 1 year ago

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 commented 5 months ago

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.