Skip to content

Branch coverage#413

Closed
ewjoachim wants to merge 1 commit into
mainfrom
branch-coverage
Closed

Branch coverage#413
ewjoachim wants to merge 1 commit into
mainfrom
branch-coverage

Conversation

@ewjoachim

Copy link
Copy Markdown
Member

(just some WIP I had lying around, for supporting branch coverage, #375 )

@github-actions

Copy link
Copy Markdown

End-to-end public repo

Admin commands cheatsheet:

  • /e2e (in approved PR review body): Trigger end-to-end tests on external contributions
  • /invite (in comment): Invite the author & admins to the end-to-end private repo

Comment on lines +96 to +100
if use_branch_coverage:
num_branches_covered = cast(int, num_branches_covered)
num_branches_total = cast(int, num_branches_total)
numerator = decimal.Decimal(num_covered + num_branches_covered)
denominator = decimal.Decimal(num_total + num_branches_total)

@ferdnyc ferdnyc Aug 30, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This math exactly checks out, with the raw_data.totals numbers and raw_data.totals.percent_covered produced by pytest-cov.

Comment on lines +91 to +93
use_branch_coverage: bool,
num_branches_covered: int | None,
num_branches_total: int | None,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Rather than conditionally including the branch numbers, you could just arrange to have them default to 0 if there's no branch data. If they're both 0 they won't affect the math, so they can just be added in unconditionally.

@ferdnyc ferdnyc mentioned this pull request Aug 31, 2024
@ewjoachim

Copy link
Copy Markdown
Member Author

Closing in favor of #466

@ewjoachim ewjoachim closed this Aug 31, 2024
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.

2 participants