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: don't add dependencies of external tests #32380

Closed
JAicewizard opened this issue Jun 1, 2019 · 13 comments
Closed

cmd/go: don't add dependencies of external tests #32380

JAicewizard opened this issue Jun 1, 2019 · 13 comments
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.

Comments

@JAicewizard
Copy link

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

go version go1.12.5 linux/amd64

Does this issue reproduce with the latest release?

presumably

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

go env Output
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/jaap/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/jaap/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/lib/go"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/jaap/go-projects/templatebuilder/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build972523621=/tmp/go-build -gno-record-gcc-switches"

What did you do?

go mod tidy

What did you expect to see?

a go.mod that doesnt include dependencies of tests in external packages

What did you see instead?

github.com/Netflix/go-expect v0.0.0-20180928190340-9d1f4485533b // indirect
got added because it is a dependency of some tests in gopkg.in/AlecAivazis/survey.v1

If a test is part of the current module(or submodule thereof) it should be included in the dependencies, but external tests will not be run on building on any platform with any build tag.

@agnivade
Copy link
Contributor

agnivade commented Jun 1, 2019

Please see this comment from @rsc

The module download is split into two parts: downloading the go.mod and downloading the actual code. If you have dependencies only needed for tests, then they will show up in your go.mod, and go get will download their go.mods, but it will not download their code. The test-only dependencies get downloaded only when you need it, such as the first time you run go test.

So the test dependency will show up in go.mod. But it doesn't get downloaded until you run go test.

@mvdan
Copy link
Member

mvdan commented Jun 2, 2019

There's also other existing issues, like #26955. This should probably be closed as a duplicate.

@JAicewizard
Copy link
Author

I get that the tests are included in go.mod
But this dependency is included in the tests of another package and wont be run during go test in the current package.

@bcmills
Copy link
Contributor

bcmills commented Jun 3, 2019

this dependency is included in the tests of another package and wont be run during go test in the current package.

go mod tidy is meant to follow the same package import graph as the all pattern in module mode.

If you run go test all, does the resulting build end up running tests within github.com/Netflix/go-expect? If so, this is working as designed. (If not, please provide more detail on the nature of the dependency. If it does not contain sensitive paths, please include the output of go mod why -m github.com/Netflix/go-expect.)

@bcmills bcmills changed the title dont add dependencies of external tests cmd/go: dont add dependencies of external tests Jun 3, 2019
@bcmills bcmills added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jun 3, 2019
@JAicewizard
Copy link
Author

JAicewizard commented Jun 3, 2019

after cleaning up all packages form /src and /pkg/mod I cant run go test at all unless I manually go get a certain package.
I think that is because gitlab.com/golang-commonmark/markdown imports 2 diferent versions of blackfriday for benchmark purposes.
This is the output
A quick ctr-f cant find github.com/Netflix/go-expect or gopkg.in/AlecAivazis/survey.v1(with/without .test)

output of go mod why -m github.com/Netflix/go-expect:

go: downloading gopkg.in/russross/blackfriday.v2 v2.0.1 //I think this is because of the markdown lib, but its not important AFAIK
# github.com/Netflix/go-expect
gitlab.com/antipy/antibuild/cli/cmd
gopkg.in/AlecAivazis/survey.v1
gopkg.in/AlecAivazis/survey.v1.test
github.com/Netflix/go-expect

@bcmills
Copy link
Contributor

bcmills commented Jun 5, 2019

A quick ctr-f cant find github.com/Netflix/go-expect or gopkg.in/AlecAivazis/survey.v1(with/without .test)

GitHub search turned it up pretty readily:
https://github.com/AlecAivazis/survey/blob/3db8b5dd7b2875d268abf7c1feb59bd929f6b838/input_test.go#L10

Edit: never mind, I think I misinterpreted.

@bcmills
Copy link
Contributor

bcmills commented Jun 5, 2019

Depending on which version of gitlab.com/antipy/antibuild you're using, the dependency on gopkg.in/AlecAivazis/survey.v1 is here:
https://gitlab.com/antipy/antibuild/cli/blob/modules/cmd/new.go#L18

@JAicewizard
Copy link
Author

I know where it is imported, but I cant find that test in go test all
I also already know where the go-expect is coming from (see go why output)
Im working on the modules branch FYI.
But the question was if it is used in a test run by go test all and I cant find it in there

@bcmills
Copy link
Contributor

bcmills commented Jun 5, 2019

Ahhh! I get it now.

What was the exact command that you ran to produce that output, and in what working directory? I don't see any paths from gitlab.com/antipy/antibuild in that output at all.

@JAicewizard
Copy link
Author

Oh I dint actually notice, I ran it from the GOPATH but gitlab.com/antipy/antibuild inst in there, thats correct.
I cant run it from inside gitlab.com/antipy/antibuild because I get this:

go: downloading gopkg.in/russross/blackfriday.v2 v2.0.1
go: downloading gopkg.in/russross/blackfriday.v2 v2.0.1
can't load package: package gopkg.in/russross/blackfriday.v2: unknown import path "gopkg.in/russross/blackfriday.v2": cannot find module providing package gopkg.in/russross/blackfriday.v2

This is because gitlab.com/golang-commonmark/markdown imports 2 diferent versions of blackfriday for benchmark purposes (and only one is put in go.mod). one uses gopkg.in and the other the github version. I am not 100% sure this is the cause but I could open an issue about this?

After removing this dependency I can run go test all and I do indeed see that the tests using go-expect are there. Although its not how I would have written it, it follows what its supposed to do so I guess that is correct. Thanks for helping me!

@bcmills
Copy link
Contributor

bcmills commented Jun 5, 2019

The issue with gopkg.in/russross/blackfriday.v2 is probably an instance of #31428.

v2.0.1 includes a go.mod file declaring its module path to be github.com/russross/blackfriday/v2, so you should be getting some sort of module fetch error for that path.

(On the other hand, v2.0.0 should work: it lacks a go.mod file and thus does not enforce import paths.)

@bcmills
Copy link
Contributor

bcmills commented Jun 5, 2019

Sounds like the open questions are all resolved though; closing. (Please do file an issue if you run into any more trouble.)

@bcmills bcmills closed this as completed Jun 5, 2019
@bcmills bcmills changed the title cmd/go: dont add dependencies of external tests cmd/go: don't add dependencies of external tests Feb 4, 2020
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/220080 mentions this issue: design/36460: add design for lazy module loading

gopherbot pushed a commit to golang/proposal that referenced this issue Mar 5, 2020
Updates golang/go#36460
Updates golang/go#27900
Updates golang/go#26955
Updates golang/go#30831
Updates golang/go#32058
Updates golang/go#32380
Updates golang/go#32419
Updates golang/go#33370
Updates golang/go#33669
Updates golang/go#36369

Change-Id: I1d4644e3e8b4e688c2fc5a569312495e5072b7d7
Reviewed-on: https://go-review.googlesource.com/c/proposal/+/220080
Reviewed-by: Russ Cox <rsc@golang.org>
@golang golang locked and limited conversation to collaborators Feb 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

5 participants