meta: Add code coverage metrics for node/ and sdk/#4654
Open
johnsaigle wants to merge 12 commits intowormhole-foundation:mainfrom
Open
meta: Add code coverage metrics for node/ and sdk/#4654johnsaigle wants to merge 12 commits intowormhole-foundation:mainfrom
johnsaigle wants to merge 12 commits intowormhole-foundation:mainfrom
Conversation
e128227 to
a7330f7
Compare
djb15
previously approved these changes
Feb 6, 2026
Collaborator
djb15
left a comment
There was a problem hiding this comment.
Approving as these changes are not intrusive (from a security point of view) and at a high level the approach is what we discussed. I'd be keen to get this into CI and see what it's like to use, we can always respond to feedback.
Awesome work getting this written so quickly!
Contributor
Author
|
Thanks! I actually asked around and found this project: https://github.com/gwatts/go-coverage-action I think it could potentially be simpler. I'm going to try it out and based on the results I'll update this PR |
fad687d to
27d6e17
Compare
pleasew8t
approved these changes
Feb 24, 2026
Contributor
pleasew8t
left a comment
There was a problem hiding this comment.
I ran through the different make targets, and they seem fine.
040f41f to
fcf18a4
Compare
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.
This PR introduces a method to measure unit test coverage in the Guardian, specificially in the
node/andsdk/directories.Overview
The PR uses Go's native code coverage metric tools to generate a percentage-based coverage of all of the files. After the tests finish running, a simple Go program evaluates the result against the previous run of the tool (which is checked in to the repo). If the numbers decrease, the tool exits and shows the offending file.
The idea is to integrate this new script into our existing unit test job in CI. If a new PR removes tests or adds new, untested code, CI will cause it to fail.
Demonstration
#4653 is a dummy PR that deliberately reduces test coverage. Its
node-testsjob fails as a result.Development Story
Case 1: Insufficient Tests
Case 2: Test coverage remains the same
Case 3: Test coverage increases
If a developer forgets to generate and check-in the new coverage report, the build still fails. The idea here is to cause any increase in coverage to ratchet forward and prevent any regressions once we hit a new target. (Otherwise, we may go from a baseline of 50% to 55% and then back down to 50% if the new coverage report does not get checked-in. It's hard to remember to do this automatically, so CI can check for us. This is somewhat intrusive but we also fail builds on spelling errors and new linting rules, so this approach is consistent with the rest of CI and existing dev expectations.)
There is a slight tolerance for some drift in coverage as certain factors may cause tiny divergences in tests (e.g. Concurrency based tests via
race; differences in the runner vs. a dev's machine).Motivation
Why not use
tool Xinstead of writing something new?I took this approach because:
Alternatives Considered