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/go/analysis/analysistest: analysistest.Run failing with modules #46041

Open
pawelmitka opened this issue May 7, 2021 · 12 comments
Open
Labels
modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@pawelmitka
Copy link

What version of Go are you using (go version)?

go version go1.15.7 darwin/amd64
analysistest v0.1.0

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env

What did you do?

I have built a clean architecture go/analysis linter which works pretty well. However, when I was trying to run analysistest on complex test cases with lots of imports it was failing.

analysistest.Run(t, analysistest.TestData(), cleanarch.Analyzer, c.Pattern)

What did you expect to see?

analysistest working does not fail with modules used in test data files

What did you see instead?

valid-simple/pkg/questions/adapter/http_adapter.go:4:2: cannot find package "github.com/$ORGNAME/$REPO/cleanarch/testdata/valid-simple/pkg/questions" in any of:
/usr/local/Cellar/go/1.15.7_1/libexec/src/github.com/$ORGNAME/$REPO/cleanarch/testdata/valid-simple/pkg/questions (from $GOROOT)
/Users/$USER/IdeaProjects/$PRJ/cleanarch/testdata/src/github.com/$ORGNAME/$REPO/cleanarch/testdata/valid-simple/pkg/questions (from $GOPATH)

I determine the root cause is: "GO111MODULE=off", "GOPROXY=off" introduced in change: golang/tools@211dcd1

What puzzled me is that analysis itself worked perfectly, however, I determined modules are not disabled in the main path: https://github.com/golang/tools/blob/fe37c9e135b934191089b245ac29325091462508/go/analysis/internal/checker/checker.go#L152

When I reverted change locally it worked like a charm for me. Let me know if I can just make a PR to resolve this issue once and for all.

@timothy-king
Copy link
Contributor

golang/tools@211dcd1 was part of a series of changes to address #27858

/cc @bcmills who was involved in #27858.

@timothy-king timothy-king changed the title analysistest does not work with modules go/analysis/analysistest: analysistest.Run failing with modules May 7, 2021
@bcmills
Copy link
Contributor

bcmills commented May 7, 2021

Let me know if I can just make a PR to resolve this issue once and for all.

If you want to send a PR, if it's simple enough we can at least run it through the TryBots and see what sticks!

(CC @matloob @jayconrod @ianthehat)

@bcmills bcmills added modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels May 7, 2021
@bcmills bcmills added this to the Unreleased milestone May 7, 2021
@bcmills bcmills changed the title go/analysis/analysistest: analysistest.Run failing with modules x/tools/go/analysis/analysistest: analysistest.Run failing with modules May 7, 2021
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label May 7, 2021
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/318709 mentions this issue: fix: using modules in analysistest

@pawelmitka
Copy link
Author

Let me know if I can just make a PR to resolve this issue once and for all.

If you want to send a PR, if it's simple enough we can at least run it through the TryBots and see what sticks!

(CC @matloob @jayconrod @ianthehat)

Looks like all bots are happy! 😄

@pawelmitka
Copy link
Author

@bcmills Could you answer a question in https://golang.org/cl/318709? I would love to complete and close this change.

@halfcrazy
Copy link

Any updates?

@bcmills
Copy link
Contributor

bcmills commented Jan 4, 2022

Last TryBot run for CL 318709 was not passing — as far as I am aware it's waiting on @pawelmitka to respond, but please let me know if I'm mistaken.

ldez added a commit to ldez/tools that referenced this issue Jun 26, 2023
Modules were explicitly disabled in analysistest. This lead to failures in tests where testdata required modules.

This change introduces new struct Options that contains configuration for a new func `RunWithOptions`.
`RunWithOptions` is called internally by `Run` and `RunWithSuggestedFixes`  with modules disabled for backward compatibility.
`RunWithOptions` can be run explicitly too, with a configuration allowing usage of modules.

Fixes golang/go#46041
Fixes golang/go#37054
ldez added a commit to ldez/tools that referenced this issue Jun 26, 2023
Modules were explicitly disabled in analysistest. This lead to failures in tests where testdata required modules.

This change introduces new struct Options that contains configuration for a new func `RunWithOptions`.
`RunWithOptions` is called internally by `Run` and `RunWithSuggestedFixes`  with modules disabled for backward compatibility.
`RunWithOptions` can be run explicitly too, with a configuration allowing usage of modules.

It also work with Go workspace.

Fixes golang/go#46041
Fixes golang/go#37054
ldez added a commit to ldez/tools that referenced this issue Jun 26, 2023
Modules were explicitly disabled in analysistest. This lead to failures in tests where testdata required modules.

This change introduces new struct Options that contains configuration for a new func `RunWithOptions`.
`RunWithOptions` is called internally by `Run` and `RunWithSuggestedFixes`  with modules disabled for backward compatibility.
`RunWithOptions` can be run explicitly too, with a configuration allowing usage of modules.

It also work with Go workspace.

Fixes golang/go#46041
Fixes golang/go#37054

Change-Id: I91a9365b46ea44afae79e78333720ea8c46489d7
@ldez
Copy link

ldez commented Jun 26, 2023

I created a new patch because the CL 318709 was incomplete and doesn't change for 2 years.

https://go-review.googlesource.com/c/tools/+/506275

I pushed all the parameters to the options, I'm not sure about that, I need feedback.
I managed Run and RunWithSuggestedFixes and created a demo repository with a CI that runs Linux, MacOS, and Windows.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/506275 mentions this issue: go/analysis/analysistest: using modules

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/506355 mentions this issue: go/analysis/analysistest: sketch of packages.Load(testdata/go.mod)

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/506356 mentions this issue: go/analysis/analysistest: sketch of a way to support modules

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/577995 mentions this issue: go/analysis/analysistest: add a TestDir helper for modules

gopherbot pushed a commit to golang/tools that referenced this issue Apr 17, 2024
Adds a new package testfiles to help write tests around go files
and directories. These are especially helpful when dealing with
go.mod files.

Adds a new CopyTestFiles function that copies a directory and
removes the ".test" file extension, e.g. "go.mod.test" becomes
"go.mod". This allows for easily creating analysistests
with go.mod files. This change also adds a new TestDir helper
to extract to a temporary directory.

Consolidates txtar usage around ExtractTxtar, which writes a
txtar archive to a directory, and adds a convenience helper
TestTxtar that extracts a txtar at a path to a temporary
directory.

Updates golang/go#61336
Updates golang/go#53063
Updates golang/go#46041
Updates golang/go#37054

Change-Id: I09210f751bbc6ac3f01c34fba22b7e8fa1ddf93f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/577995
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

6 participants