Skip to content

CI: submodule test worker runs 'go mod tidy', making the run non-hermetic #3869

Description

@akshat-kumar-singhal

Summary

The Submodule-Unit-Testing worker mutates each submodule's go.mod/go.sum before testing it (.github/workflows/go.yml:369-374):

# Download dependencies for the submodule
go mod download
go mod tidy

# Run tests with a focus on failed tests first
go test ./... -v -short -coverprofile="${module_name}.cov" -coverpkg=./...

Why this is a problem

It tests something other than what is committed. go mod tidy can add missing requirements and drop unused ones, so the dependency graph the tests run against is whatever tidy resolves at that moment, not what the repo declares. A submodule whose committed go.mod is missing a requirement passes CI and fails for anyone who clones it — CI has quietly repaired the exact defect it should be reporting.

It hides untidiness rather than reporting it. There is no git diff --exit-code after the tidy, so a go.mod that is out of date is silently corrected in the runner and the correction is thrown away with the runner.

It can reach the network mid-test-run and change resolution between legs. tidy may fetch modules download didn't, so the three matrix legs are not guaranteed to resolve identically.

Suggested fix

Drop the go mod tidy line. go mod download already populates the module cache, and go test fails loudly on a genuinely incomplete go.mod — which is the signal that is currently being suppressed.

If tidiness should be enforced (worth doing separately, and arguably belongs in code_quality rather than the test job):

go mod tidy -diff   # non-zero exit if tidy would change anything, no mutation

-diff has been available since Go 1.23 and the repo is on 1.26, so the older cp go.mod go.mod.bak && go mod tidy && diff dance isn't needed.

Expect fallout

If any submodule's committed go.mod is currently untidy, removing the tidy will surface it as a build failure rather than a silent repair. That is the point, but as with #3824 it means the fix and its fallout should land together rather than leaving the job red.

Context

Found while reviewing run 31266842627.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions