Skip to content

fix: replace node20 very_good_coverage with lcov threshold check - #809

Open
talalashraf wants to merge 3 commits into
developmentfrom
fix/pla-1666-very-good-coverage
Open

fix: replace node20 very_good_coverage with lcov threshold check#809
talalashraf wants to merge 3 commits into
developmentfrom
fix/pla-1666-very-good-coverage

Conversation

@talalashraf

Copy link
Copy Markdown
Contributor

GitHub removes Node ≤20 action runtimes from runners on 2026-09-16 — changelog. VeryGoodOpenSource/very_good_coverage is archived (node20); the coverage floor is enforced inline instead:

  • VeryGoodOpenSource/very_good_coverage@v2 (node20, archived) → an awk step that reads the same lcov.info, excludes test dirs, and fails below the 6.98% line-coverage floor

References

@talalashraf
talalashraf requested a review from dekz as a code owner July 7, 2026 14:16
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

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.

@github-actions github-actions Bot added the stale label Aug 7, 2026
@github-actions github-actions Bot closed this Aug 24, 2026
@talalashraf talalashraf reopened this Aug 24, 2026
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.
@github-actions github-actions Bot removed the stale label Aug 25, 2026
Comment thread .github/workflows/ci.yml
Comment on lines +137 to +151
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`);
}

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'?

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.

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 ?

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