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: change go test to FAIL if -run is specified but no tests match #64500

Closed
hyangah opened this issue Dec 1, 2023 · 10 comments
Closed
Assignees
Labels
GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Proposal
Milestone

Comments

@hyangah
Copy link
Contributor

hyangah commented Dec 1, 2023

Proposal Details

If -run is provided but there is no test matching the pattern, go test currently considers the test run PASS and exits with 0.

$ go test -run=Test_sum/1_+_1
testing: warning: no tests to run
PASS
ok      example/test    0.133s

Even though there is a warning like "testing: warning: no tests to run", this is easy to miss or dropped when the test binary runs by other runners (ide plugins, dlv, ..). I think when users set -run and if there is no matching test, that's likely not what users wanted, and treating it like misuse of command is more appropriate.

@dmitshur dmitshur added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go labels Dec 1, 2023
@dmitshur dmitshur added this to the Backlog milestone Dec 1, 2023
@dmitshur
Copy link
Contributor

dmitshur commented Dec 1, 2023

CC @bcmills, @matloob.

Slightly related, but in the opposite direction: go test and go vet currently print a warning and also exit with a fatal error when an import path pattern matches 0 packages (#31271, #52287), which can create overhead for a CI system wishing to permit modules that are empty or have packages for a subset of Go's ports, since they have no choice but need to convert such false-positive failures to successes on their own.

@bcmills bcmills changed the title cmd/go: change go test to FAIL if -run is specified but no tests match proposal: cmd/go: change go test to FAIL if -run is specified but no tests match Dec 1, 2023
@bcmills bcmills added this to Proposals Dec 1, 2023
@bcmills bcmills moved this to Incoming in Proposals Dec 1, 2023
@bcmills bcmills modified the milestones: Backlog, Proposal Dec 1, 2023
@bcmills
Copy link
Contributor

bcmills commented Dec 1, 2023

@navytux
Copy link
Contributor

navytux commented Jul 31, 2024

For the reference: I'm using go test -run ø -bench . to only run benchmarks without running tests. If -run ø is changed to result in FAIL, please add another way to only run the benchmarks. Thanks.

@dmitshur
Copy link
Contributor

@navytux That's a good point. In the original proposal here, there was only the -run flag specified, without -bench, so it likely shouldn't apply if something else, like benchmarks, does match and run.

As a side note, there is a possibility of that invocation matching some tests if they happen to have that character in their name. Since -run flag accepts a regular expression, consider doing go test -run=^$ -bench=. to ensure only benchmarks are run. The "^$" value is fairly explicit about the intention to match no tests/examples/fuzz tests.

@navytux
Copy link
Contributor

navytux commented Jul 31, 2024

@dmitshur, thanks for feedback. And I agree that ^$ is generally more strict and correct in limiting the set of allowed tests to run to be empty.

@golang golang deleted a comment from gopherbot Sep 4, 2024
@rsc
Copy link
Contributor

rsc commented Sep 4, 2024

Will add to proposal minutes next week, but I don't see how to do this in a defensible way.

  • It seems very strange to me that go test -run=XXX should fail but go test -run=XXX -bench=. should succeed.
  • It also seems strange to say that go test -run=XXX should fail but go test -run=^$ should succeed.
  • And what if I do go test -run=Foo -skip=Foo? Should that fail too?
  • And right now go test notests succeeds if notests has no tests (and compiles and passes vet). Should it start failing? If not, what if I use go test notests -run=.*? Should that start failing?

All these strange contradictions go away if go test -run=XXX continues to succeed as it does today but also continues to print a message about not having any tests to run.

The original design of this warning in #15211 explicitly left the exit code unchanged to avoid all these problems. I think we should continue to leave it unchanged.

@rsc
Copy link
Contributor

rsc commented Sep 11, 2024

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

@rsc rsc moved this from Incoming to Active in Proposals Sep 11, 2024
@aclements
Copy link
Member

Given the tricky contradictions in this proposal, I don't think we can do this, unfortunately. If someone finds a way to thread that needle, we can of course revisit. :)

@aclements
Copy link
Member

Based on the discussion above, this proposal seems like a likely decline.

@aclements aclements moved this from Active to Likely Decline in Proposals Oct 4, 2024
@aclements
Copy link
Member

No change in consensus, so declined.

@aclements aclements moved this from Likely Decline to Declined in Proposals Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Proposal
Projects
Status: Declined
Development

No branches or pull requests

7 participants