Skip to content

fix: register go-test-coverage tool in tools/go.mod#14200

Open
Valyrian-Code wants to merge 5 commits into
kgateway-dev:mainfrom
Valyrian-Code:fix/register-go-test-coverage-tool
Open

fix: register go-test-coverage tool in tools/go.mod#14200
Valyrian-Code wants to merge 5 commits into
kgateway-dev:mainfrom
Valyrian-Code:fix/register-go-test-coverage-tool

Conversation

@Valyrian-Code

Copy link
Copy Markdown
Contributor

Description

The GO_TEST_COVERAGE Makefile variable referenced the coverage tool via
go tool github.com/vladopajic/go-test-coverage/v2, but the tool was not
registered in any module's tool directive, so make go-test-with-coverage
and make validate-test-coverage failed to resolve it.

This registers github.com/vladopajic/go-test-coverage/v2 in tools/go.mod
(alongside the other tooling dependencies) and updates the Makefile invocation
to use -modfile tools/go.mod, matching the existing yamlfmt and goreleaser
patterns.

Verified locally: go tool -modfile tools/go.mod github.com/vladopajic/go-test-coverage/v2 --help
now resolves and runs the tool.

Fixes #12575

Change Type

/kind fix

Changelog

NONE

The GO_TEST_COVERAGE Makefile variable invoked 'go tool github.com/vladopajic/go-test-coverage/v2', but the tool was not registered in any module's tool directive, so the go-test-with-coverage and validate-test-coverage targets failed to resolve it.

Register the tool in tools/go.mod alongside the other tooling dependencies and point the Makefile invocation at it with -modfile tools/go.mod, matching the existing yamlfmt and goreleaser patterns.

Fixes kgateway-dev#12575

Signed-off-by: RAJVEER42 <irajveer.bishnoi2310@gmail.com>
Copilot AI review requested due to automatic review settings June 8, 2026 15:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR updates the repository’s Go tooling setup to run go-test-coverage via the dedicated tools/go.mod module.

Changes:

  • Add github.com/vladopajic/go-test-coverage/v2 to the tools module toolchain.
  • Update tools/go.mod / tools/go.sum with transitive dependencies required by the new tool.
  • Adjust the Makefile to invoke go-test-coverage using -modfile tools/go.mod.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.

File Description
tools/go.mod Registers go-test-coverage as a tool and records new indirect dependencies in the tools module.
tools/go.sum Captures module checksums for the newly introduced tool/transitive dependencies.
Makefile Runs go-test-coverage using the tools module via -modfile tools/go.mod.

Comment thread Makefile
Comment thread tools/go.mod
Comment thread tools/go.mod
@gateway-bot gateway-bot added kind/fix Categorizes issue or PR as related to a bug. release-note-none labels Jun 8, 2026
@Valyrian-Code

Copy link
Copy Markdown
Contributor Author

Thanks for the review. I looked into all three points:

1. -modfile and workspace mode — This repo doesn't use Go workspace mode (there's no go.work file and GOWORK is unset), so -modfile is safe here. It also matches the existing convention in this same Makefile: YAMLFMT ?= go tool -modfile tools/go.mod yamlfmt and the release target (goreleaser) both use the identical pattern. If a workspace-safe form is preferred regardless, the setup-envtest line uses go -C tools tool … — happy to switch to that for consistency if you'd rather.

2. aws/aws-sdk-go (v1) — This is a required transitive dependency of the coverage tool itself, not something avoidable on our side. go mod why traces it to the tool's badge uploader (which backs the --cdn-* flags):

github.com/vladopajic/go-test-coverage/v2/pkg/testcoverage/badgestorer
github.com/aws/aws-sdk-go/aws

go mod tidy won't drop it since it's genuinely in the tool's import graph — removing it would require an upstream change in go-test-coverage.

3. Two go-github majors (v82 + v84)v84 already existed in tools/go.mod on main (pulled in by goreleaser) before this PR; this change only adds v82, which the same badgestorer package imports. They're distinct module paths and coexist fine; consolidating would likewise need upstream alignment, not something this PR can do.

Net: the only first-party change here is the one-line Makefile invocation plus registering the tool — the additional go.mod/go.sum entries are the tool's own required transitive dependencies. Let me know if you'd prefer the go -C tools tool form.

@Valyrian-Code

Copy link
Copy Markdown
Contributor Author

Gentle ping on this one. It's a small fix that's been open a few weeks, would appreciate a review whenever a maintainer has time. Thanks!

@davidjumani

Copy link
Copy Markdown
Contributor

@Valyrian-Code can you please fix the failing checks. This can be done by running make verify and updating the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/fix Categorizes issue or PR as related to a bug. release-note-none

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unregistered tool reference in the GO_TEST_COVERAGE Makefile variable

4 participants