Skip to content

cmd/go: "go test -notrealflag ./..." fails silently if the current directory has no tests #35097

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.

Sign up for GitHub

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

Jump to bottom
Closed
mstoykov opened this issue Oct 23, 2019 · 6 comments
Closed

cmd/go: "go test -notrealflag ./..." fails silently if the current directory has no tests #35097

mstoykov opened this issue Oct 23, 2019 · 6 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.

Comments

@mstoykov
Copy link

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

$ go version
go version go1.13.3 linux/amd64

Does this issue reproduce with the latest release?

yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/mstoykov/.cache/go-build"
GOENV="/home/mstoykov/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/mstoykov/.gvm/pkgsets/go1.13.3/global"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/mstoykov/.gvm/gos/go1.13.3"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/mstoykov/.gvm/gos/go1.13.3/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-build162107444=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Running go test -notrealflag ./... in a directory with no tests doesn't not report that there is no such flag.
In the real case I hit I was mistakenly providing -fastfail instead of -failfast and this did not run any tests and gave me no reason why it didn't.

There is an error if the current directory has tests.

What did you expect to see?

An error and the help of go test

What did you see instead?

$ go test -notrealflag ./...
?       github.com/loadimpact/k6        [no test files]
$ echo $?
0
@bcmills
Copy link
Contributor

bcmills commented Oct 23, 2019

Per https://golang.org/cmd/go/#hdr-Testing_flags:

The 'go test' command takes both flags that apply to 'go test' itself and flags that apply to the resulting test binary.

Since the pattern did not match any test binaries, no such binaries were run. However, you could just as easily run go test -notrealflag on a package that actually does define -notrealflag, and that would not be an error.

Since go test did explicitly tell you that there were [no test files] in this case, I don't think there is anything more to be done here: it would be inconsistent (and oddly specific) to reject additional flags only when go test has no other actual work to do.

@bcmills bcmills closed this as completed Oct 23, 2019
@mstoykov
Copy link
Author

@bcmills if you run it without the flag there are a tons of sub packages that will be tested. And if that same command is ran inside a directory which there are tests it will actually correctly report that this flag is not there

