Miscalculating branch coverage

[ewjoachim]

In this discussion, "branch coverage" means the thing that lets you check if all paths through an if
statement are covered. Completely unrelated to git branches.

I think in some case we're miscalculating branch coverage:

• Without branches, coverage is computed as number of executed statements / total number of statements
• With branches, I think it's currently sometimes computed as (executed statements + fully executed branches)/(total statements + total branches) though I think it's not always consistent and sometimes, it's computed as if branches weren't here
• And I'm not sure how it should be computed, or dependening on the how, if it's even possible to do it from the json report.

In #211 , I'm removing branch coverage completely from the equation. But it might be worth investigating if we want to re-add it, though I believe it might be too complex to express in simple terms all the notions, especially given that "branch" is used both for if and git branch.

For now I'll just open this discussion.

[ferdnyc]

I think in some case we're miscalculating branch coverage:

That certainly seems to be true. Pydot is now using the action in its CI (pydot/pydot#395), and our most recent run — from a set of pytest runs with branch coverage enabled — has the following values in data.json:

{
  "coverage": 86.44927536231884,
  "raw_data": {
    "totals": {
      "percent_covered": 83.22649572649573,
      "percent_covered_display": 83
      "num_branches": 492,
      "num_partial_branches": 61,
      "covered_branches": 365,
      "missing_branches": 127
    }
  }
}

So the badge is showing 86%, but the HTML report shows numbers that consistently work out to 83%.

It's not the end of the world, but the inconsistency is a bit disconcerting. Is there a reason the action can't just "believe" the raw_data.totals.percent_covered value?

[ferdnyc]

(If you're interested in the full raw data, it's at the obvious spot: https://raw.githubusercontent.com/pydot/pydot/python-coverage-comment-action-data/data.json )

[ewjoachim]

Is there a reason the action can't just "believe" the raw_data.totals.percent_covered value?

Ah, I'm not 100% sure it's the best decision (but I'm not 100% sure either the alternative is better) but the reason is explained in:
#361 (comment)

TL;DR:
I like the pycoverage lib, but I think our reliance on that lib is causing growing pains: mainly that the .coverage data file is not meant to be an exchange format, and the fact that we run coverage operations ourselves mean that we may need to actually integrate with the project more that we can afford to (e.g. support coverage plugins). Similarily, if the coverage report mentions files that don't exist in the repo (e.g. auto-generated files), we may end up crashing.

For all this, at some point, I'm considering switching to a format that's more independent from the pycoverage, such as cobertura.xml, using pycobertura. (This would also have the benefit to make us compatible with non-python projects too!)

In the mean time, I'm quite wary of relying on more data provided by pycoverage, especially when it's just a convenience from calculating it ourselves from the raw data. It can be a constraint that the new tool & format we decide to use in the future must provide raw branch data. But I'd like to avoid the additional constraint that it must provide branch totals & subtotals.

Thus, I'm more comfortable with the idea that what we need is the raw data, and then we're not using more than that.

It's not a hard decision, and I'm not 100% it's the right one, but my gut feeling says that.