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/dist: regex of test does not allow test names in -run parameter #29413

Closed
iWdGo opened this issue Dec 25, 2018 · 11 comments

Comments

Projects
None yet
4 participants
@iWdGo
Copy link
Contributor

commented Dec 25, 2018

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

go version go1.11.4 windows/amd64

Does this issue reproduce with the latest release? Yes, on tip

>go tool dist version
devel +c482e0cd38 Sun Dec 23 14:07:55 2018 +0100

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

go env Output
$ go env
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\***\AppData\Local\go-build
set GOEXE=.exe
set GOFLAGS=-v -vet=off
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=C:\Users\***\Documents\Google\
set GOPROXY=
set GORACE=
set GOROOT=C:\Users\***\Documents\Google\golang\go
set GOTMPDIR=
set GOTOOLDIR=C:\Users\***\Documents\Google\golang\go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set CXX=g++
set CGO_ENABLED=0
set GOMOD=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\***\AppData\Local\Temp\go-build911439174=/tmp/go-build -gno-record-gcc-switches

What did you do?

C:\Program Files (x86)\Google\Cloud SDK>go tool dist test -run=^TestScript:cmd/go$

What did you expect to see?

C:\Program Files (x86)\Google\Cloud SDK>go tool dist test -run=^cmd/go$

##### Testing packages.
...(some tests executed)
ALL TESTS PASSED (some were excluded)

What did you see instead?

No test is executed.

C:\Program Files (x86)\Google\Cloud SDK>go tool dist test -run=^TestScript:cmd/go$

ALL TESTS PASSED (some were excluded)

