go/src: all.bash not picking up changes in go/src/cmd/go/testdata/script directory

[zpavlinovic]

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

$ go version
go version devel +b7a85e0003 linux/amd64

Does this issue reproduce with the latest release?

Yes. I pulled the most recent version of Go master branch.

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

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/usr/local/google/home/zpavlinovic/.cache/go-build"
GOENV="/usr/local/google/home/zpavlinovic/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/usr/local/google/home/zpavlinovic/go/pkg/mod"
GONOPROXY="*.git.corp.google.com"
GONOSUMDB="*.git.corp.google.com"
GOOS="linux"
GOPATH="/usr/local/google/home/zpavlinovic/go"
GOPRIVATE="*.git.corp.google.com"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/google/home/zpavlinovic/golang-dev/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/google/home/zpavlinovic/golang-dev/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="devel +b7a85e0003"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/usr/local/google/home/zpavlinovic/golang-dev/go/src/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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2036562605=/tmp/go-build -gno-record-gcc-switches"

What did you do?

1. Built Go using go/src$ ./all.bash
2. Introduced a ^WRONGPREFIX change in go/src/cmd/go/testdata/script/test_vet.txt

[short] skip
  
# Test file
! go test p1_test.go
stderr 'Logf format %d'
go test -vet=off
stdout '^ok'

# Non-test file
! go test p1.go
stderr 'Printf format %d'
go test -x -vet=shift p1.go
stderr '[\\/]vet.*-shift'
stdout '\[no test files\]'
go test -vet=off p1.go
! stderr '[\\/]vet.*-shift'
stdout '\[no test files\]'

# Test issue #22890
go test m/vetcycle
stdout 'm/vetcycle.*\[no test files\]'

# Test with ...
! go test ./vetfail/...
stderr 'Printf format %d'
stdout '^WRONGPREFIXok\s+m/vetfail/p2'

...

3. Build and test again go/src$ ./all.bash

What did you expect to see?

Based on instructions in Contribution Guide, failure along the lines of

go test proxy running at GOPROXY=http://127.0.0.1:46381/mod
--- FAIL: TestScript (0.01s)
    --- FAIL: TestScript/test_vet (4.10s)
        script_test.go:252: 
            # Test file (1.304s)
            # Non-test file (1.315s)
            # Test issue #22890 (0.487s)
            # Test with ... (0.866s)
            > ! go test ./vetfail/...
            [stdout]
            ok          m/vetfail/p2    0.023s
            [stderr]
            # m/vetfail/p1
            vetfail/p1/p1.go:1:1: invalid non-alphanumeric build constraint: !foo-bar
            vetfail/p1/p1.go:8:2: Printf format %d has arg "hello" of wrong type string
            [exit status 2]
            > stderr 'Printf format %d'
            > stdout '^WRONGPREFIXok\s+m/vetfail/p2'
            FAIL: testdata/script/test_vet.txt:26: no match for `(?m)^WRONGPREFIXok\s+m/vetfail/p2` found in stdout
            
FAIL
exit status 1
FAIL    cmd/go  61.526s

This failure can be reproduced by running go test -run TestScript in go/src/cmd/go (after setting the GOROOT properly; see output of go env above).

What did you see instead?

Success.

...
ALL TESTS PASSED

[ianlancetaylor]

Do you have GOROOT set in the environment when you run all.bash?

[zpavlinovic]

My GOROOT is set to src/go. go env above shows it. I've set it up this way so go test can pick up the changes; otherwise, it also does not.

[ianlancetaylor]

What happens if you run all.bash without setting GOROOT in the environment?

In general the Go tools behave badly when GOROOT is set. GOROOT exists today mainly for specialized uses like running a cmd/go binary that was not built from the sources that you are using.

[zpavlinovic]

With GOROOT="/usr/lib/google-golang", I got success again

...
ALL TESTS PASSED
...

[jayconrod]

all.bash runs tests with -short, so the line [short] skip in your test will cause it to be skipped. You can test with go test cmd/go -run=TestScript/test_vet instead, or more broadly, go test cmd/go/....

As @ianlancetaylor, it's best not to set GOROOT, since the default is two directories up from where the go command is installed. /usr/local/google/home/zpavlinovic/golang-dev/go looks correct.

[zpavlinovic]

I wonder if that is what we want here. At the very least, I would expect such tests to be run by continuous integration: https://go-review.googlesource.com/c/go/+/341735.

[jayconrod]

The longtest builders do run these tests, but they aren't run by default in TryBots (presubmit builds) to shorten turnaround time and save CPU. You can opt into them with SlowBots, for example, by adding TRY=linux-amd64-longtest when running TryBots.

Longtest builders do run regularly after changes are merged. https://build.golang.org/ shows their status.

[zpavlinovic]

So, in summary

• all.bash does not run all tests, only short ones
• longtests including script tests are run only by SlowBots by the CI system if explicitly specified
• longtests including script tests are run periodically after merging

I understand the need for this setup, so I am not suggesting something should necessarily be changed there. However, I wonder if documentation can updated. If someone is, say, changing flags for commands, these script tests will likely be needed. With no documentation, potential bugs can creep into the repo and only discovered later after the fact.

[jayconrod]

All correct. And bugs creeping in post-merge, only visible on the build.golang.org dashboard does happen.

I'll close this issue since it's not a bug per so, but I'd encourage you to bring this up on the golang-dev mailing list for discussion with a wider audience.

Documentation would help, but there are a lot of moving parts here, and there should be some discussion about where it should live so it's discoverable.