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" should fail if package does not build #7108

Closed
alexbrainman opened this issue Jan 12, 2014 · 9 comments
Closed

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

alexbrainman opened this issue Jan 12, 2014 · 9 comments
Milestone

Comments

@alexbrainman
Copy link
Member

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
Copy link
Contributor

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
Copy link
Member Author

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
Copy link
Contributor

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
Copy link
Member Author

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
Copy link
Contributor

rsc commented Apr 3, 2014

Comment 5:

Issue #7372 has been merged into this issue.

@rsc
Copy link
Contributor

rsc commented Apr 3, 2014

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
Copy link
Member Author

Comment 7:

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

Status changed to Started.

@gopherbot
Copy link

Comment 8:

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

@alexbrainman
Copy link
Member Author

Comment 9:

This issue was closed by revision c8f9097.

Status changed to Fixed.

@rsc rsc added this to the Go1.3 milestone Apr 14, 2015
@rsc rsc removed the release-go1.3 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants