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

gotype: type-checking external test files that rely on internal test files fails #22030

Open
jellevandenhooff opened this issue Sep 25, 2017 · 3 comments
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@jellevandenhooff
Copy link
Contributor

I tried to use go/types to type check a package with internal test files (with package foo) and external test files (with package foo_test). The internal test files expose some symbols that the external test files consume. This works fine in go test. However, the go/types code does not recognize the symbols exported by the internal test files.

I want to use the go/types package to write a refactoring tool that consumes external test files that rely on symbols exported by internal tests files, but I can't because of this error.

I think the issue is that the go/types import logic ignores the internal _test.go files when type-checking the external _test.go files. The go/types config https://golang.org/pkg/go/types/#Config does not seem to have a knob to import _test.go files.

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

go version go1.9 darwin/amd64

Does this issue reproduce with the latest release?

Yes

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

GOARCH="amd64"
GOOS="darwin"

What did you do?

I created a new package testgotypes with 3 files:

testgotypes.go, without this file gotype doesn't run:

package testgotypes

internal_test.go, which exports a variable for the external test:

package testgotypes

var Export = "hello"

external_test.go, which tries to access the variable exported by internal_test.go:

package testgotypes_test

import (
	"testing"

	"testgotypes"
)

func TestExport(t *testing.T) {
	if testgotypes.Export != "hello" {
		t.Error("uh oh")
	}
}

I ran tests, which worked just fine:

➜  testgotypes git:(master) go test -v  
=== RUN   TestExport
--- PASS: TestExport (0.00s)
PASS
ok  	testgotypes	0.006s

Then I tried to run gotype, compiled from /usr/local/Cellar/go/1.9/libexec/src/go/types/gotype.go:

➜  testgotypes git:(master) gotype -x -t .
external_test.go:10:5: Export not declared by package testgotypes

It fails with the same error my refactoring code sees.

To sanity check gotype, I then renamed internal_test.go to internal.go to make it part of the non-test build. Then gotype is happy:

➜  testgotypes git:(master) mv internal_test.go internal.go
➜  testgotypes git:(master) gotype -x -t .

What did you expect to see?

gotype successfully type checking external_test.go.

What did you see instead?

gotype fails to type check the external_test.go file.

@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 25, 2017
@ianlancetaylor ianlancetaylor added this to the Go1.10 milestone Sep 25, 2017
@griesemer griesemer changed the title go/types: type-checking external test files that rely on internal test files fails gotype: type-checking external test files that rely on internal test files fails Oct 2, 2017
@griesemer
Copy link
Contributor

This is not a go/types issue, it's a gotype issue: go/types doesn't care how the files are organized, it simply accepts a set of files.

The documentation inside gotype.go says for the -t flag:

-t
	include local test files in a directory (ignored if -x is provided)

so this is at least working as internally documented. But it should be documented externally, and furthermore, this behavior seems incorrect. It appears that go test does something different.

@griesemer griesemer 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 Oct 2, 2017
@griesemer
Copy link
Contributor

This is not a trivial fix. An external test package imports the local package consisting of the local package and local test files. The go/command builds that package in a temporary directory and then imports it from that temporary directory to make it all work.

gotype relies on go/types to type-check the external test package. go/types simply uses an importer to get the information about the package to be tested, and that importer doesn't know that it needs that package built with its local test files. Furthermore, the existing top-level API (go/importer) doesn't have a mechanism to communicate that information. Also, if the importer is not a "source" importer, but say a "gc" importer, the importer relies on installed packages, and they are never built with the local test files.

One option might be to implement a new custom importer, only known to gotype.go, which is based on go/internal/srcimporter. The srcimporter's mode argument (currently unused) could be used to signal that local test files need to be included. But that would not work for gotype -c=gc which doesn't rely on the source importer.

Still thinking.

@rsc
Copy link
Contributor

rsc commented Nov 22, 2017

Punting to Go 1.11 since gotype is an unreleased program.

@rsc rsc modified the milestones: Go1.10, Go1.11 Nov 22, 2017
@griesemer griesemer modified the milestones: Go1.11, Go1.12 Jun 27, 2018
@griesemer griesemer modified the milestones: Go1.12, Go1.13 Dec 11, 2018
@griesemer griesemer modified the milestones: Go1.13, Go1.14 May 6, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants