cmd/test2json: tests that panic are marked as passing #40132

zolotov · 2020-07-09T10:09:42Z

What version of Go are you using (`go version`)?

$ go version
go version go1.15beta1 darwin/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (`go env`)?

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/zolotov/Library/Caches/go-build"
GOENV="/Users/zolotov/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/zolotov/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/zolotov/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/zolotov/go/go1.15beta1"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/zolotov/go/go1.15beta1/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/pg/md95lhn52rj2x8jtfdr_d1cc0000gp/T/go-build033989892=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Compile test with go test -c

package main

import (
	"testing"
)

func TestColor(t *testing.T) {
	panic(123)
}

run it with test2json tool
get the output like that

/Users/zolotov/go/go1.15beta1/bin/go tool test2json -t /private/var/folders/pg/md95lhn52rj2x8jtfdr_d1cc0000gp/T/___1TestColor_in_awesomeProject21                                       
{"Time":"2020-07-06T23:35:36.37331+03:00","Action":"output","Test":"TestColor","Output":"--- FAIL: TestColor (0.00s)\n"}
{"Time":"2020-07-06T23:35:36.375884+03:00","Action":"output","Test":"TestColor","Output":"panic: 123 [recovered]\n"}
{"Time":"2020-07-06T23:35:36.375897+03:00","Action":"output","Test":"TestColor","Output":"\tpanic: 123\n"}
{"Time":"2020-07-06T23:35:36.375906+03:00","Action":"output","Test":"TestColor","Output":"\n"}
{"Time":"2020-07-06T23:35:36.375911+03:00","Action":"output","Test":"TestColor","Output":"goroutine 21 [running]:\n"}
{"Time":"2020-07-06T23:35:36.375917+03:00","Action":"output","Test":"TestColor","Output":"testing.tRunner.func1.1(0x411d400, 0x416e6c8)\n"}
{"Time":"2020-07-06T23:35:36.375923+03:00","Action":"output","Test":"TestColor","Output":"\t/Users/zolotov/go/go1.15beta1/src/testing/testing.go:1057 +0x30d\n"}
{"Time":"2020-07-06T23:35:36.375934+03:00","Action":"output","Test":"TestColor","Output":"testing.tRunner.func1(0xc000082900)\n"}
{"Time":"2020-07-06T23:35:36.375938+03:00","Action":"output","Test":"TestColor","Output":"\t/Users/zolotov/go/go1.15beta1/src/testing/testing.go:1060 +0x41a\n"}
{"Time":"2020-07-06T23:35:36.375943+03:00","Action":"output","Test":"TestColor","Output":"panic(0x411d400, 0x416e6c8)\n"}
{"Time":"2020-07-06T23:35:36.375947+03:00","Action":"output","Test":"TestColor","Output":"\t/Users/zolotov/go/go1.15beta1/src/runtime/panic.go:969 +0x175\n"}
{"Time":"2020-07-06T23:35:36.375952+03:00","Action":"output","Test":"TestColor","Output":"awesomeProject21.TestColor(0xc000082900)\n"}
{"Time":"2020-07-06T23:35:36.376078+03:00","Action":"output","Test":"TestColor","Output":"\t/Users/zolotov/go/src/awesomeProject21/go_test.go:9 +0x39\n"}
{"Time":"2020-07-06T23:35:36.376086+03:00","Action":"output","Test":"TestColor","Output":"testing.tRunner(0xc000082900, 0x414f870)\n"}
{"Time":"2020-07-06T23:35:36.376089+03:00","Action":"output","Test":"TestColor","Output":"\t/Users/zolotov/go/go1.15beta1/src/testing/testing.go:1108 +0xef\n"}
{"Time":"2020-07-06T23:35:36.376092+03:00","Action":"output","Test":"TestColor","Output":"created by testing.(*T).Run\n"}
{"Time":"2020-07-06T23:35:36.376095+03:00","Action":"output","Test":"TestColor","Output":"\t/Users/zolotov/go/go1.15beta1/src/testing/testing.go:1159 +0x386\n"}
{"Time":"2020-07-06T23:35:36.376305+03:00","Action":"pass","Test":"TestColor","Elapsed":0.07}

What did you expect to see?

The last event will be fail

What did you see instead?

The last event is pass

The text was updated successfully, but these errors were encountered:

dmitshur · 2020-07-10T19:36:10Z

/cc @jayconrod @bcmills @matloob

bcmills · 2020-07-17T15:08:54Z

