fix: replace node20 very_good_coverage with lcov threshold check - #809
Conversation
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Match very_good_coverage: empty/zero-line lcov now fails (was passing). Drop the no-op tests exclude (minimatch '**/tests' matched nothing) so the check is faithful and robust to the report's path prefix.
More readable node24 implementation; identical coverage math, 6.98% floor, and fail-closed on empty report.
| 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`); | ||
| } |
There was a problem hiding this comment.
Are we missing the exclude: '**/tests'?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Let's keep this as is and then see if anything changes ?
tobernguyen
left a comment
tobernguyen
left a comment
There was a problem hiding this comment.
LGTM. One note: the old very_good_coverage config had exclude: '**/tests' which the inline lcov check doesn't replicate, so the computed % may shift slightly — with a 6.98% threshold that's unlikely to matter. CI failure here ('Install dependencies') is pre-existing on every branch incl. development, unrelated to this change.
2 participants
GitHub removes Node ≤20 action runtimes from runners on 2026-09-16 — changelog.
VeryGoodOpenSource/very_good_coverageis archived (node20); the coverage floor is enforced inline instead:VeryGoodOpenSource/very_good_coverage@v2(node20, archived) → anawkstep that reads the samelcov.info, excludes test dirs, and fails below the 6.98% line-coverage floorReferences