Skip to content

fix: replace node20 very_good_coverage with lcov threshold check #809

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
talalashraf wants to merge 3 commits into
base: development
Choose a base branch
from
+16 −4
Open

fix: replace node20 very_good_coverage with lcov threshold check #809

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2f3b2e4
fix: replace node20 very_good_coverage with awk lcov threshold check
talalashraf Jul 7, 2026
18df494
fix: make lcov coverage check fail closed on empty report
talalashraf Aug 24, 2026
821134c
refactor: use github-script for coverage check instead of awk
talalashraf Aug 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,23 @@ jobs:
path-to-lcov: ./contracts/zero-ex/lcov.info

- name: Check coverage threshold
uses: VeryGoodOpenSource/very_good_coverage@v2
uses: actions/github-script@v9
with:
path: ./contracts/zero-ex/lcov.info
min_coverage: 6.98
exclude: '**/tests'
script: |
const fs = require('fs');
const MIN_COVERAGE = 6.98;
const lcov = fs.readFileSync('./contracts/zero-ex/lcov.info', 'utf8');
let linesFound = 0;
let linesHit = 0;
for (const line of lcov.split('\n')) {
if (line.startsWith('LF:')) linesFound += Number(line.slice(3));
else if (line.startsWith('LH:')) linesHit += Number(line.slice(3));
}
const coverage = linesFound ? (100 * linesHit) / linesFound : 0;
core.info(`Line coverage: ${coverage.toFixed(2)}% (min ${MIN_COVERAGE}%)`);
if (coverage < MIN_COVERAGE) {
core.setFailed(`Coverage ${coverage.toFixed(2)}% is below the ${MIN_COVERAGE}% threshold`);
}
Comment on lines +137 to +151

@tobernguyen tobernguyen Aug 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are we missing the exclude: '**/tests'?

@talalashraf talalashraf Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The exclude was dropped intentionally because it was a no-op: very_good_coverage's exclude: '**/tests' uses minimatch, and **/tests matches a path that is tests — not files under tests/. Forge emits relative tests/... paths, so the action never actually excluded anything; coverage was always computed over all files, and the 6.98% floor is calibrated to that. So omitting it preserves the exact number.

@talalashraf talalashraf Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Let's keep this as is and then see if anything changes ?


- name: Run Forge build on governance contracts
working-directory: ./contracts/governance
Expand Down
Loading