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: go test all should not run standard library tests #29676

Closed
darkfeline opened this Issue Jan 11, 2019 · 7 comments

Comments

Projects
None yet
5 participants
@darkfeline
Copy link

darkfeline commented Jan 11, 2019

Bugs for the test faliures:

#28387
#29712
#29713

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

$ go version
go version go1.11.4 linux/amd64

Does this issue reproduce with the latest release?

Yes, 1.11.4 is the latest (not counting betas?).

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

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/ionasal/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/ionasal/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=""
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-build457575040=/tmp/go-build -gno-record-gcc-switches"

What did you do?

mkdir -p /tmp/tmp3
cd /tmp/tmp3
go mod init example.com/test
cat >main.go <<EOF
package foo

import (
	_ "net"
	_ "os"
	_ "path/filepath"
)
EOF
go test all

What did you expect to see?

All tests pass

What did you see instead?

Test failures:

--- FAIL: TestLookupDotsWithLocalSource (0.00s)
    lookup_test.go:612: netgo: got localhost.; want localhost
FAIL
FAIL	net	29.770s
--- FAIL: TestStatError (0.00s)
    os_test.go:200: symlink no-such-file symlink: permission denied
--- FAIL: TestHardLink (0.00s)
    os_test.go:694: open "hardlinktestto" failed: open hardlinktestto: permission denied
--- FAIL: TestSymlink (0.00s)
    os_test.go:780: Create("symlinktestto") failed: open symlinktestto: permission denied
--- FAIL: TestLongSymlink (0.00s)
    os_test.go:847: symlink "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", "longsymlinktestfrom" failed: symlink 0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef longsymlinktestfrom: permission denied
--- FAIL: TestRename (0.00s)
    os_test.go:868: open "renamefrom" failed: open renamefrom: permission denied
--- FAIL: TestRenameOverwriteDest (0.00s)
    os_test.go:896: write file "renameto" failed: open renameto: permission denied
--- FAIL: TestAppend (0.00s)
    os_test.go:1684: Open: open append.txt: permission denied
--- FAIL: TestSameFile (0.00s)
    os_test.go:1766: Create(a): open a: permission denied
FAIL
FAIL	os	18.381s
--- FAIL: TestBug3486 (0.00s)
    path_test.go:1268: lstat /usr/lib/go/test: no such file or directory
FAIL
FAIL	path/filepath	0.013s
  1. I think it's ridiculous that the tests for the standard library fail.
  2. It's odd that the tests for the standard library are run. I think it's safe to assume a release of Go is already tested. The tests for the standard library take a lot of time to run for something that is likely pointless for the vast majority of users (perhaps this makes sense for some people running non-release versions of Go).
  3. go test all is the Blessed command suggested by the Modules doc.
@ALTree

This comment has been minimized.

Copy link
Member

ALTree commented Jan 11, 2019

Thanks for reporting this.

I see two different issues here

  1. A few stdlib packages have tests that fail on some environments
  2. go test all runs the tests of all dependencies, included the stdlib one, which is expensive and maybe overkill

Regarding (1), I think it would be better to report those failures separately, by opening new github issues (you could group the failures by package).

Regarding (2), while I personally agree with this:

The tests for the standard library take a lot of time to run for something that is likely pointless for the vast majority of users

it's also true that special-casing them would make the whole system less elegant, and the all test not really exhaustive. Right now, it's simple: if you depend on it, all runs its test. It doesn't really matter if it's an external package or a stdlib dependency. After all, from a "philosophical" point of view, stdlib package are not "special" or different from non-stdlib packages.

I'm going to re-title this specific issue to be about (2), if you don't mind. Feel free to open new issues about the specific test failures on your system, since those should be fixed anyway, and separately from the resolution of (2).

@ALTree ALTree changed the title go test all fails out of the box cmd/go: go test all should not run standard library tests Jan 11, 2019

@ALTree ALTree added this to the Go1.13 milestone Jan 11, 2019

@dpinela

This comment has been minimized.

Copy link
Contributor

dpinela commented Jan 12, 2019

