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

proposal: cmd/go: add -tests to list of vet checks run by "go test" #44251

Open
kybin opened this issue Feb 13, 2021 · 5 comments
Open

proposal: cmd/go: add -tests to list of vet checks run by "go test" #44251

kybin opened this issue Feb 13, 2021 · 5 comments
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) Proposal
Milestone

Comments

@kybin
Copy link
Contributor

kybin commented Feb 13, 2021

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.

@seankhliao seankhliao changed the title ExampleXXX(t *testing.T) test sliently discarded, instead of raising error testing: ExampleXXX(t *testing.T) test sliently discarded, instead of raising error Feb 13, 2021
@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 13, 2021
@D1CED
Copy link

D1CED commented Feb 13, 2021

Is at least correctly reported on the playground https://play.golang.org/p/dj_FUdOxN1j

@ianlancetaylor
Copy link
Contributor

The problem is reported by go vet.

@kybin
Copy link
Contributor Author

kybin commented Feb 14, 2021

@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...

@ianlancetaylor
Copy link
Contributor

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.

@ianlancetaylor ianlancetaylor changed the title testing: ExampleXXX(t *testing.T) test sliently discarded, instead of raising error proposal: cmd/go: add -tests to list of vet checks run by "go test" Feb 14, 2021
@gopherbot gopherbot added this to the Proposal milestone Feb 14, 2021
@ianlancetaylor ianlancetaylor removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 14, 2021
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Feb 14, 2021
@kybin
Copy link
Contributor Author

kybin commented Feb 15, 2021

Great, thank you.

@timothy-king timothy-king added the Analysis Issues related to static analysis (vet, x/tools/go/analysis) label Mar 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) Proposal
Projects
Status: Incoming
Development

No branches or pull requests

6 participants