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: go test quietly skips test if custom flag is defined #36527

Closed
jarifibrahim opened this issue Jan 13, 2020 · 2 comments
Closed

cmd/go: go test quietly skips test if custom flag is defined #36527

jarifibrahim opened this issue Jan 13, 2020 · 2 comments

Comments

@jarifibrahim
Copy link

@jarifibrahim jarifibrahim commented Jan 13, 2020

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

$ go version
go version go1.13 linux/amd64

Does this issue reproduce with the latest release?

I guess. I haven't tried.

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN="/home/ibrahim/Projects/go/bin"
GOCACHE="/home/ibrahim/.cache/go-build"
GOENV="/home/ibrahim/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/ibrahim/Projects/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
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-build158215380=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I have a package that has one subpackage. Let's call the top package as foo and sub-package bar. The directory structure looks like

.
├── bar
│   └── bar_test.go
└── main_test.go

1 directory, 2 files

main_test.go and bar_test.go both have one test each.

cat main_test.go
package main
...
var manual = flag.Int("manual", 0, "")
func TestFoo(t *testing.T) {}
cat bar_test.go
package bar
...
func TestHelloWorld(t *testing.T) {
	// t.Fatal("not implemented")
}

If I try to list all the tests via

go test -v -list= ./...                                                              
=== RUN   TestFoo
--- PASS: TestFoo (0.00s)
PASS
ok  	github.com/jarifibrahim/foo	0.001s
=== RUN   TestHelloWorld
--- PASS: TestHelloWorld (0.00s)
PASS
ok  	github.com/jarifibrahim/foo/bar	0.001s

which looks as expected but if I run

go test -v -manual=1 -list= ./...

it doesn't show the test TestHelloWorld

=== RUN   TestFoo
--- PASS: TestFoo (0.00s)
PASS
ok  	github.com/jarifibrahim/foo	0.001s

I understand the syntax of go test is go test [build/test flags] [packages] [build/test flags & test binary flags] but go test -manual=1 ... command doesn't crash with an error. It continues to run the test on the top level package only. It seems like go test assumed that there's only one package and ran the tests.

What did you expect to see?

I expect go test -manual=... packageName to return an error saying package manual (or whatever) is incorrect. It shouldn't quietly skip some tests.

What did you see instead?

go test -manual=0 ./... ran only the top level tests and go test ignored all the tests in the sub package since the top level package had a custom flag defined.

@jayconrod

This comment has been minimized.

Copy link
Contributor

@jayconrod jayconrod commented Jan 13, 2020

This is working as intended, though the behavior is a bit confusing.

-manual=1 is not a known build or test flag, and it can't be a package (starts with -), so -manual=1 and everything after that are passed in the list of arguments to each test executable. Since the list of packages is empty, go test tests the package in the current directory: go test and go test . are equivalent. That's consistent with other build commands.

I think the fix for this would mean reporting an error if an unrecognized flag is not preceded by explicit package arguments. That would be a breaking change, and commands like go test -manual=1 would not work. Personally, I use things like go test -u or go test -testwork all the time, so I expect many people would miss this. We don't interpret arguments passed to tests, so checking for arguments that look like packages at the end of the command line would not be good enough to differentiate the new behavior.

cc @bcmills who spent some time looking at test flags recently.

@jayconrod jayconrod closed this Jan 13, 2020
jarifibrahim added a commit to dgraph-io/badger that referenced this issue Jan 15, 2020
This could possibly be a bug in `go test` command
golang/go#36527 .

The `go test` command would skip tests in sub-packages if
the top-level package has a `custom flag` defined and the
sub-packages don't define it.

Issue golang/go#36527 (comment)
has an example of this.

This PR also removes the code from the test that would unnecessary
start a web server. I see two problems here
1. An unnecessary web server running.
2. We cannot run multiple tests are the same time since the second
    run of the test would try to start a web server and
    crash saying `port already in use`.
@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented Jan 22, 2020

Duplicate of #35097

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.