cmd/go: "go test" should fail if package does not build

[alexbrainman]

Steps to reproduce:

# cd $GOROOT/src/pkg/image/color/palette
# hg id
72f6ec05c6ca
# hg st
# echo "Hello world" >> palette.go
# go test; echo $?
?       image/color/palette     [no test files]
0

But "go build" fails:

# go build; echo $?
# image/color/palette
./palette.go:505: non-declaration statement outside function body
./palette.go:505: syntax error: unexpected name, expecting semicolon or newline
2

[davecheney]

Comment 1:

@alex, hmm, that is an interesting problem. I think at the moment the go tool skips
generating the test runner if there are no _test files. It sounds like you are
suggesting that the go tool should always build the test runner, or execute an
unconditional go build in each package.
There would probably have to be some more restrictions. ie, we shouldn't attempt to
build empty directories, or directories that contain no .go files.

Status changed to Accepted.

[alexbrainman]

Comment 2:

I discovered this when one of go.tools packages was broken on windows (could not be
built), but our windows builder was green. So I though it is worth considering. I think
"go build" should do, but others will know better.
Alex

[davecheney]

Comment 3:

@alex, i'm not sure if go test should do go build if there are no tests, or constructs a
test binary which prints the 'warning no tests run' message.
I'm worried that the rules for when to do these actions probably need some nuance to
avoid Go 1.3 rejecting larger packages that contain empty intermediary or parent
directories (I know Juju and its dependencies do this)
Offhand the rules
* no _test.go files in scope (build tags are considered to affect this), run go build or
a dummy go test.
* no *.go files at all, do nothing, present the [no tests found] message
Should be a good starting point, but I'm sure i've overlooked something.

Labels changed: added release-go1.3maybe.

[alexbrainman]

Comment 4:

I don't propose a solution, I just think we might have a problem. We have enough of wise
people. I will agree to anything (most likely :-)).
Alex

[rsc]

Comment 5:

Issue #7372 has been merged into this issue.

[rsc]

Comment 6:

We should do this.
diff -r 4ca10cc10176 src/cmd/go/test.go
--- a/src/cmd/go/test.go    Wed Apr 02 23:00:40 2014 -0400
+++ b/src/cmd/go/test.go    Thu Apr 03 14:24:50 2014 -0400
@@ -524,7 +524,7 @@
 
 func (b *builder) test(p *Package) (buildAction, runAction, printAction *action, err error) {
    if len(p.TestGoFiles)+len(p.XTestGoFiles) == 0 {
-       build := &action{p: p}
+       build := b.action(modeBuild, modeBuild, p)
        run := &action{p: p, deps: []*action{build}}
        print := &action{f: (*builder).notest, p: p, deps: []*action{run}}
        return build, run, print, nil
If someone wants to make that a CL with a test in test.bash, that'd be great.

Labels changed: added release-go1.3, removed release-go1.3maybe.

[alexbrainman]

Comment 7:

https://golang.org/cl/84300043/

Status changed to Started.

[gopherbot]

Comment 8:

CL https://golang.org/cl/84300043 references this issue.

[alexbrainman]

Comment 9:

This issue was closed by revision c8f9097.

Status changed to Fixed.