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

Jump to bottom

cmd/test2json: has no timeout and behaves differently from go test in terms of signal handling #54023

Closed
ianwoolf opened this issue Jul 24, 2022 · 7 comments
Closed

cmd/test2json: has no timeout and behaves differently from go test in terms of signal handling #54023

ianwoolf opened this issue Jul 24, 2022 · 7 comments
Labels
FrozenDueToAge

Comments

@ianwoolf
Copy link
Contributor

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

$ go version
go version go1.19beta1 darwin/amd64
(also reproduces on 1.18.3)

Does this issue reproduce with the latest release?

yes

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

go env Output
GO111MODULE="on"
GOARCH="amd64"
GOBIN="/Users/bytedance/lang/go/bin"
GOCACHE="/Users/bytedance/Library/Caches/go-build"
GOENV="/Users/bytedance/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/bytedance/code/go/pkg/mod"
GONOPROXY="*.byted.org,*.everphoto.cn"
GONOSUMDB="*.byted.org,*.everphoto.cn"
GOOS="darwin"
GOPATH="/Users/bytedance/code/go"
GOPRIVATE="*.byted.org,*.everphoto.cn"
GOPROXY="https://goproxy.cn"
GOROOT="/Users/bytedance/code/self/go"
GOSUMDB="sum.golang.google.cn"
GOTMPDIR=""
GOTOOLDIR="/Users/bytedance/code/self/go/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.19beta1"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/bytedance/code/self/go/src/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/p0/kb961x5d6wd79gfp2h6t_8500000gn/T/go-build1211973374=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

What did you expect to see?

go test has -timeout and does not handle the Ctrl-C. But test2json does not have test2json and handles Ctrl-C.

What did you see instead?

test2json should behave the same as go test in terms of signal handling.
@gopherbot
Copy link

Change https://go.dev/cl/419295 mentions this issue: cmd/test2json: add timeout and signal handlers

@ianwoolf
Copy link
Contributor Author

related to #53563

@ianwoolf
Copy link
Contributor Author

I submitted a cl. As I replied in cl, I think it might be more appropriate to put the signal related code into src/cmd/internal/base？

@ianlancetaylor
Copy link
Contributor

Why do we need a timeout? We don't expect people to run test2json themselves. We expect them to run it via go test -json.

@ianwoolf
Copy link
Contributor Author

Why do we need a timeout? We don't expect people to run test2json themselves. We expect them to run it via go test -json.

If we don't expect people to run test2json themselves, then test2json does not behave exactly like go test. If so, I think it would be appropriate to remove the timeout. But I want to confirm this, because there should be people using test2json now.

By the way, I want to confirm, should I add the signal related code directly to the test2json package? I originally wanted to move it from cmd/go/internal/base to cmd/internal/base, but I thought about it again, maybe other cmd don't need this.

how about you？@bcmills

@bcmills
Copy link
Contributor

bcmills commented Jul 25, 2022

Duplicate of #53563

@bcmills bcmills marked this as a duplicate of #53563 Jul 25, 2022
@bcmills bcmills closed this as not planned Won't fix, can't repro, duplicate, stale Jul 25, 2022
@bcmills
Copy link
Contributor

bcmills commented Jul 25, 2022

If you want to run the test with a timeout outside of go test, you need to pass it the -test.timeout flag explicitly.

The signal-handling behavior is exactly the root cause of #53563.

@golang golang locked and limited conversation to collaborators Jul 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@ianlancetaylor @bcmills @gopherbot @ianwoolf