-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
cmd/internal/test2json: test timeout panic trace is attributed to the running test, even if the test just passed #57305
Comments
I think the attribution may be correct with the test runner still processing the first test, but there's a race between PASS and timeout? |
Yeah, there's a race between printing PASS and the test actually exiting which affects the timeout result. But re: attribution, I would argue that when using |
@bcmills per owners. |
See previously #35180. |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
I ran
go test
with json output and a timeout which occurs right as one test is passing.With the following test file:
Running
go test . -json -timeout=1s
reproduces the problem sporadically. The behavior is racy. Sometimes the test timeout panic trace is attributed to a test, other times not. And occasionally it's attributed to a test that passed.What did you expect to see?
I wanted to see the test timeout panic output not attributed to any particular test (no
{"Test":"TestHello"}
) since there could be multiple tests running.go test . -json -timeout=1s
OutputWhen adding
t.Parallel()
to both tests:go test . -json -timeout=1s
Output (parallel)Note that I'm unsure if removing
{"Test":"TestWorld"}
would be problematic in this case, or break any existing tools, but it doesn't intuitively make sense to me that any one test would be attributed this output.What did you see instead?
I saw two things:
PASS
but the stack trace is attributed to it after the fact ({"Test":"TestHello"}
)PASS
but Go 1.20rc1 considers it to be running still.The first case is problematic since tools like
gotestsum
rely onPASS
to ignore future output from a test. The second is more of an inconvenience, however, if tools relied on the output to distinguish problematic tests, the statistics would be skewed.go test . -json -timeout=1s
OutputWhen adding
t.Parallel()
to both tests:go test . -json -timeout=1s
Output (parallel)The text was updated successfully, but these errors were encountered: