What version of Go are you using (go version
)?
go 1.15.8
What did you do?
I resturctured my test TestXXX as an example ExampleXXX. But forgot to remove the argument.
What did you expect to see?
ExampleXXX(t testing.T) is wrong signature for an example (has testing.T as an argument),
so it should raise error on go test
.
What did you see instead?
It didn't run. Made me assumes the test has successfully finished.
Comment From: D1CED
Is at least correctly reported on the playground https://play.golang.org/p/dj_FUdOxN1j
Comment From: ianlancetaylor
The problem is reported by go vet
.
Comment From: kybin
@D1CED Ah, I thought it isn't relevant to playground, my guess was wrong.
@ianlancetaylor I keep forget about go vet
, thanks. Doesn't go test
do basic vetting? I feel that I read it somewhere...
Comment From: ianlancetaylor
Yes, go test
runs go vet
by default. But it only runs it with a limited set of vet tests, ones that are considered to be very reliable when it comes to showing problems. Perhaps we should add this check to that list.
I'll change this issue into a proposal to do that.
Comment From: kybin
Great, thank you.
Comment From: adonovan
Perusing the source of the tests
analyzer, it appears to be in effect a type checker, and 100% precise, so I have no objection to adding this to the 'go test' suite. I'll send a CL.
Comment From: gopherbot
Change https://go.dev/cl/529816 mentions this issue: cmd/go/internal/test: add 'tests' vet check to 'go test' suite
Comment From: rsc
@adonovan to run ecosystem metrics pipeline analysis to make sure there aren't false positives in the broader Go ecosystem.
Comment From: 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
Comment From: adonovan
This text file shows diagnostics found by running the tests
analyzer across a sample of 1056 modules in the Go module proxy's corpus. Of those modues, 38 had diagnostics, a total of 242. Of those diagnostics, 96% (233) are of the form "ExampleXYZ refers to unknown identifier: XYZ". I inspected a couple dozen diagnostics and they all looked sound.
The first 10 "Example" diagnostics :
$ grep Example tests.txt | head
https://go-mod-viewer.appspot.com/github.com/olekukonko/tablewriter@v0.0.5/table_test.go#L57: ExampleLong refers to unknown identifier: Long https://go-mod-viewer.appspot.com/github.com/olekukonko/tablewriter@v0.0.5/table_test.go#L84: ExampleCSV refers to unknown identifier: CSV https://go-mod-viewer.appspot.com/github.com/gonum/matrix@v0.0.0-20181209220409-c518dec07be9/mat64/cholesky_example_test.go#L72: ExampleCholeskySymRankOne refers to unknown identifier: CholeskySymRankOne https://go-mod-viewer.appspot.com/github.com/robertkrimen/otto@v0.2.1/documentation_test.go#L7: ExampleSynopsis refers to unknown identifier: Synopsis https://go-mod-viewer.appspot.com/github.com/robertkrimen/otto@v0.2.1/documentation_test.go#L72: ExampleConsole refers to unknown identifier: Console https://go-mod-viewer.appspot.com/github.com/cosmos/cosmos-sdk@v0.50.1/crypto/hd/hdpath_test.go#L188: ExampleStringifyPathParams refers to unknown identifier: StringifyPathParams https://go-mod-viewer.appspot.com/github.com/cosmos/cosmos-sdk@v0.50.1/crypto/hd/hdpath_test.go#L198: ExampleSomeBIP32TestVecs refers to unknown identifier: SomeBIP32TestVecs https://go-mod-viewer.appspot.com/github.com/cosmos/cosmos-sdk@v0.50.1/crypto/ledger/encode_test.go#L27: ExamplePrintRegisteredTypes refers to unknown identifier: PrintRegisteredTypes https://go-mod-viewer.appspot.com/github.com/biogo/store@v0.0.0-20201120204734-aad293a2328f/interval/int_interval_example_test.go#L41: Example_2 has malformed example suffix: 2 https://go-mod-viewer.appspot.com/github.com/biogo/store@v0.0.0-20201120204734-aad293a2328f/interval/interval_example_test.go#L67: Example_1 has malformed example suffix: 1
The 9 non-Example ones:
$ grep -v Example tests.txt
https://go-mod-viewer.appspot.com/github.com/cihub/seelog@v0.0.0-20170130134532-f561c5e57575/common_constraints_test.go#L63: TestlistConstraintsWithDuplicates has malformed name: first letter after 'Test' must not be lowercase https://go-mod-viewer.appspot.com/github.com/cihub/seelog@v0.0.0-20170130134532-f561c5e57575/common_constraints_test.go#L88: TestlistConstraintsWithOffInList has malformed name: first letter after 'Test' must not be lowercase https://go-mod-viewer.appspot.com/github.com/cihub/seelog@v0.0.0-20170130134532-f561c5e57575/dispatch_filterdispatcher_test.go#L31: TestfilterDispatcher_Pass has malformed name: first letter after 'Test' must not be lowercase https://go-mod-viewer.appspot.com/github.com/cihub/seelog@v0.0.0-20170130134532-f561c5e57575/dispatch_filterdispatcher_test.go#L51: TestfilterDispatcher_Deny has malformed name: first letter after 'Test' must not be lowercase https://go-mod-viewer.appspot.com/github.com/cihub/seelog@v0.0.0-20170130134532-f561c5e57575/dispatch_splitdispatcher_test.go#L42: TestsplitDispatcher has malformed name: first letter after 'Test' must not be lowercase https://go-mod-viewer.appspot.com/github.com/cihub/seelog@v0.0.0-20170130134532-f561c5e57575/writers_formattedwriter_test.go#L31: TestformattedWriter has malformed name: first letter after 'Test' must not be lowercase https://go-mod-viewer.appspot.com/github.com/alexflint/go-arg@v1.4.3/example_test.go#L203: output comment block must be the last comment block https://go-mod-viewer.appspot.com/github.com/justinas/nosurf@v1.1.1/utils_test.go#L8: TestsContains has malformed name: first letter after 'Test' must not be lowercase https://go-mod-viewer.appspot.com/github.com/justinas/nosurf@v1.1.1/utils_test.go#L22: TestsameOrigin has malformed name: first letter after 'Test' must not be lowercase
Comment From: rsc
Very nice!
Are there any remaining concerns about this proposal?
Comment From: kybin
It's great to see my small, old issue has been changed to a proposal and has come this far.
I don't have any complaints. If you asked to me ;)
Thanks! @adonovan @rsc
Comment From: rsc
Based on the discussion above, this proposal seems like a likely accept. — rsc for the proposal review group
The proposal is to run ‘go vet -tests’ automatically during ‘go test’, with the many other vet checks that run then.
Comment From: rsc
No change in consensus, so accepted. 🎉 This issue now tracks the work of implementing the proposal. — rsc for the proposal review group
The proposal is to run ‘go vet -tests’ automatically during ‘go test’, with the many other vet checks that run then.
Comment From: dmitshur
Reopening since CL 529816 was rolled back in CL 571695, and a roll forward hasn't been submitted.
Comment From: gopherbot
Change https://go.dev/cl/603476 mentions this issue: cmd/go/internal/test: add 'tests' vet check to 'go test' suite