CC @eliben @mvdan (by way of #31969)

Perhaps cmd/test2json with an explicit command argument should check the exit code of the binary; but then it still wouldn't be able to correctly annotate test logs supplied via standard input. The log of a test binary is fundamentally not sufficient to tell whether it passed or failed.

Since test functions that call os.Exit(0) should be exceedingly rare anyway (#29062), I agree with @mvdan's comment in #31969 (comment) that we should interpret a not-affirmatively-passing test as a failure rather than a success.

dnephin · 2020-08-13T15:10:43Z

This issue sounds related to #38382

I agree with @mvdan's comment in #31969 (comment) that we should interpret a not-affirmatively-passing test as a failure rather than a success.

I had to write that logic in gotestsum to handle the case of missing Action=fail events. It would be great if this could be handled by test2json or go test.

bcmills · 2020-08-14T20:45:24Z

@gopherbot, please backport to Go 1.15. This seems like a significant regression.

gopherbot · 2020-08-14T20:45:34Z

Backport issue(s) opened: #40805 (for 1.15).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

gopherbot · 2020-08-14T21:52:37Z

Change https://golang.org/cl/248624 mentions this issue: cmd/test2json: do not emit a final Action if the result is not known

gopherbot · 2020-08-17T19:44:36Z

Change https://golang.org/cl/248725 mentions this issue: [release-branch.go1.15] cmd/test2json: do not emit a final Action if the result is not known

…the result is not known If we are parsing a test output, and the test does not end in the usual PASS or FAIL line (say, because it panicked), then we need the exit status of the test binary in order to determine whether the test passed or failed. If we don't have that status available, we shouldn't guess arbitrarily — instead, we should omit the final "pass" or "fail" action entirely. (In practice, we nearly always DO have the final status, such as when running 'go test' or 'go tool test2json some.exe'.) Updates #40132 Fixes #40805 Change-Id: Iae482577361a6033395fe4a05d746b980e18c3de Reviewed-on: https://go-review.googlesource.com/c/go/+/248624 Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com> (cherry picked from commit 1b86bdb) Reviewed-on: https://go-review.googlesource.com/c/go/+/248725

…the result is not known If we are parsing a test output, and the test does not end in the usual PASS or FAIL line (say, because it panicked), then we need the exit status of the test binary in order to determine whether the test passed or failed. If we don't have that status available, we shouldn't guess arbitrarily — instead, we should omit the final "pass" or "fail" action entirely. (In practice, we nearly always DO have the final status, such as when running 'go test' or 'go tool test2json some.exe'.) Updates golang#40132 Fixes golang#40805 Change-Id: Iae482577361a6033395fe4a05d746b980e18c3de Reviewed-on: https://go-review.googlesource.com/c/go/+/248624 Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com> (cherry picked from commit 1b86bdb) Reviewed-on: https://go-review.googlesource.com/c/go/+/248725

There have been some updates to test2json.go since d47526ed777958aa4a2542382e931eb7b3c4c6a9, most notably a fix for golang/go#40132 that was important enough to recevie a backport. Update rules_go's copy of test2json.go to the latest.

zolotov mentioned this issue Jul 9, 2020

cmd/go: tests that panic or exit are marked as passing when -json flag is used #37555

Closed

dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 10, 2020

jayconrod added this to the Go1.16 milestone Jul 10, 2020

gopherbot mentioned this issue Aug 14, 2020

cmd/test2json: tests that panic are marked as passing [1.15 backport] #40805

Closed

bcmills self-assigned this Aug 14, 2020

bcmills added the release-blocker label Aug 14, 2020

gopherbot closed this as completed in 1b86bdb Aug 17, 2020

dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Sep 2, 2020

robbertvanginkel mentioned this issue May 6, 2021

Update test2json.go bazelbuild/rules_go#2873

Merged

bcmills mentioned this issue Jun 18, 2021

x/playground: a test that terminates itself with a signal is spuriously reported as passing #46823

Open

golang locked and limited conversation to collaborators Sep 2, 2021

gopherbot added the FrozenDueToAge label Sep 2, 2021

rsc unassigned bcmills Jun 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/test2json: tests that panic are marked as passing #40132

cmd/test2json: tests that panic are marked as passing #40132

zolotov commented Jul 9, 2020

dmitshur commented Jul 10, 2020

bcmills commented Jul 17, 2020

dnephin commented Aug 13, 2020

bcmills commented Aug 14, 2020

gopherbot commented Aug 14, 2020

gopherbot commented Aug 14, 2020

gopherbot commented Aug 17, 2020

cmd/test2json: tests that panic are marked as passing #40132

cmd/test2json: tests that panic are marked as passing #40132

Comments

zolotov commented Jul 9, 2020

What version of Go are you using (go version)?

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

What did you do?

What did you expect to see?

What did you see instead?

dmitshur commented Jul 10, 2020

bcmills commented Jul 17, 2020

dnephin commented Aug 13, 2020

bcmills commented Aug 14, 2020

gopherbot commented Aug 14, 2020

gopherbot commented Aug 14, 2020

gopherbot commented Aug 17, 2020

What version of Go are you using (`go version`)?

What operating system and processor architecture are you using (`go env`)?