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: exclude standard library from "all" pattern in modules #26317

Closed
myitcv opened this issue Jul 10, 2018 · 26 comments
Closed

cmd/go: exclude standard library from "all" pattern in modules #26317

myitcv opened this issue Jul 10, 2018 · 26 comments
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@myitcv
Copy link
Member

myitcv commented Jul 10, 2018

Please answer these questions before submitting your issue. Thanks!

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

go version go1.10.3 linux/amd64
vgo commit: cdd82a7bc7c6d7c407792afe279ed74331335370

Does this issue reproduce with the latest release?

Yes.

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/ubuntu/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/ubuntu/gostuff"
GORACE=""
GOROOT="/home/ubuntu/usr/go1.10.3"
GOTMPDIR=""
GOTOOLDIR="/home/ubuntu/usr/go1.10.3/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
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-build858106816=/tmp/go-build -gno-record-gcc-switches"

What did you do?

cd `mktemp -d`
mkdir hello
cd hello
vgo mod -init -module github.com/you/hello
cat <<EOD > hello.go
package main

import (
        "fmt"
)

func main() {
        fmt.Println("Hello")
}
EOD
vgo test all

What did you expect to see?

I'm not totally clear what to expect because I couldn't see exactly where the special all argument is documented? But my guess is that it shouldn't include standard library packages.

What did you see instead?

?       github.com/you/hello    0.005s [no test files]
ok      fmt     0.083s
ok      errors  0.001s
ok      io      0.021s
ok      math    0.003s
ok      os      18.463s
ok      reflect 0.757s
ok      strconv 2.009s
...
@gopherbot gopherbot added this to the vgo milestone Jul 10, 2018
@myitcv
Copy link
Member Author

myitcv commented Jul 10, 2018

/cc @rsc @bcmills

@rsc rsc modified the milestones: vgo, Go1.11 Jul 12, 2018
@rsc rsc added the modules label Jul 12, 2018
@rsc rsc changed the title x/vgo: "vgo test all" includes standard library packages cmd/go: "go test all" includes standard library packages when using modules Jul 12, 2018
@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 16, 2018
@rsc
Copy link
Contributor

rsc commented Jul 17, 2018

I guess probably "all" should exclude the standard library (in all commands not just go test), on the theory that it's already tested, not subject to vendoring, and so on. It's a little weird to special case it but it really is special.

@rsc rsc changed the title cmd/go: "go test all" includes standard library packages when using modules cmd/go: exclude standard library from "all" pattern in modules Jul 17, 2018
@rsc rsc 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 Jul 17, 2018
@bcmills bcmills self-assigned this Jul 19, 2018
@bcmills
Copy link
Contributor

bcmills commented Jul 25, 2018

Should ... patterns also exclude the standard library in module mode? (modload.ImportPaths currently implements all in terms of ....)

@bcmills
Copy link
Contributor

bcmills commented Jul 25, 2018

Actually, there is currently a special postprocessing phase for all, so it seems that if we make a minimal change then patterns involving ... will continue to match the standard library.

Still, it seems like an interesting edge-case to consider.

@bcmills
Copy link
Contributor

bcmills commented Jul 25, 2018

I stand corrected again. Patterns involving ... already do not match the standard library in module mode.

@gopherbot
Copy link

Change https://golang.org/cl/125835 mentions this issue: cmd/go:/internal/modload: omit the standard library from "all"

@bcmills
Copy link
Contributor

bcmills commented Aug 8, 2018

Per discussion on https://golang.org/cl/125835, I think this is working as intended: "all" should include the subset of the standard library that is actually (transitively) imported by the current module.

To test all packages in the current module and its submodules, you can execute go test ./... from the module root, or go test $(go list -m)/... from anywhere in the module.

It's true that we don't have a good pattern for “exactly the packages in the current module”, but all is not that pattern.

@myitcv
Copy link
Member Author

myitcv commented Aug 8, 2018

Per discussion on https://golang.org/cl/125835, I think this is working as intended: "all" should include the subset of the standard library that is actually (transitively) imported by the current module.

Thanks. What I was after was a way of testing all packages in the current module and the transitive closure of package dependencies, excluding the standard library, i.e.

go test $(go list -f "{{if not .Standard}}{{.ImportPath}}{{end}}" $(go  list -f '{{ join .Deps  "\n"}}' $(go list -m)/...) $(go list -m)/...)

Edit: updated command to include $(go list -m)/... argument to go test

