proposal: cmd/go: make "go test" exit 0 if *.go files exist but none match context

[bradfitz]

It's a little weird right now that go test in a directory of non-context-matching *.go files (like golang.org/x/sys/windows/registry on Linux) results in an error, but go test in a directory without tests is fine (exit 0), and go test ./... is also fine, as long as one directory somewhere has a context-matching file.

dev:~ $ mkdir /tmp/foo
dev:~ $ cd /tmp/foo
dev:foo $ go mod init foo
go: creating new go.mod: module foo

dev:foo $ go test
no Go files in /tmp/foo
dev:foo $ echo $?
1

dev:foo $ cat > x_windows.go
package main

dev:foo $ go test
package foo: build constraints exclude all Go files in /tmp/foo
dev:foo $ echo $?
1

dev:foo $ cat > x.go
package main

dev:foo $ go test
?       foo     [no test files]
dev:foo $ echo $?
0

dev:foo $ go test ./...
go: warning: "./..." matched no packages
no packages to test

dev:foo $ mkdir bar
dev:foo $ cat > bar/bar.go
package bar

dev:foo $ go test ./...
?       foo/bar [no test files]
dev:foo $ echo $?
0

dev:foo $ go test golang.org/x/sys/windows/registry
go: finding module for package golang.org/x/sys/windows/registry
go: downloading golang.org/x/sys v0.0.0-20200602225109-6fdc65e7d980
go: found golang.org/x/sys/windows/registry in golang.org/x/sys v0.0.0-20200602225109-6fdc65e7d980
package golang.org/x/sys/windows/registry: build constraints exclude all Go files in /home/bradfitz/pkg/mod/golang.org/x/sys@v0.0.0-20200602225109-6fdc65e7d980/windows/registry

Can we instead make go test act more like the package has no tests? I'd like to see:

$ go test golang.org/x/sys/windows/registry
?       golang.org/x/sys/windows/registry [no matching files]
$ echo $?
0

This arose due to integration of Go in another build system (that builds non-Go stuff) and that build system was using the heuristic that if a directory contains *_test.go files, it running go test in that directory was reasonable. That's currently not true for directories like x/sys/windows/registry or other such OS-specific packages.

[rsc]

It's unclear what the change would be here exactly.

Files that exist but don't match context are treated the same as if they didn't exist at all.

go test without an argument is identical to go test ., and when you say a specific package like . or math (not a wildcard), it is required to exist.

Currently go test . says

package foo: build constraints exclude all Go files in /tmp/foo

and that is consistent with go build ., go install ., go doc ., go run ..

It seems like all of those four really need to fail instead of just printing a warning.

So then the question is whether go test should be different from those, and if so, how to draw the line.

We may have backed ourselves into a bit of a puzzle where there's no easy way out.

[rsc]

Adding to minutes; I still don't see an easy way out here.

[rsc]

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

[rsc]

/cc @bcmills @jayconrod @matloob

[jayconrod]

The current go test behavior seems more consistent with other commands.

Might be good to know about the other build system and whether it can avoid running go test in directories like this?

As a point of comparison, Bazel will generally have go_library and go_test targets for each package. Build constraints are applied at execution time, right before invoking the compiler. If all source files are excluded, Bazel just compiles a file with package empty.

[bcmills]

IMO the cleaner solution here is to set up the .go source files for the package in such a way that OS-specific packages continue to exist (and continue to compile) on platforms that do not support the OS-specific functionality.

For the x/sys/windows/registry package, that might mean that on an unsupported platform the CreateKey, OpenKey, etc. functions that can return an error return an invariantly non-nil one (such as one wrapping errors.ErrUnsupported; see #41198).

Then go test could correctly report that all matching tests pass (if there are any OS-agnostic tests for the package), and could at the very least report any go vet errors on the OS-agnostic parts of the API.

[rsc]

IMO the cleaner solution here is to set up the .go source files for the package in such a way that OS-specific packages continue to exist (and continue to compile) on platforms that do not support the OS-specific functionality.

FWIW that's just echo package p >dummy.go.

[rsc]

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

[rsc]

No change in consensus, so declined.
— rsc for the proposal review group