Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/go: GO111MODULE does not invalidate test cache #69203

Closed
rittneje opened this issue Sep 1, 2024 · 5 comments
Closed

cmd/go: GO111MODULE does not invalidate test cache #69203

rittneje opened this issue Sep 1, 2024 · 5 comments
Assignees
Labels
GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@rittneje
Copy link

rittneje commented Sep 1, 2024

Go version

go version go1.22.6 darwin/amd64

Output of go env in your module/workspace:

GO111MODULE='auto'
GOARCH='amd64'
GOBIN=''
GOCACHE='/tmp/.gocache'
GOENV='/Users/rittneje/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/rittneje/go/pkg/mod'
GONOPROXY='[REDACTED]'
GONOSUMDB='[REDACTED]'
GOOS='darwin'
GOPATH='/Users/rittneje/go'
GOPRIVATE='[REDACTED]'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/Users/rittneje/go1.22.6'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/Users/rittneje/go1.22.6/pkg/tool/darwin_amd64'
GOVCS='[REDACTED]'
GOVERSION='go1.22.6'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD=''
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/kf/kr7_s3xx0l12zbj3jrn082hmzy5gvy/T/go-build1131392119=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

$ GO111MODULE=auto go test net/http -v -run=^TestReadResponseErrors$
=== RUN   TestReadResponseErrors
    response_test.go:944: status "200 OK" "Content-Length:\r\nContent-Length:\r\n\r\nGophers\r\n": unexpected success; want error with substring "invalid empty Content-Length"
--- FAIL: TestReadResponseErrors (0.00s)
FAIL
FAIL	net/http	0.282s
FAIL

$ GO111MODULE=on go test net/http -v -run=^TestReadResponseErrors$
=== RUN   TestReadResponseErrors
--- PASS: TestReadResponseErrors (0.00s)
PASS
ok  	net/http	0.278s

$ GO111MODULE=auto go test net/http -v -run=^TestReadResponseErrors$
=== RUN   TestReadResponseErrors
--- PASS: TestReadResponseErrors (0.00s)
PASS
ok  	net/http	(cached)

$ go clean -testcache

$ GO111MODULE=auto go test net/http -v -run=^TestReadResponseErrors$
=== RUN   TestReadResponseErrors
    response_test.go:944: status "200 OK" "Content-Length:\r\nContent-Length:\r\n\r\nGophers\r\n": unexpected success; want error with substring "invalid empty Content-Length"
--- FAIL: TestReadResponseErrors (0.00s)
FAIL
FAIL	net/http	0.366s
FAIL

What did you see happen?

As seen above, changing GO111MODULE did not invalidate the test cache, even though it factors into the compiler's behavior around the default GODEBUG selection and thus by extension the behavior of the code being tested.

What did you expect to see?

Changing GO111MODULE should invalidate the test cache, given that it changes the default GODEBUG. (Or at least, the default GODEBUG itself ought to factor into the cache.)

@dmitshur dmitshur added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go labels Sep 4, 2024
@dmitshur dmitshur added this to the Backlog milestone Sep 4, 2024
@dmitshur
Copy link
Contributor

dmitshur commented Sep 4, 2024

CC @matloob, @samthanawalla.

@matloob
Copy link
Contributor

matloob commented Sep 4, 2024

I wonder why we don't seem to be adding the default GODEBUG to the action id for the link step? I think we could just add it to printLinkerConfig here: https://cs.opensource.google/go/go/+/master:src/cmd/go/internal/work/exec.go;drc=6b2ffc72b67713de4f08915937a64392aa4dbff0;l=347

@matloob matloob self-assigned this Sep 10, 2024
@matloob
Copy link
Contributor

matloob commented Sep 11, 2024

So we don't currently add it because it's usually a part of the buildinfo, which is used as part of the action id: https://cs.opensource.google/go/go/+/master:src/cmd/go/internal/work/exec.go;l=327;drc=6b2ffc72b67713de4f08915937a64392aa4dbff0 . But the buildinfo isn't generated for tests unless the test is a test of package main.

I think we should probably still explicitly add default GODEBUG to the link action id. One alternative would be to add build info for all tests, but we'd be doing unnecessary work to get the build info.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/610875 mentions this issue: cmd/go: explicitly add default GODEBUG to linker config

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Sep 12, 2024
@dmitshur dmitshur modified the milestones: Backlog, Go1.24 Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants