ci: enable coverage for pull requests

[mizvekov]

No description provided.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.46%. Comparing base (e2c8aac) to head (4cf8769).

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1065      +/-   ##
===========================================
+ Coverage    85.14%   85.46%   +0.31%     
===========================================
  Files          193      194       +1     
  Lines        20928    22244    +1316     
===========================================
+ Hits         17820    19010    +1190     
- Misses        3108     3234     +126     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[cppalliance-bot]

An automated preview of the documentation is available at https://1065.mrdocs.prtest2.cppalliance.org/index.html

If more commits are pushed to the pull request, the docs will rebuild at the same URL.

2025-10-09 22:59:28 UTC

[alandefreitas]

This looks great.

This action already embeds that feature that fails on reduced coverage, right?

Should all PRs be blocked by that? I know most should. What do you think?

[mizvekov]

I am testing how that behaves, but I think if we need to fine tune that behavior, we can leave it to another PR and have this one be just about having coverage reports on PRs.

I think it's reasonable for decreased coverage to block, if it can still be overridden intentionally.

We may need to adjust the logic about how it counts decreased coverage.

It can't be about decreased percentage covered, because that can block NFC PRs which reduce amount of covered lines.
I think the logic should be about blocking when adding uncovered lines.

There are several options to choose here, and this needs some study and experiments.

[alandefreitas]

Good idea. The GitHub action may have a reasonable threshold. In parallel, I should check if there's a way for GitHub to allow merging PRs that fail CI under exceptional circumstances.

[mizvekov]

You can see here it does fail the PR: #1066

But it can still be merged without any hassle.

I think this can be improved through GitHub configuration changes.

The LLVM Repo has a setting where there is a check box you have to mark saying you want to "bypass the rules", so it looks less prone to accidents.

[alandefreitas]

You can see here it does fail the PR: #1066

That's valuable information. Now I’m the one having trouble keeping my side of the promise:

Once I set up the repository so that no merge could be done unless everything passed CI. My plan now was to tweak something there to handle the coverage issue, because we know that from time to time this test gives us false negatives (or positives, depending on how you define it) and blocks the merge.

I can’t seem to find where that configuration is anymore. Maybe something changed and that setting doesn’t even exist now, since you already tried it and it seems to let you merge. But I’d still like other cases to block the PR.

Do you have any suggestion for how to achieve that?

[mizvekov]

Yes, you have to go into the branch settings in the GitHub settings page for this repo, and enable this:

That should probably be done for the master and develop branches.

Then I believe you have to set up so every developer role has the bypass branch protection permission.

Then maybe only allow bypassing the rules on the develop branch, but not on master. Should be this setting on the same page:

[alandefreitas]

Oh... Yes. That's the setting we had. So someone must have just removed it altogether without saying anything. 😐😞😩