Reason being, it's probably safe to assume that the standard library packages I'm using (transitively) are already tested 😄. Build caching would obviate this concern to an extent, but I'd still have to run the tests for a standard library package once per release... but then that's already been done for me. Unless I'm missing something here about the benefit of (re)testing and hence including standard library packages in this definition?

To test all packages in the current module and its submodules, you can execute go test ./... from the module root, or go test $(go list -m)/... from anywhere in the module.

I don't think ./... includes submodules, does it? Since https://go-review.googlesource.com/c/vgo/+/122397, ./... has explicitly excluded submodules. For the reason that a subdirectory is not guaranteed to be a submodule.

However ./... being defined as such is somewhat unhelpful when it is the case that subdirectories are submodules (e.g. CI testing of a mono-repo). For now I've resorted to using find -name go.mod... because I realise this is probably something of an edge case, at least for Go 1.11.

It's true that we don't have a good pattern for “exactly the packages in the current module”, but all is not that pattern.

Given the above, I think m/... does actually cover this, because it doesn't recurse into submodules.

@bcmills
Copy link
Contributor

bcmills commented Aug 8, 2018

We do have kind of a weird inconsistency here, though. all applies to all modules and includes the standard library, but ... applies to all modules but excludes the standard library. I don't see any way to list patterns within the standard library in module mode, but maybe that's just not a thing we need to support.

@gopherbot
Copy link

Change https://golang.org/cl/128637 mentions this issue: cmd/go: test package patterns with multiple modules

@mwf
Copy link

mwf commented Aug 9, 2018

It's a pity we staying with the behaviour go test all testing std lib.

From https://research.swtch.com/vgo-tour:

Of course, after an upgrade, it's a good idea to test that everything still works. Our dependencies rsc.io/quote and rsc.io/sampler have not been tested with the newer text module. We can run their tests in the configuration we've created:

$ vgo test all

Guys, I believe we definitely need some short pattern to test the entire project with dependencies excluding std lib. Because as @myitcv mentioned, why should we ever test it if it's been already tested in Go release?

Patterns like go test $(go list -f "{{if not .Standard}}{{.ImportPath}}{{end}}" $(go list -f '{{ join .Deps "\n"}}' $(go list -m)/...)) look too clumsy, no one will be happy using such things in their project.

And the second reason for excluding std lib - some tests are very slow, I don't want to run them even once. Consider this small example (I still have go1.11beta2, sorry):

cd `mktemp -d`
go mod -init -module example.com/hello
cat <<EOD >main.go
package main

import (
        "fmt"
        "net/http"
        _ "net/http/pprof"
)

func main() {
        addr := "localhost:6060"
        fmt.Printf("Listening at: %q\n", addr)
        http.ListenAndServe(addr, nil)
}

EOD
go test all
?   	example.com/hello	[no test files]
ok  	fmt	0.142s
ok  	net/http	46.180s
ok  	net/http/pprof	2.200s
ok  	errors	0.033s
ok  	io	0.061s
ok  	math	0.042s
ok  	os	19.777s

46 sec? 20 sec? Guys, really? I see it and I just want to cancel :)