The stdlib test failures may be related to #28387. If they are, some (like package os) are already fixed on tip.

@darkfeline

This comment has been minimized.

Copy link

darkfeline commented Jan 12, 2019

Thanks. Regarding desired behavior, some personal observations of Go's philosophy:

  1. There should be a balance between elegance and practicality.
  2. It's more important for the interface to be simple than the implementation be simple.
  3. Go prioritizes compile speed; it seems like test speed should also be prioritized as part of the development cycle.

If we do some basic calculations of how much time is wasted by users running the stdlib tests globally (assuming they run go test all) I'm sure it would be quite large. Of course they can always run a more complex command to filter out stdlib tests, but in my opinion the common case should be the default.

Also, there's very little a developer can do if the stdlib tests fail except maybe downgrade their version of Go, or construct a local environment where the tests pass. So whether or not the stdlib tests fail as part of go test all, there's nothing a developer can do. In which case, what's the point?

@thepudds

This comment has been minimized.

Copy link

thepudds commented Jan 13, 2019

I think part of the theory is:

  1. if a standard library test fails, it might be because of a legitimate bug in the standard library that is triggered in a specific environment, or because of an improper environment setup, and in either case case might be good to identify those types of problems.
  2. the standard library test results should in theory cache well on a given machine.
  3. this is new behavior, so might have some rough spots as it rolls out as more people more frequently run the standard library tests in more environments, but I think the hope is the frequency of the standard library improperly failing would go down to the point where failures are seen as useful, rather than noise.

In any event, I think that was part of the theory, but certainly makes sense to evaluate how things are working in practice.

Also, there's very little a developer can do if the stdlib tests fail except maybe downgrade their version of Go, or construct a local environment where the tests pass. So whether or not the stdlib tests fail as part of go test all, there's nothing a developer can do.

One thought would be go list is fairly flexible. For example, I think this might return the import paths for your main package and its dependencies, exclusive of the standard library:

go list -deps -f '{{if not .Standard}}{{.ImportPath}}{{end}}'

If that is correct, then perhaps as a work-around in the near-term you could couple it with go test, such as something along the lines of:

go test $(go list -deps -f '{{if not .Standard}}{{.ImportPath}}{{end}}')

My morning coffee is still kicking in, so please treat that as something that might be in the ball park of what might help, rather than a definitive answer. Also, that of course is not an ideal long-term solution if people had to do something like that frequently. In any event, see the official doc, and also Dave Cheney has a great blog post on using 'go list'.

@darkfeline

This comment has been minimized.

Copy link

darkfeline commented Jan 14, 2019

  1. if a standard library test fails, it might be because of a legitimate bug in the standard library that is triggered in a specific environment, or because of an improper environment setup, and in either case case might be good to identify those types of problems.

That sounds reasonable, but for most users using common environments, running the tests is a waste of CPU cycles. Test caching, which I forgot about, would help immensely, but I imagine that makes disabling caching expensive without also filtering out stdlib tests (again, a question of defaults. There would of course be ways to explicitly run stdlib tests).

  1. the standard library test results should in theory cache well on a given machine.
  2. this is new behavior, so might have some rough spots as it rolls out as more people more frequently run the standard library tests in more environments, but I think the hope is the frequency of the standard library improperly failing would go down to the point where failures are seen as useful, rather than noise.

Oops, yes, Test caching would alleviate a lot of overhead.

@bcmills

This comment has been minimized.

Copy link
Member

bcmills commented Jan 14, 2019

If a standard library test fails, that should provide a strong signal: it should indicate either a bug (perhaps an invalid assumption) in the Go standard library, or a defect of some sort in your particular platform (perhaps a kernel bug).

The fact that standard library tests do not all work reliably today is something we should fix, but skipping those tests (and discarding the information they could otherwise provide) is not a great solution.

@bcmills

This comment has been minimized.

Copy link
Member

bcmills commented Jan 14, 2019

Closing as duplicate of #26317. (See especially #26317 (comment).)

@bcmills bcmills closed this Jan 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment