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

Jump to bottom

cmd/go: go test should not report two compilation failures #26125

Closed
rsc opened this issue Jun 29, 2018 · 7 comments
Closed

cmd/go: go test should not report two compilation failures #26125

rsc opened this issue Jun 29, 2018 · 7 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone
Go1.11

Comments

@rsc
Copy link
Contributor

rsc commented Jun 29, 2018 

$ cat x_test.go
package p

func f() {
	return 1
}
$ go test x_test.go
# command-line-arguments [command-line-arguments.test]
./x_test.go:4:2: too many arguments to return
	have (number)
	want ()
# command-line-arguments
./x_test.go:4:9: no result values expected
vet: typecheck failures
FAIL	command-line-arguments [build failed]
$

The compiler is failing and the vet is failing. It's annoying (or worse) to print the same errors twice, in slightly different forms.
Go 1.10 did the right thing (perhaps because it ignored vet typecheck errors entirely in go test).
Should fix for Go 1.11.
We may need to start ignoring typecheck errors for one more release anyway, in which case we'd get this for free, but we should put in a real fix too, and a test, or else this will happen again in Go 1.12 or whenever we do re-enable typecheck errors from go vet during go test.
@rsc rsc added this to the Go1.11 milestone Jun 29, 2018
@jimmyfrasche
Copy link
Member

Duplicate of #26100 ?

@robpike robpike mentioned this issue Jun 29, 2018
Closed
@robpike
Copy link
Contributor

robpike commented Jun 29, 2018

It is a duplicate, but this one has an example. I'll close that one.

@robpike
Copy link
Contributor

robpike commented Jun 29, 2018

My suggestion from 26100 is just to suppress errors from vet until compilation has succeeded.

@ysmolski
Copy link
Member

Wanted behaviour of vet in the compiler is similar to the way vet is implemented in Playground. There vet check is performed only if the compilation has succeeded. I see this as a good UI.

Playground with vet check is not deployed, thought. @andybons, have you planned the deployment of Playground?

@andybons
Copy link
Member

@ysmolsky let’s discuss playground deployment off this thread :). (short answer: no, but only because I just haven’t gotten to it).

@myitcv myitcv mentioned this issue Jul 1, 2018
Closed
@agnivade agnivade mentioned this issue Jul 5, 2018
Closed
This was referenced Jul 5, 2018
Merged
Merged
@bradfitz bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. label Jul 12, 2018
@ysmolski
Copy link
Member

Anyone working on this?

@gopherbot
Copy link

Change https://golang.org/cl/123938 mentions this issue: cmd/go: skip vet check when the compiler has failed

@golang golang locked and limited conversation to collaborators Jul 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Milestone
Go1.11
Development

No branches or pull requests
7 participants
@bradfitz @rsc @ysmolski @andybons @jimmyfrasche @robpike @gopherbot