And signal.test runs almost forever eating all CPU :(

$ ps aux | grep signal
ikorolev         66777 185.8  0.0  4380320   1932 s009  R+    2:58PM  12:51.24 /var/folders/_b/d1934m9s587_8t_6ngv3hnc00000gp/T/go-build777311748/b810/signal.test -test.testlogfile=/var/folders/_b/d1934m9s587_8t_6ngv3hnc00000gp/T/go-build777311748/b810/testlog.txt

PS Just tried to run it on go1.11beta3, got an error:

$ go test all
build fmt_test: cannot find module for path fmt_test

Maybe it's connected with #26722 and fixed on tip. Will check

@mwf
Copy link

mwf commented Aug 9, 2018

Yeap, working on tip.

Actually I can't understand why the tests are so slow.
If the tests are run when building go from source they are much faster:

ok  	net	1.985s
ok  	net/http	4.355s
ok  	net/http/cgi	0.684s
ok  	net/http/cookiejar	0.028s
ok  	net/http/fcgi	0.023s
ok  	net/http/httptest	0.042s
ok  	net/http/httptrace	0.019s
ok  	net/http/httputil	0.061s
ok  	net/http/internal	0.015s
ok  	net/http/pprof	2.032s
$ time go test net/http
ok  	net/http	40.735s
go test net/http  4.84s user 2.45s system 17% cpu 41.623 total

Why are the same tests so much faster when building go vs go test net/http?

Anyway with so slow stdlib tests go test all seems to be useless :(

@mwf
Copy link

mwf commented Aug 9, 2018

This is the full timing of go test all for the same example project:

$ time go test all
...

*** Test killed: ran too long (10m0s).
FAIL	os/signal	605.012s
...
go test all  1521.37s user 69.95s system 228% cpu 11:35.33 total

Strange thing that testing os/signal fails on my machine. Though it passes when building go. And it passes in go 1.10.3
I'll file a separate issue for that.

Anyway, if we don't count the os/signal stuff, the longest tests taking >10s are:

ok  	net/http	44.172s
ok  	os	18.907s
ok  	net	30.170s
ok  	runtime	176.980s
ok  	runtime/pprof	42.158s
ok  	runtime/trace	10.776s
ok  	regexp	19.281s
ok  	compress/flate	11.576s
ok  	crypto/dsa	29.287s

Does it worth testing them at all?

@myitcv
Copy link
Member Author

myitcv commented Aug 9, 2018

@bcmills just to confirm my understanding (because this is the first time I'd seen the ... in isolation, i.e. with no prefix):

go list ./...   // all packages in the current directory and below,
                // excluding modules in subdirectories (which may be submodules)

go list m/...   // all packages in module m (by definition this does not
                // include submodules)

go list ...     // all packages in the current module and its module dependencies

go list all     // the transitive set of package imports, including std lib

go test all     // equivalent to go test $(go list all)

I don't see any way to list patterns within the standard library in module mode, but maybe that's just not a thing we need to support.

True, but we also don't have a way to easily list (and therefore test) the subset of go list all that are not standard library packages. Rather selfishly, this is what I need now; I'm relying on the standard library tests already having been run.

I don't actually find go list ... particularly useful. Let me demonstrate:

cd `mktemp -d`
export GOPATH=$(mktemp -d)
mkdir hello
cd hello
go mod init example.com/hello
cat <<EOD > main.go
package main

import _ "rsc.io/quote"

func main() {}
EOD
go list ...

gives:

Lots of output
example.com/hello
golang.org/x/crypto/acme
golang.org/x/crypto/acme/autocert
golang.org/x/crypto/acme/autocert/internal/acmetest
golang.org/x/crypto/argon2
golang.org/x/crypto/bcrypt
golang.org/x/crypto/blake2b
golang.org/x/crypto/blake2s
golang.org/x/crypto/blowfish
golang.org/x/crypto/bn256
golang.org/x/crypto/cast5
golang.org/x/crypto/chacha20poly1305
golang.org/x/crypto/cryptobyte
golang.org/x/crypto/cryptobyte/asn1
golang.org/x/crypto/curve25519
golang.org/x/crypto/ed25519
golang.org/x/crypto/ed25519/internal/edwards25519
golang.org/x/crypto/hkdf
golang.org/x/crypto/internal/chacha20
golang.org/x/crypto/internal/subtle
golang.org/x/crypto/md4
golang.org/x/crypto/nacl/auth
golang.org/x/crypto/nacl/box
golang.org/x/crypto/nacl/secretbox
golang.org/x/crypto/nacl/sign
golang.org/x/crypto/ocsp
golang.org/x/crypto/openpgp
golang.org/x/crypto/openpgp/armor
golang.org/x/crypto/openpgp/clearsign
golang.org/x/crypto/openpgp/elgamal
golang.org/x/crypto/openpgp/errors
golang.org/x/crypto/openpgp/packet
golang.org/x/crypto/openpgp/s2k
golang.org/x/crypto/otr
golang.org/x/crypto/pbkdf2
golang.org/x/crypto/pkcs12
golang.org/x/crypto/pkcs12/internal/rc2
golang.org/x/crypto/poly1305
golang.org/x/crypto/ripemd160
golang.org/x/crypto/salsa20
golang.org/x/crypto/salsa20/salsa
golang.org/x/crypto/scrypt
golang.org/x/crypto/sha3
golang.org/x/crypto/ssh
golang.org/x/crypto/ssh/agent
golang.org/x/crypto/ssh/knownhosts
golang.org/x/crypto/ssh/terminal
golang.org/x/crypto/ssh/test
golang.org/x/crypto/tea
golang.org/x/crypto/twofish
golang.org/x/crypto/xtea
golang.org/x/crypto/xts
golang.org/x/net/bpf
golang.org/x/net/context
golang.org/x/net/context/ctxhttp
golang.org/x/net/dict
golang.org/x/net/dns/dnsmessage
golang.org/x/net/html
golang.org/x/net/html/atom
golang.org/x/net/html/charset
golang.org/x/net/http/httpguts
golang.org/x/net/http/httpproxy
golang.org/x/net/http2
golang.org/x/net/http2/h2c
golang.org/x/net/http2/h2i
golang.org/x/net/http2/hpack
golang.org/x/net/icmp
golang.org/x/net/idna
golang.org/x/net/internal/iana
golang.org/x/net/internal/nettest
golang.org/x/net/internal/socket
golang.org/x/net/internal/socks
golang.org/x/net/internal/sockstest
golang.org/x/net/internal/timeseries
golang.org/x/net/ipv4
golang.org/x/net/ipv6
golang.org/x/net/nettest
golang.org/x/net/netutil
golang.org/x/net/proxy
golang.org/x/net/publicsuffix
golang.org/x/net/trace
golang.org/x/net/webdav
golang.org/x/net/webdav/internal/xml
golang.org/x/net/websocket
golang.org/x/net/xsrftoken
golang.org/x/sys/cpu
golang.org/x/sys/unix
golang.org/x/sys/windows
golang.org/x/sys/windows/registry
golang.org/x/text
golang.org/x/text/cases
golang.org/x/text/cmd/gotext
golang.org/x/text/collate
golang.org/x/text/collate/build
golang.org/x/text/collate/tools/colcmp
golang.org/x/text/currency
golang.org/x/text/encoding
golang.org/x/text/encoding/charmap
golang.org/x/text/encoding/htmlindex
golang.org/x/text/encoding/ianaindex
golang.org/x/text/encoding/internal
golang.org/x/text/encoding/internal/enctest
golang.org/x/text/encoding/internal/identifier
golang.org/x/text/encoding/japanese
golang.org/x/text/encoding/korean
golang.org/x/text/encoding/simplifiedchinese
golang.org/x/text/encoding/traditionalchinese
golang.org/x/text/encoding/unicode
golang.org/x/text/encoding/unicode/utf32
golang.org/x/text/feature/plural
golang.org/x/text/internal
golang.org/x/text/internal/catmsg
golang.org/x/text/internal/colltab
golang.org/x/text/internal/export/idna
golang.org/x/text/internal/format
golang.org/x/text/internal/gen
golang.org/x/text/internal/number
golang.org/x/text/internal/stringset
golang.org/x/text/internal/tag
golang.org/x/text/internal/testtext
golang.org/x/text/internal/triegen
golang.org/x/text/internal/ucd
golang.org/x/text/internal/utf8internal
golang.org/x/text/language
golang.org/x/text/language/display
golang.org/x/text/message
golang.org/x/text/message/catalog
golang.org/x/text/number
golang.org/x/text/runes
golang.org/x/text/search
golang.org/x/text/secure
golang.org/x/text/secure/bidirule
golang.org/x/text/secure/precis
golang.org/x/text/transform
golang.org/x/text/unicode
golang.org/x/text/unicode/bidi
golang.org/x/text/unicode/cldr
golang.org/x/text/unicode/norm
golang.org/x/text/unicode/rangetable
golang.org/x/text/unicode/runenames
golang.org/x/text/width
golang.org/x/tools/benchmark/parse
golang.org/x/tools/blog
golang.org/x/tools/blog/atom
golang.org/x/tools/cmd/benchcmp
golang.org/x/tools/cmd/bundle
golang.org/x/tools/cmd/callgraph
golang.org/x/tools/cmd/compilebench
golang.org/x/tools/cmd/cover
golang.org/x/tools/cmd/digraph
golang.org/x/tools/cmd/eg
golang.org/x/tools/cmd/fiximports
golang.org/x/tools/cmd/getgo
golang.org/x/tools/cmd/getgo/server
golang.org/x/tools/cmd/go-contrib-init
golang.org/x/tools/cmd/godex
golang.org/x/tools/cmd/godoc
golang.org/x/tools/cmd/goimports
golang.org/x/tools/cmd/gomvpkg
golang.org/x/tools/cmd/gorename
golang.org/x/tools/cmd/gotype
golang.org/x/tools/cmd/goyacc
golang.org/x/tools/cmd/guru
golang.org/x/tools/cmd/guru/serial
golang.org/x/tools/cmd/heapview
golang.org/x/tools/cmd/heapview/internal/core
golang.org/x/tools/cmd/html2article
golang.org/x/tools/cmd/present
golang.org/x/tools/cmd/ssadump
golang.org/x/tools/cmd/stress
golang.org/x/tools/cmd/stringer
golang.org/x/tools/cmd/tip
golang.org/x/tools/cmd/toolstash
golang.org/x/tools/container/intsets
golang.org/x/tools/cover
golang.org/x/tools/go/ast/astutil
golang.org/x/tools/go/buildutil
golang.org/x/tools/go/callgraph
golang.org/x/tools/go/callgraph/cha
golang.org/x/tools/go/callgraph/rta
golang.org/x/tools/go/callgraph/static
golang.org/x/tools/go/gccgoexportdata
golang.org/x/tools/go/gcexportdata
golang.org/x/tools/go/internal/cgo
golang.org/x/tools/go/internal/gccgoimporter
golang.org/x/tools/go/internal/gcimporter
golang.org/x/tools/go/loader
golang.org/x/tools/go/packages
golang.org/x/tools/go/packages/golist
golang.org/x/tools/go/packages/gopackages
golang.org/x/tools/go/packages/raw
golang.org/x/tools/go/pointer
golang.org/x/tools/go/ssa
golang.org/x/tools/go/ssa/interp
golang.org/x/tools/go/ssa/ssautil
golang.org/x/tools/go/types/typeutil
golang.org/x/tools/go/vcs
golang.org/x/tools/godoc
golang.org/x/tools/godoc/analysis
golang.org/x/tools/godoc/redirect
golang.org/x/tools/godoc/static
golang.org/x/tools/godoc/util
golang.org/x/tools/godoc/vfs
golang.org/x/tools/godoc/vfs/gatefs
golang.org/x/tools/godoc/vfs/httpfs
golang.org/x/tools/godoc/vfs/mapfs
golang.org/x/tools/godoc/vfs/zipfs
golang.org/x/tools/imports
golang.org/x/tools/internal/fastwalk
golang.org/x/tools/playground
golang.org/x/tools/playground/socket
golang.org/x/tools/present
golang.org/x/tools/refactor/eg
golang.org/x/tools/refactor/importgraph
golang.org/x/tools/refactor/rename
golang.org/x/tools/refactor/satisfy
rsc.io/quote
rsc.io/quote/buggy
rsc.io/sampler

Whereas what I'm actually interested in is (ignoring build constraints for now):

$ go list -f "{{if not .Standard}}{{.ImportPath}}{{end}}" $(go  list -f '{{ join .Deps  "\n"}}' $(go list -m)/...) $(go list -m)/...
golang.org/x/text/internal/tag
golang.org/x/text/language
rsc.io/quote
rsc.io/sampler
example.com/hello

@bcmills
Copy link
Contributor

bcmills commented Aug 9, 2018

The more I think about it, the more I think the current patterns are correct. Here's my reasoning:

  • all is the pattern you want for guru and other analysis tools. They need to scan and index the relevant parts of the standard library in order to make behaviors like “jump to definition” work uniformly.
  • ... is the pattern you want for pre-release testing. Your dependencies likely have a few integration tests, and for large integration tests, the packages that provide those integration tests might be distinct from the packages being tested. And integration tests are exactly the ones that are important to run when you're testing new combinations of dependency versions.
  • ./... is the pattern you want for day-to-day feature testing. You know your dependencies are fine, and you just want to re-test the module you're working on.

So I think this is mostly just a documentation issue: we should be recommending go test ./... or go test ..., not go test all.

@bcmills
Copy link
Contributor

bcmills commented Aug 9, 2018

@myitcv

go list ./...   // all packages in the current directory and below,
                // excluding modules in subdirectories (which may be submodules)

Not just subdirectories, but excluding submodules in general (even if they aren't stored in subdirectories).

go list m/...   // all packages in module m (by definition this does not
                // include submodules)

No, this one is “all packages in all modules with prefix m/

go list ...     // all packages in the current module and its module dependencies

go list all     // the transitive set of package imports, including std lib

Yep.

go test all     // equivalent to go test $(go list all)

Correct: go test <pattern> should always be equivalent to go test $(go list <pattern>).

@myitcv
Copy link
Member Author

myitcv commented Aug 9, 2018

@bcmills

... is the pattern you want for pre-release testing.

Is this really what you want for pre-release testing though? Because given my example in #26317 (comment) this would involve me testing golang.org/x/tools/playground which (amongst other packages in that list) is totally irrelevant unless I'm missing something obvious here?

@bcmills
Copy link
Contributor

bcmills commented Aug 9, 2018

Given that we don't have any other means to specify “package example.com/x/y is an integration test for package example.com/z/w”, I think so, yes. (Figuring out some sort of specification for that is a whole other can of worms.)

If caching is working properly, the overhead of “running” those tests should generally be fairly minimal — especially if we split the x/ repos into smaller modules and/or start giving more of them semantic version tags.

@myitcv
Copy link
Member Author

myitcv commented Aug 9, 2018

@bcmills

Given that we don't have any other means to specify “package example.com/x/y is an integration test for package example.com/z/w”, I think so, yes.

I see what I was missing, thanks.

(Figuring out some sort of specification for that is a whole other can of worms.)

True, but an interesting/worthwhile idea though.

split the x/ repos into smaller modules and/or start giving more of them semantic version tags.

Indeed.

In addition to this point, the bit I clearly forgot in all of this is the first line of go help modules:

A module is a collection of related Go packages.

which helps to make more sense of the definition for ....

Thanks for taking the time to talk this through.

Do you want to leave this issue open in order to address:

So I think this is mostly just a documentation issue: we should be recommending go test ./... or go test ..., not go test all.

??

Will leave up to you.

@rsc
Copy link
Contributor

rsc commented Aug 9, 2018

The standard library is incorrectly excluded from ... in module mode. It should be included.

We should be encouraging people to use all to test the current module and its dependencies. Testing ... instead will test lots of completely irrelevant packages. One of the main reasons for redefining all was precisely to make it useful for go test [-short] all.

@mwf
Copy link

mwf commented Aug 9, 2018

We should be encouraging people to use all to test the current module and its dependencies.

@rsc do you mean we should exclude standard library from all then? Otherwise it could be painful to use even with -short mode on.

@rsc
Copy link
Contributor

rsc commented Aug 9, 2018

all means "all your dependencies - the things that actually get built into the packages in your modules". I don't see a compelling reason to exclude fmt if you use fmt but not exclude something else 10 levels down. Especially if you use -short, it should not be painful. And it should cache very well.

@mwf
Copy link

mwf commented Aug 9, 2018

Thanks for the clarification!

OK, let's try to live with it. Only time could judge wether the community accepts this behaviour or not 😄

Maybe it's reasonable for a case, when there is really something wrong in your OS, making standard library tests to fail :) And it could help us to debug some crazy cases in future go releases.

gopherbot pushed a commit that referenced this issue Aug 9, 2018
This change replaces https://golang.org/cl/125835.

Updates #26317.

Change-Id: I38ae1f93e5f5c86737a4b489df498c18b179781d
Reviewed-on: https://go-review.googlesource.com/128637
Reviewed-by: Russ Cox <rsc@golang.org>
@rsc
Copy link
Contributor

rsc commented Aug 10, 2018

I think this is now working as documented, so closing.

@anacrolix
Copy link
Contributor

I'm on a very recent Macbook (Pro, 2018, 6 cores): Running go test std with GO111MODULE=on takes (even with everything cached):

real	1m43.740s
user	3m43.726s
sys	0m48.724s

because of

--- FAIL: TestCrashDumpsAllThreads (1.75s)
    crash_unix_test.go:70: building source: exit status 1
        go: cannot find main module; see 'go help modules'
FAIL
FAIL	runtime	98.016s

Without the tests cached:

real	10m23.312s
user	27m35.928s
sys	1m34.860s

I think there's a bad assumption here that the std tests "should" always succeed (and that we can handwave them away with caching), because they do not, and from cold-start they are very time-consuming.

I'd like to do go test all as a quick check that everything I'm depending on works together, it's not convenient or a quick part of my workflow if it can blow out like this so easily.

@bcmills
Copy link
Contributor

bcmills commented Mar 28, 2019

I think there's a bad assumption here that the std tests "should" always succeed (and that we can handwave them away with caching), because they do not

If you find that a test in std is consistently failing, please file an issue with steps to reproduce the failure.

@golang golang locked and limited conversation to collaborators Mar 27, 2020
@rsc rsc unassigned bcmills Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants