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

testing: output hard to read when a test case is stalling #19397

Open
rakyll opened this Issue Mar 4, 2017 · 10 comments

Comments

Projects
None yet
7 participants
@rakyll
Member

rakyll commented Mar 4, 2017

When the package contains a stalling case, the verbose is hard to read to figure out what has run and not finished finished yet. I often have to pipe the output to another program to match RUNs and PASSes (or FAILs) to filter what has not finished.

We might just consider printing the currently running test cases upon SIGINT.

go test -v
=== RUN   TestTrace
=== RUN   TestTraceWithWait
--- PASS: TestTraceWithWait (0.01s)
=== RUN   TestTraceFromHeader
=== RUN   TestTraceFromHeaderWithWait
--- PASS: TestTraceFromHeaderWithWait (2.00s)
=== RUN   TestNewSpan
--- PASS: TestNewSpan (0.01s)
=== RUN   TestNoTrace
--- PASS: TestNoTrace (0.04s)
=== RUN   TestNoTraceWithWait
--- PASS: TestNoTraceWithWait (0.14s)
=== RUN   TestNoTraceFromHeader
--- PASS: TestNoTraceFromHeader (0.04s)
=== RUN   TestNoTraceFromHeaderWithWait
--- PASS: TestNoTraceFromHeaderWithWait (0.04s)
=== RUN   TestSample
--- PASS: TestSample (0.00s)
=== RUN   TestSampling
=== RUN   TestBundling
=== RUN   TestWeights
--- PASS: TestWeights (0.02s)
=== RUN   TestPropagation
--- PASS: TestPropagation (0.00s)
=== RUN   TestNoTraceIncoming
--- PASS: TestNoTraceIncoming (0.00s)
--- PASS: TestSampling (1.98s)
--- PASS: TestTrace (2.00s)
--- PASS: TestTraceFromHeader (2.00s)
^Csignal: interrupt
FAIL	cloud.google.com/go/trace	189.182s

@rakyll rakyll added this to the Go1.9 milestone Mar 4, 2017

@klingtnet

This comment has been minimized.

klingtnet commented Mar 7, 2017

I would like to have a command line flag like go test -only-failed which omits passed tests in the output.
This would make it very easy to spot failed test runs even for very large test suites and without the need of changing the output format.

This is already go tests default behavior.

@bradfitz

This comment has been minimized.

Member

bradfitz commented Mar 7, 2017

@klingtnet, we don't need a new flag because that is already the default behavior.

@klingtnet

This comment has been minimized.

klingtnet commented Mar 8, 2017

@bradfitz Sorry, I misunderstood the initial question which was about stalling not failing tests.
Anyways, I always run go test -v so I didn't knew that showing only failed tests is the default behavior which is luckily exactly what I want. Thanks for pointing this out.

@meirf

This comment has been minimized.

Member

meirf commented May 24, 2017

Some notes:

  • To be more specific on the use of the word "stalling": currently in practice this only applies to test cases running in Parallel. Parallel test cases aren't necessarily slow, but rather their executions are delayed ("stalled") such that all Parallel tests run in parallel. So even though the output shows "RUN" next to their test name, their executions and therefore terminations ("PASS", "FAIL") might happen after the next test's RUN, which makes the output hard to read. But even though in practice this illegibility might only occur for Parallel tests, in theory and for the implementation of the code change, the fact that this is only for Parallel tests should be ignored. To safeguard against changes to the Parallel implementation, we should just follow rakyll's advice to print any currently running tests on SIGINT.
  • b.Do(root) is where the test action tree is run. While this is running, need to make sure SIGINTs can be (1) captured and (2) acted on.
  • cmd.Start() is the start of each specific test. When this command is started a test should be considered in progress. Either when this command completes naturally or is stopped/killed externally, can consider it no longer running.

(I'd like to work on this - I realize we're in a freeze and that the code might not qualify for review since it's tooling-related and not an actual test.)

@gopherbot

This comment has been minimized.

gopherbot commented May 29, 2017

CL https://golang.org/cl/44352 mentions this issue.

@ianlancetaylor ianlancetaylor changed the title from cmd/test: output hard to read when a test case is stalling to testing: output hard to read when a test case is stalling Jun 1, 2017

@gopherbot gopherbot closed this in 11c61eb Jun 9, 2017

@bradfitz bradfitz reopened this Jul 13, 2017

@gopherbot

This comment has been minimized.

gopherbot commented Jul 13, 2017

CL https://golang.org/cl/48370 mentions this issue.

@bradfitz bradfitz modified the milestones: Go1.10, Go1.9 Jul 13, 2017

gopherbot pushed a commit that referenced this issue Jul 13, 2017

testing: roll back CL 44352 (show in-progress tests upon SIGINT)
CL 44352 changed the behavior of SIGINT, which can break tests that
themselves use SIGINT.  I think we can only implement this if the
testing package has a way to know whether the code under test is using
SIGINT, but os/signal does not provide an API for that.  Roll back for
1.9 and think about this again for 1.10.

Updates #19397

Change-Id: I021c314db2b9d0a80d0088b120a6ade685459990
Reviewed-on: https://go-review.googlesource.com/48370
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented Jul 13, 2017

I rolled back the first fix for this issue. It changed the behavior of SIGINT, which affects tests that use SIGINT. I think that a patch along those lines can only be used if there is a way to check whether the code under test is using SIGINT, but the os/signal package does not currently provide any API for that.

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented Jul 13, 2017

See #21000 for a test case.

@rsc

This comment has been minimized.

Contributor

rsc commented Nov 29, 2017

I'm not particularly worried about trying to fix this. I think it may actually be impossible - testing can't just take over SIGINT. But moved to unplanned in case anyone does see a way to fix it.

@rsc

This comment has been minimized.

Contributor

rsc commented Nov 29, 2017

Note that the new -json support will let people produce nice status dashboards that you pipe test output into, and maybe that will be enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment