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: testing: flag show skipped #34306

Closed
mvndaai opened this issue Sep 15, 2019 · 11 comments
Closed

proposal: testing: flag show skipped #34306

mvndaai opened this issue Sep 15, 2019 · 11 comments
Labels
Projects
Milestone

Comments

@mvndaai
Copy link

@mvndaai mvndaai commented Sep 15, 2019

I would love if there was a flag for go test that was somewhere between normal and verbose that only showed failures and skips.

go test ./... hides that tests were skipped

go test -v ./... show too much so I miss things

It would be lovely to have something like go test -showSkipped ./... that still shows things like

=== RUN   TestMarkReviewed
--- SKIP: TestMarkReviewed (3.52s)
    flagged_test.go:133: Skipping integration test because a mongo db is not available]

but ignores

=== RUN   TestFlaggedThread
--- PASS: TestFlaggedThread (0.00s)
@gopherbot gopherbot added this to the Proposal milestone Sep 15, 2019
@gopherbot gopherbot added the Proposal label Sep 15, 2019
@mvdan
Copy link
Member

@mvdan mvdan commented Sep 15, 2019

While I agree that this is generally useful, this can be accomplished with little effort today:

$ go test -v ./... | grep -E '--- (SKIP|FAIL)'

If you need anything fancier, you could also write a short wrapper around go test -json ./.... It could only keep results that are either SKIP or FAIL, for example. Or it could even error if anything was skipped, in a "don't allow skips" mode.

@bcmills
Copy link
Member

@bcmills bcmills commented Sep 17, 2019

See also #25951.

@rsc rsc added this to Incoming in Proposals Dec 4, 2019
@rsc
Copy link
Contributor

@rsc rsc commented Jan 8, 2020

This would seem like taking the 'go test pkg1 pkg2' behavior, where it silences the output from pkg1 if pkg1 succeeds but shows pkg2 if pkg2 fails, down to the individual test case level. That seems fine given a good name.

@rsc rsc moved this from Incoming to Active in Proposals Jan 8, 2020
@rsc
Copy link
Contributor

@rsc rsc commented Jan 15, 2020

To summarize the state of the world:

  • go test shows only failing test results
  • go test -v shows all test results - fails, skips, and passes.

This issue is asking for a way to see failing test results and skips, but not passes.

I mistakenly thought last week that there was not a mode that shows only failing test results. That's the default mode. That mode should very likely not show skips, since if all you have are passes and skips, that's a successful run.

Given that we already have a "only failing test results" mode, I am not sure I see the strong rationale for a "only fails and skips" mode. Especially since it can be reconstructed with go test -json and a filter, or just with grep. How important this is clearly depends on exactly how much you care about skips. If you know about the skip and it's supposed to be there, printing it is just noise.

@mvndaai
Copy link
Author

@mvndaai mvndaai commented Jan 15, 2020

I argue that go test should show failing and skipped by default instead of just failing. Since a skipped test isn't actually a pass.

I would probably use grep for this, but I would want the t.Log statements to still show around failing tests which makes that impossible.

I could probably go down the path of the -json, I just haven't done that yet because of the complexity of joining the RUN and the final state entries and other things I haven't considered.

@rsc
Copy link
Contributor

@rsc rsc commented Jan 22, 2020

A skipped test isn't actually a pass but also not actually a fail.
Usually it is a known fail that you don't want to see. That's why you skip it.
If you do want to see it, don't mark it skipped.
We can differ about the defaults here but they were chosen years ago and are now set.

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

@rsc rsc moved this from Active to Likely Decline in Proposals Jan 22, 2020
@mvndaai
Copy link
Author

@mvndaai mvndaai commented Jan 23, 2020

I disagree with

Usually it is a known fail that you don't want to see. That's why you skip it.

In my experience, it is usually something that either cannot be tested in a specific environment, like CI or a test that needs to be updated that someone skipped in order to get through the CI until the work can be done.

I want my tech debt visible.

@mvdan
Copy link
Member

@mvdan mvdan commented Jan 23, 2020

Skips aren't generally tech debt, though. For example, in some of the portable projects I maintain, there are lots of tests that can only run on Windows, while others can only run on Linux or Unix-y systems. It's literally impossible to run all the tests at once without skips, as far as I can tell.

Showing skips similar to failures, or treating them like tech debt, would in my opinion cause pain for no reason in cases like that.

I want my tech debt visible.

If you indeed have tech debt, I'd encourage you to look at #25951.

@cespare
Copy link
Contributor

@cespare cespare commented Jan 23, 2020

Skips aren't generally tech debt, though. For example, in some of the portable projects I maintain, there are lots of tests that can only run on Windows, while others can only run on Linux or Unix-y systems. It's literally impossible to run all the tests at once without skips, as far as I can tell.

Can't you use build tags (or filename suffixes) to only include platform-specific tests on the appropriate platforms?

@mvdan
Copy link
Member

@mvdan mvdan commented Jan 23, 2020

Sure, I could. But skips on runtime.GOOS are perfectly fine too. That was just one example that's pretty common; another one is when large/complex dependencies aren't installed or running.

I'm still not sure why a flag here is explicitly needed, though. I use commands like go test -v ./... | grep SKIP all the time, to see if any skips are misbehaving. #25951 would help here in the long run, but a flag to save a grep line wouldn't, to my mind.

@rsc
Copy link
Contributor

@rsc rsc commented Feb 5, 2020

No change in consensus, so declining.

@rsc rsc closed this Feb 5, 2020
@rsc rsc moved this from Likely Decline to Declined in Proposals Feb 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Proposals
Declined
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants