path/filepath: TestBug3486 fails if GOROOT/test is removed

[darkfeline]

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 (
    _ "path/filepath"
)
EOF
go test all

What did you expect to see?

All tests pass

What did you see instead?

Test failures:

--- FAIL: TestBug3486 (0.00s)
    path_test.go:1268: lstat /usr/lib/go/test: no such file or directory
FAIL
FAIL    path/filepath   0.013s

(I found a bug for the same test, but the failure mode is different: https://github.com/golang/go/issues/5863)

[bcmills]

CC @robpike @rsc for path/filepath

[bcmills]

How did you install your copy of the go toolchain?

The test is looking for the test subdirectory of runtime.GOROOT(): https://github.com/golang/go/blob/56c9f8e8cfecafda3bd9f58c6421cd253a770d54/src/path/filepath/path_test.go#L1293

The test passes when run with the go1.11.5 binary obtained via go get golang.org/dl/go1.11.5:

scratch$ go mod init golang.org/issue/scratch
go: creating new go.mod: module golang.org/issue/scratch

scratch$ go1.11.5 test -v -run=TestBug3486 path/filepath
=== RUN   TestBug3486
--- PASS: TestBug3486 (0.00s)
PASS
ok      path/filepath   0.023s

And the directory is clearly present in the official distribution tarball, at least for linux/amd64:

scratch$ curl -O https://dl.google.com/go/go1.11.5.src.tar.gz
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 20.1M  100 20.1M    0     0  55.6M      0 --:--:-- --:--:-- --:--:-- 55.4M

scratch$ tar -zxf go1.11.5.src.tar.gz

scratch$ ls -dF go/test
go/test/

So as far as I can tell, the most likely explanation is some sort of packaging issue. Perhaps your distro maintainer pruned files out of GOROOT without realizing that the test suite requires them?

[darkfeline]

I'm using the official go package for Arch Linux. It does seem to omit the test dir.

I can file a bug with the distro, but is the test dir required as part of a Go install? I have had no issues with it missing (other than this test failure). It also seems wrong that a stdlib test requires the toolchain/runtime test dir to exist. And furthermore end users almost certainly would have no need for toolchain/runtime tests for a Go install?

[bcmills]

is the test dir required as part of a Go install?

There is currently nothing in the documentation that says it can be removed, so I'd say that yes, it is required.

I would argue that anything that is safe to prune out for individual distros should also be pruned out of the standard release tarball (see #27151), and conversely, anything that is not pruned out of the standard release should not be pruned out in individual distros either.

If you think we should prune out the test directory (and make the tests pass without it), that's probably something to mention on #27151.

[bcmills]

Marking as NeedsDecision: we need to determine whether ordinary tests in the standard library should depend on the existence of GOROOT/test.

[ianlancetaylor]

I think it's perfectly fine for standard library tests to require the existence of GOROOT/test, just as on some systems they require the existence of GOROOT/lib.

In general, if you don't have a complete distribution, you shouldn't expect the tests to pass.

This is not to say that Arch Linux is doing the wrong thing by omitting the test directory from their default installation of Go. It's only to say that once they've made that decision, there is no reason for them to expect the standard library tests to continue to pass.

[darkfeline]

I believe this is the only stdlib test that requires GOROOT/test though (since I only ran into 2-3 failures, the others of which were unrelated to GOROOT/test). From what I can tell GOROOT/test is for toolchain/runtime tests. For a single stdlib test to add a dependency on all of GOROOT/test for what I suspect is just a test that needs to stat an arbitrary dir (will look at the test later) seem rather excessive.

Edit: Looking at https://github.com/golang/go/blob/447965d4e008764a8635df6ca7d5d2e59c6d4229/src/path/filepath/path_test.go#L1286

I think it could be easily rewritten to not rely on GOROOT/test. I'd be happy to write a patch if a Go dev agrees that it makes sense.

[ianlancetaylor]

It is excessive and I don't mind changing it for the 1.13 release. But the general principle still holds: if the distro doesn't include the complete Go release, then you should not expect the standard library tests to pass. That just isn't an invariant we have any reason to maintain, nor are we going to test that it works.

[darkfeline]

I understand and that's fine.

However if you'll entertain a little philosophical discussion, I don't see when it should ever be the case that stdlib tests rely on any files outside of the src/ dir. If any stdlib test needs any additional files, shouldn't they be included as testdata like tests for regular Go packages would? Again, I understand if that invariant is not formally maintained, but would there ever be a reason to violate that invariant?

That just isn't an invariant we have any reason to maintain

Assuming there are no such reasons to violate this invariant (which of course could be a mistaken assumption on my part), one potential reason to maintain this invariant is that if the "lite" Go distribution idea in https://github.com/golang/go/issues/27151 gains traction, a "lite" Go distribution that omits the test/ directory would still be able to run stdlib tests as part of a normal go test all workflow. If the only thing preventing this is spending a little effort to be conscientious about putting stdlib test files in testdata, it seems like a worthy investment (again, hypothetically).

I have already filed a downstream bug, so I'm not pushing this idea with the intent of changing Go policy for the sake of fixing an Arch Linux bug. I'd just like to clarify my own thoughts about this.

[ianlancetaylor]

This kind of thing is only maintainable if it is tested. So if we decide that we care about a "lite" Go distribution, then our first step would be to arrange for our automated testers to remove the appropriate directories and then run all the tests. Unless and until we take that step, this simply isn't worth worrying about.

[darkfeline]

I took a little bit of time to see if I could fix this, and ironically, the reason this test is using the GOROOT/test subdir is to work around another stdlib test bug https://github.com/golang/go/issues/28387 that is also causing go test all to fail.

This test was originally written for https://github.com/golang/go/issues/3486 and only check GOROOT/lib and GOROOT/src. It was changed to use GOROOT/test in https://github.com/golang/go/commit/5dc8c4dbfb6a04d9eb7a11c9c3fe698d33d0c0ee because the os tests write files into GOROOT/src causing a race. But it is exactly this behavior also causing os tests to fail in https://github.com/golang/go/issues/28387.

So I will postpone fixing this bug until https://github.com/golang/go/issues/28387 is resolved.

[ALTree]

Looks like there's now another std lib test that depends on GOROOT/test existing. Failure if it's deleted:

--- FAIL: TestImportTypeparamTests (0.00s)
    gcimporter_test.go:168: open /workdir/go/test/typeparam: no such file or directory
FAIL
FAIL    go/internal/gcimporter  0.482s
FAIL