test(evm-module): skip RandomSampling fairness + gas benchmark under coverage (fix coverage CI on main)#1235
Merged
Conversation
…der coverage
The "Tornado: Solidity coverage (push safety net)" lane went red on main: the
`@unit RandomSampling > fairness: draw frequency within ±2pp` test (5000 draws via
previewChallengeForSeed) exceeds the 600s mocha timeout under coverage instrumentation
(each instrumented draw is far slower; CI took 34m vs ~12m locally, tipping it over).
Both this statistical test and the `@gas` benchmark are timing/gas-shaped and add no
unique contract-line coverage, so guard them to skip under coverage — matching the
existing convention in ContextGraphStorage.test.ts (`process.argv.some(a =>
a.includes('coverage'))`). Verified: both still RUN + pass under normal `hardhat test`;
both SKIP under `hardhat coverage` (gas suite shows 2 pending). Introduced by #1226,
which added these tests without the coverage guard.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
otReviewAgent
left a comment
There was a problem hiding this comment.
Review Agent completed this review and found no issues.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's broken
main's Tornado: Solidity coverage (push safety net) lane is red:npx hardhat coverageexits 1 with 1 failing test —All 781 tests pass locally under coverage (12m); on CI the run takes ~34m and the 5000-draw fairness test tips past the 600s mocha timeout under instrumentation.
Cause
Introduced by #1226 (the BIT sampling PR), which added two timing/gas-shaped tests without the coverage guard the repo already uses:
previewChallengeForSeeddraws), and@gasbenchmark suite.Under solidity-coverage every draw runs heavily instrumented, so the fairness test blows the timeout. Neither adds unique contract-line coverage (they re-exercise the same paths).
Fix
Guard both to skip under coverage, matching the existing convention in
ContextGraphStorage.test.ts:Verified
hardhat test: both still run + pass (gas 2✓, fairness 1✓).hardhat coverage: both skip (gas suite →2 pending, "Coverage skipped for: …"), so no timeout.🤖 Generated with Claude Code