Skip to content

meta: Add code coverage metrics for node/ and sdk/#4654

Open
johnsaigle wants to merge 12 commits intowormhole-foundation:mainfrom
johnsaigle:code-coverage-ci
Open

meta: Add code coverage metrics for node/ and sdk/#4654
johnsaigle wants to merge 12 commits intowormhole-foundation:mainfrom
johnsaigle:code-coverage-ci

Conversation

@johnsaigle
Copy link
Contributor

@johnsaigle johnsaigle commented Jan 30, 2026

This PR introduces a method to measure unit test coverage in the Guardian, specificially in the node/ and sdk/ 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-tests job fails as a result.

Development Story

Case 1: Insufficient Tests

  • Make changes
  • If tests are insufficient, the build fails
  • Add tests and re-submit, the build should pass

Case 2: Test coverage remains the same

  • Make changes
  • The build succeeds because the current tests coverage is the same as the file in version control

Case 3: Test coverage increases

  • Make changes, generate new code coverage report, check in to CI
  • CI passes because its test coverage matches

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

  • Getting more robust unit test coverage will help prevent regressions to security-sensitive behaviour during refactoring efforts

Why not use tool X instead of writing something new?

I took this approach because:

  • Our use case is very simple
  • The maintenance burden of this code should be small
  • It's done in a modular-enough way that it should be easy to scrap later if we want.
  • his also has the benefit of giving us more control over our testing standards for the repo
  • This solution also avoids the overhead involved with subscribing to a new service, connecting APIs and so on
  • No added dependencies as we figure out whether coverage metrics are really useful.

Alternatives Considered

  • Codecov seems to require an account and is a larger SaaS service that may be more than what we need
  • https://github.com/gwatts/go-coverage-action also does low-dependency code coverage but its workflow requires that the GH Action to have write-access to this repo.

djb15
djb15 previously approved these changes Feb 6, 2026
Copy link
Collaborator

@djb15 djb15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

@johnsaigle
Copy link
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

@johnsaigle johnsaigle force-pushed the code-coverage-ci branch 2 times, most recently from fad687d to 27d6e17 Compare February 13, 2026 20:57
@johnsaigle johnsaigle marked this pull request as ready for review February 18, 2026 14:40
Copy link
Contributor

@pleasew8t pleasew8t left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran through the different make targets, and they seem fine.

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.

3 participants