$ go test -count 0 ./...
?       github.com/loadimpact/k6        [no test files]
ok      github.com/loadimpact/k6/api    0.038s [no tests to run]
?       github.com/loadimpact/k6/api/common     [no test files]
ok      github.com/loadimpact/k6/api/v1 0.029s [no tests to run]
?       github.com/loadimpact/k6/api/v1/client  [no test files]
ok      github.com/loadimpact/k6/api/v1 0.029s [no tests to run]                                                                                                                                           [47/2129]
?       github.com/loadimpact/k6/api/v1/client  [no test files]
ok      github.com/loadimpact/k6/cmd    0.033s [no tests to run]
ok      github.com/loadimpact/k6/converter/har  0.020s [no tests to run]
ok      github.com/loadimpact/k6/core   0.013s [no tests to run]
ok      github.com/loadimpact/k6/core/local     0.015s [no tests to run]
ok      github.com/loadimpact/k6/js     0.013s [no tests to run]
ok      github.com/loadimpact/k6/js/common      0.006s [no tests to run]
ok      github.com/loadimpact/k6/js/compiler    0.012s [no tests to run]
?       github.com/loadimpact/k6/js/lib [no test files]
?       github.com/loadimpact/k6/js/modules     [no test files]
ok      github.com/loadimpact/k6/js/modules/k6  0.032s [no tests to run]
ok      github.com/loadimpact/k6/js/modules/k6/crypto   0.010s [no tests to run]
ok      github.com/loadimpact/k6/js/modules/k6/crypto/x509      0.016s [no tests to run]
ok      github.com/loadimpact/k6/js/modules/k6/encoding 0.020s [no tests to run]
ok      github.com/loadimpact/k6/js/modules/k6/html     0.041s [no tests to run]
?       github.com/loadimpact/k6/js/modules/k6/html/gen [no test files]
ok      github.com/loadimpact/k6/js/modules/k6/http     0.009s [no tests to run]
ok      github.com/loadimpact/k6/js/modules/k6/metrics  0.034s [no tests to run]
ok      github.com/loadimpact/k6/js/modules/k6/ws       0.018s [no tests to run]
ok      github.com/loadimpact/k6/lib    0.023s [no tests to run]
?       github.com/loadimpact/k6/lib/consts     [no test files]
ok      github.com/loadimpact/k6/lib/fsext      0.009s [no tests to run]
?       github.com/loadimpact/k6/lib/metrics    [no test files]
?       github.com/loadimpact/k6/lib/netext     [no test files]
ok      github.com/loadimpact/k6/lib/netext/httpext     0.005s [no tests to run]
ok      github.com/loadimpact/k6/lib/scheduler  0.021s [no tests to run]
?       github.com/loadimpact/k6/lib/testutils  [no test files]
?       github.com/loadimpact/k6/lib/testutils/httpmultibin     [no test files]
ok      github.com/loadimpact/k6/lib/types      0.003s [no tests to run]
ok      github.com/loadimpact/k6/loader 0.005s [no tests to run]
ok      github.com/loadimpact/k6/stats  0.015s [no tests to run]
ok      github.com/loadimpact/k6/stats/cloud    0.005s [no tests to run]
ok      github.com/loadimpact/k6/stats/csv      0.032s [no tests to run]
ok      github.com/loadimpact/k6/stats/datadog  0.037s [no tests to run]
ok      github.com/loadimpact/k6/stats/dummy    0.005s [no tests to run]
ok      github.com/loadimpact/k6/stats/influxdb 0.031s [no tests to run]
ok      github.com/loadimpact/k6/stats/json     0.027s [no tests to run]
ok      github.com/loadimpact/k6/stats/kafka    0.048s [no tests to run]
ok      github.com/loadimpact/k6/stats/statsd   0.005s [no tests to run]
ok      github.com/loadimpact/k6/stats/statsd/common    0.005s [no tests to run]
?       github.com/loadimpact/k6/stats/statsd/common/testutil   [no test files]
ok      github.com/loadimpact/k6/ui     0.004s [no tests to run]
[mstoykov@himinbjorg k6]$ cd js
[mstoykov@himinbjorg js]$ go test -notrealflag ./...
flag provided but not defined: -notrealflag
Usage of /tmp/go-build525921648/b001/js.test:
  -test.bench regexp
        run only benchmarks matching regexp
... more test flags

@bcmills
Copy link
Contributor

bcmills commented Oct 23, 2019

The flag provided but not defined error that you see there comes from the binary under test, not the go test command itself.

Compare:

~/go/src/testing$ go test -c testing

~/go/src/testing$ ./testing.test -notrealflag
flag provided but not defined: -notrealflag
Usage of ./testing.test:
  -test.bench regexp
        run only benchmarks matching regexp
[…]

@mstoykov
Copy link
Author

The problem is not where it comes from, the problem is that in the one case the tests are not ran for any of the other directories which leads to the user having no idea why test haven't ran ...

@bcmills bcmills mentioned this issue Jan 22, 2020
Closed
@bcmills bcmills reopened this Jan 22, 2020
@bcmills bcmills added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jan 22, 2020
@bcmills
Copy link
Contributor

bcmills commented Jan 22, 2020

#36527 reported a similar issue, although with an actual test file in the top-level directory (so there actually was a test to pass the flag to), and that test file actually did define the flag in question.

@bcmills
Copy link
Contributor

bcmills commented Jan 22, 2020

Thinking about this some more, I still don't think there's anything we can do about it without breaking existing use-cases. Any special case to reject unknown flags if there is no work at all would still fail for cases like #36527 where there actually is work to do.

It seems like the only robust way to ensure that the user's intended package argument is actually parsed as such is to require a package argument in all cases, but lots of users today have already learned to run commands like go test -v with the implicit . package argument. We shouldn't break the workflows of those users without extremely compelling evidence, and so far we have only two reports of issues.

CC @jayconrod @matloob

@bcmills bcmills closed this as completed Jan 22, 2020
@golang golang locked and limited conversation to collaborators Jan 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mstoykov @bcmills @gopherbot