Conversation
…b.com/sprintertech/sprinter-stash-contracts into feat/update_deterministic_hardhat_tests
| echo "==================================================" | ||
|
|
||
| # Parse CI-generated coverage using dedicated script | ||
| CI_LINES=$(npx ts-node --files scripts/get-coverage-percentage.ts) |
There was a problem hiding this comment.
Why is the get-coverage-percentage script not used anymore, and instead inline nodejs code is used?
| PR_LINES=$(jq -r .lines baseline-from-pr.json) | ||
| MAIN_LINES=$(jq -r .lines baseline-from-main.json) |
There was a problem hiding this comment.
Why abandoning this part if it was perfectly alright?
| # Fetch main branch | ||
| git fetch origin main | ||
| # Get baseline from main (for comparison - must not decrease) | ||
| git show origin/main:coverage-baseline.json > baseline-from-main.json 2>/dev/null || echo '{"lines":"0","functions":"0","branches":"0","statements":"0"}' > baseline-from-main.json |
There was a problem hiding this comment.
Why this code was abandoned?
| : (process.env.FORK_PROVIDER || process.env.BASE_RPC || "https://base-mainnet.public.blastapi.io"), | ||
| blockNumber: process.env.FORK_BLOCK_NUMBER ? parseInt(process.env.FORK_BLOCK_NUMBER) : undefined, | ||
| : (process.env.BASE_RPC || "https://base-mainnet.public.blastapi.io"), | ||
| blockNumber: undefined, |
There was a problem hiding this comment.
Why changing this logic? Is there a problem with it?
There was a problem hiding this comment.
It was not the case for the discrepancy, so forked block creates more complexity as we need to update it from time to time. I opt again for "latest"
There was a problem hiding this comment.
This is the diff with main branch, before the introduction of coverage test on a particular block.
FORK_PROVIDER and FORK_BLOCK_NUMBER logic is used for other purposes as well.
| lines: linesFound > 0 ? ((linesHit / linesFound) * 100).toFixed(2) : "0", | ||
| functions: functionsFound > 0 ? ((functionsHit / functionsFound) * 100).toFixed(2) : "0", | ||
| branches: branchesFound > 0 ? ((branchesHit / branchesFound) * 100).toFixed(2) : "0", | ||
| statements: linesFound > 0 ? ((linesHit / linesFound) * 100).toFixed(2) : "0", |
There was a problem hiding this comment.
All new parenthesis don't change the results, so why adding them?
| // Tolerant comparison | ||
| function checkDrop(metric: keyof CoverageData, baseline: CoverageData, label: string): string | null { | ||
| const base = parseFloat(baseline[metric]); | ||
| const curr = parseFloat(current[metric]); |
There was a problem hiding this comment.
current should be in function arguments instead of using a global one.
|
Added tolerance in the arbitrum native bridge branch. |
Added hardcoded Blocks for forked blockchain to keep it constant locally and on GitHub CI action