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: apply "main" and "./" import restrictions to test code #17475

Closed
nikhilm opened this issue Oct 17, 2016 · 3 comments
Closed

cmd/go: apply "main" and "./" import restrictions to test code #17475

nikhilm opened this issue Oct 17, 2016 · 3 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@nikhilm
Copy link

nikhilm commented Oct 17, 2016

Please answer these questions before submitting your issue. Thanks!

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

1.7.1

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/nikhil/iron/gocode"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/3t/tx0j5bws5kv7m6s2rj2dtyvm0000gn/T/go-build891883855=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"

What did you do?

This is a bug that was originally reported as onsi/ginkgo#305 but can also be reproduced by go test -i.

The reproducible test case is at https://github.com/nikhilm/ginkgo-broken. It features a package main program (in project/) that exports a function that is then tested via tests/project_test.go. While not idiomatic, this is legal go code.

To see the bug:

$ go get github.com/nikhilm/ginkgo-broken
$ cd $GOPATH/src/github.com/nikhilm/ginkgo-broken/tests

# This passes
$ go test -v .
=== RUN   TestYo
YOYOYO
--- PASS: TestYo (0.00s)
PASS
ok      github.com/nikhilm/ginkgo-broken/tests  0.006s

# This breaks the setup
$ go test -i .

$ go test -v .
# github.com/nikhilm/ginkgo-broken/tests_test
./project_test.go:6: can't find import: "github.com/nikhilm/ginkgo-broken/project"
FAIL    github.com/nikhilm/ginkgo-broken/tests [build failed]

# update original
$ touch ../project/main.go
$ go test -v  .
=== RUN   TestYo
YOYOYO
--- PASS: TestYo (0.00s)
PASS
ok      github.com/nikhilm/ginkgo-broken/tests  0.009s

What did you expect to see?

go test -v . should always pass.

What did you see instead?

go test -v . immediately after go test -i . fails.

@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 17, 2016
@quentinmit quentinmit added this to the Go1.8Maybe milestone Oct 17, 2016
@quentinmit quentinmit changed the title go test -i breaks go test until package being tested is update. cmd/go: go test -i corrupts imported "main" package Oct 17, 2016
@rsc
Copy link
Contributor

rsc commented Oct 20, 2016

Importing a package main is not supposed to work. I feel like we've looked at this before. Not sure why it is working as well as it does for you.

@rsc rsc modified the milestones: Go1.8, Go1.8Maybe Oct 20, 2016
@rsc rsc changed the title cmd/go: go test -i corrupts imported "main" package cmd/go: reject import of "main" package Oct 20, 2016
@rsc
Copy link
Contributor

rsc commented Oct 24, 2016

This was #4210.
The fix did not catch imports of "main" packages in tests.
We do want to allow importing main from its own test,
but not from other tests.

Similarly, the prohibition on local imports from non-local-imported packages
is meant to apply to tests but was dropped.

@rsc rsc changed the title cmd/go: reject import of "main" package cmd/go: apply "main" and "./" import restrictions to test code Oct 24, 2016
@gopherbot
Copy link

CL https://golang.org/cl/31821 mentions this issue.

@golang golang locked and limited conversation to collaborators Oct 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge 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