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

x/tools/gopls: integration testing may not test cases with old go versions #69630

Closed
hyangah opened this issue Sep 26, 2024 · 3 comments
Closed
Assignees
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@hyangah
Copy link
Contributor

hyangah commented Sep 26, 2024

The go directive in gopls's go.mod is set to 1.23.1 per #65917.
The assumption is that gopls built with go1.23.1+ can be still used to
analyze codebase with slightly older go versions. (e.g. go1.22.x, go1.21.x)

Gopls integration tests run on workspaces created in temp directories
outside golang.org/x/tools/gopls module and their go.mod files have go
version old enough to prevent toolchain switch. With this setup, I hoped
running gopls tests on go1.X builders (X < 23) is sufficient to test our assumption.

But it looks like this is not true.

CL 404134 prepends GOROOT/bin to PATH in go test, as
mentioned in #68005.

In CL 616055 I attempted to verify the go version.

$ GOTOOLCHAIN=local go version
go version go1.22.6 darwin/amd64

$ go version
go version go1.23.1 darwin/amd64

$ go test ./internal/test/integration/workspace -run TestCheckGoVersion
--- FAIL: TestCheckGoVersion (0.02s)
    workspace_test.go:1287: /Users/hakim/sdk/go1.23.1/bin/go <nil>
    workspace_test.go:1288: go version in PATH = 23
...

$ go test ./internal/test/integration/workspace -run TestViewGoVersion
...
--- FAIL: TestViewGoVersion (0.28s)
    --- FAIL: TestViewGoVersion/default (0.28s)
        workspace_test.go:1320: SummarizeViews() mismatch (-want +got):
              []command.View{
              	{
              		... // 1 ignored and 2 identical fields
              		Folder:     "file:///var/folders/5p/zn7ykc111kn3lm09h_47mz2w001py5/T/gopls-te"...,
              		EnvOverlay: nil,
            - 		GoVersion:  "",
            + 		GoVersion:  "go1.23.1",
              	},
              }
              ...

I don't know how we can recover the original PATH inside tests reliably.

go test also sets GOROOT environment variable, too, but we can unset it.

@golang/tools-team

@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Sep 26, 2024
@gopherbot gopherbot added this to the Unreleased milestone Sep 26, 2024
@findleyr
Copy link
Member

Thanks for investigating! This definitely needs to be fixed (see also #69321).

@findleyr findleyr modified the milestones: Unreleased, gopls/v0.17.0 Sep 26, 2024
@findleyr findleyr self-assigned this Oct 30, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/623175 mentions this issue: gopls/internal/test: fix path to local go in integration tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

4 participants