Fix branch coverage rate comparison when used with --branch without actual branches
#37
Conversation
…ut there are no branches When `--branch` option is set but there are no branches `coverage.xml` will contain 0 branches but have branch-rate="1". Fixes smarie#23
marcegeek
changed the title
--branch without actual branches--branch without actual branches. Fixes #23
marcegeek
changed the title
--branch without actual branches. Fixes #23--branch without actual branches
|
Thanks @marcegeek ! I just pushed a pr that should hopefully have fixed the layout and ci. would you mind updating this branch and push again ? |
…t there are no branches
…anch` situations into account This makes behaviour consistent so the previous comparison was restored.
| # detect whether the --branch option were set or not | ||
| # so CoverageStats knows how to distinguish between them | ||
| cov.branch_option = cov.branches_valid > 0 or branch_rate == 1.0 |
There was a problem hiding this comment.
(nitpick)
If branch_option is only computed based on the other two fields according to this formula, could we implement it as a @property instead of a field ? That way it would not need to be present in the constructor arguments, nor to be late-set here.
There was a problem hiding this comment.
I'm not sure, as cov.branch_rate is computed as a @property while the original branch-rate="1" and branches-valid="0" fields from the XML are what we need to discover whether the --branch option was set or not. Wouldn't that require to store the original branch_rate from the XML? As a _branch_rate/_original_branch_rate/_xml_branch_rate field maybe?
There was a problem hiding this comment.
oh I see your point. And I see that CoverageStats was already entirely implemented in this weird "init as none - then fill" pattern. I do not remember where this class was copied from but indeed making it "nice" would require to change all of it, not just this new feature.
Not worth the effort IMO :) let's keep it like this for now.
|
Great, thanks @marcegeek ! Just one minor comment, otherwise this is "good to go" for me |
Co-authored-by: Sylvain Marié <[email protected]>
When
--branchoption is set but there are no branchescoverage.xmlwill contain 0 branches (branches-valid="0" and branches-covered="0") but have branch-rate="1". Fixes #23