This is especially a problem to test cmd/go package.
Running the tests of cmd/go times out even in short mode (#25300).
Further, In short mode, no external network is usable and no test of f.i. of git is doable (#26837).

The fix handles the regex in the format testname:package-name as an added case.

@gopherbot

This comment has been minimized.

Copy link

commented Dec 25, 2018

Change https://golang.org/cl/155838 mentions this issue: cmd/dist: fix test command to allow usage of test names in -run parameter

@mvdan

This comment has been minimized.

Copy link
Member

commented Dec 25, 2018

As far as I know, go tool dist test is meant to test the entirety of Go. If you just want to test the scripts within cmd/go, you can just do go test -run=Script cmd/go.

@iWdGo

This comment has been minimized.

Copy link
Contributor Author

commented Dec 25, 2018

The purpose of go tool dist test seems to re-build the tests of the updated packages and to only run the relevant tests.

A new build created in golang/go/bin/ using make.bat requires to set GOROOT to bin\.. folder. Then go tool dist is usable.

go test gets an error for several packages which is related to a version file as far as I know.

golang\go\src>set GO
GOFLAGS=-vet=off
GOPATH=C:\Users\***\Documents\Google\
GOROOT=C:\Users\***\Documents\Google\golang\go
GOROOT_BOOTSTRAP=c:\Go
GO_TEST_SHORT=0
golang\go\src>go tool dist version
devel +7ef8502d16 Tue Dec 25 11:33:12 2018 +0100
golang\go\src>go test -run=Script cmd/go
...
# internal/cpu
compile: version "devel +7ef8502d16 Tue Dec 25 11:33:12 2018 +0100" does not match go tool version "go1.11.4"
...

This fix seems inline with the description of the tool and allows to fix tests independently of the build process as the -compile-only flag hints:

golang\go\src>go tool dist test -h
usage: go tool dist test [options]
...
  -compile-only
        compile tests, but don't run them. This is for some builders. Not all dist tests respect this flag, but most do.
...
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Dec 25, 2018

The purpose of go tool dist test seems to re-build the tests of the updated packages and to only run the relevant tests.

Yes, but only during the process of a complete build of Go. If you just want to test a package yourself, just use go test. No reason to use go tool dist test. The purpose of the -run option to go tool dist test is to permit sharding the tests across several different automatic builders. I don't see any reason for anybody other than the builders to run go tool dist test -run themselves. Just use go test.

@iWdGo

This comment has been minimized.

Copy link
Contributor Author

commented Dec 25, 2018

When working on several projects, a stable Go version resides in the default directory. When re-building go to investigate some issue. This unstable version resides under golang\go\ as all tools foresee.

Using go test on this unstable requires to override the stable version or to update the go env. This is cumbersome and a flag is easy to forget when you work on a single machine. This set up allows to easily switch between projects.

If go tool dist is targeted at builders. This issue might be eventually marked as a proposal.

@mvdan

This comment has been minimized.

Copy link
Member

commented Dec 25, 2018

If you want to work on Go's master/tip version, but still be able to use the stable version, you can simply use aliases. For example, go on my system is always master, and then I do alias go1=/usr/bin/go, which currently is 1.11.x. Some people do the opposite, with gotip being an alias to the master version. Just using the right binary should be enough; there's no need to fiddle with GOPATH.

Like @ianlancetaylor said, if you're working on Go itself, you're expected to just use go test. I'm going to close this issue for now, as there doesn't seem to be a particular suggestion or bug here. Feel free to reopen or create a new issue if you have a particular suggestion for the go tool, but I'd recommend discussing it with other Go contributors before sending proposals or CLs.

@mvdan mvdan closed this Dec 25, 2018

@iWdGo

This comment has been minimized.

Copy link
Contributor Author

commented Jan 13, 2019

On Windows, this is not solving the issue. Here is the process go 1.12 beta 2 of which some seems undocumented when reading https://golang.org/doc/install. I use GOPATH to distinguish development work on Go from the rest. Go is installed on the default directory c:\Go.
Below the path is removed and > stands for the prompt in CMD.

>go get golang.org/dl/go1.12beta2
>cd %GOPATH%
>dir bin go*
19-12-18  07:08         6.308.864 go1.12beta1.exe
13-01-19  15:05         7.003.136 go1.12beta2.exe
>cd bin
>go1.12beta2.exe download
...
>cd %USERPROFILE%\sdk\
>dir
...
 Directory of %USERPROFILE\sdk

13-01-19  15:08    <DIR>          .
13-01-19  15:08    <DIR>          ..
13-01-19  15:16    <DIR>          go1.12beta2
               0 File(s)              0 bytes

>doskey go112beta2=%USERPROFILE%\sdk\go1.12beta2\bin\go.exe $*

>cd my/sandbox/path
>go112beta2 version
go version go1.12beta2 windows/amd64
>go112beta2 test .
# internal/race
compile: version "go1.12beta1" does not match go tool version "go1.12beta2"
# errors
compile: version "go1.12beta1" does not match go tool version "go1.12beta2"
# internal/syscall/windows/sysdll
compile: version "go1.12beta1" does not match go tool version "go1.12beta2"
# runtime/internal/sys
compile: version "go1.12beta1" does not match go tool version "go1.12beta2"
# unicode/utf16
compile: version "go1.12beta1" does not match go tool version "go1.12beta2"
# internal/cpu
compile: version "go1.12beta1" does not match go tool version "go1.12beta2"
# sync/atomic
compile: version "go1.12beta1" does not match go tool version "go1.12beta2"
# unicode/utf8
compile: version "go1.12beta1" does not match go tool version "go1.12beta2"
# unicode
compile: version "go1.12beta1" does not match go tool version "go1.12beta2"
# math/bits
compile: version "go1.12beta1" does not match go tool version "go1.12beta2"
# runtime/internal/atomic
compile: version "go1.12beta1" does not match go tool version "go1.12beta2"

>set GOROOT=%USERPROFILE%\sdk\go1.12beta2\
>go112beta2 test .
(works as expected)

Documentation is at least incomplete. I believe that this issue is worth re-opening.

@mvdan

This comment has been minimized.

Copy link
Member

commented Jan 20, 2019

Your last comment does not contain -run, so I don't think we should reopen this issue; that would be confusing. If you found a bug when go-getting specific Go versions, please open a new issue. As for documenting that process, I sent https://go-review.googlesource.com/c/go/+/157457 earlier this month.

@iWdGo

This comment has been minimized.

Copy link
Contributor Author

commented Feb 9, 2019

Running the instructions using go 1.12 beta2 is indeed successful. On Windows, two things to know which might be written elsewhere:

  1. You have to run these instruction in CMD
  2. Directory %GOPATH%/bin must in in the PATH using f.i. set PATH=%PATH%;%GOPATH%/bin
@iWdGo

This comment has been minimized.

Copy link
Contributor Author

commented Feb 9, 2019

Similarly, using make.bat builds go tip into the golang repo bin directory.
The easiest is to move the executable like move %GOPATH%\golang\go\bin\go.exe %GOPATH%/bin/gotip.exe (on Windows). Then gotip test encoding/xml works as expected.

Maybe, make.bat should build directly into %GOPATH%/bin for consistency purposes.

@mvdan

This comment has been minimized.

Copy link
Member

commented Feb 9, 2019

No, the make scripts should not write the output binaries to GOPATH/bin. If that were the case, it would be impossible to build two versions of Go from source, as the go binaries would be installed in the same place.

Please don't use this issue to make other comments about buiding or testing Go; the issue is closed. If you have any questions or would like to share tips with other Go developers, see https://github.com/golang/go/wiki/Questions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.