cmd/go: unable to disable test timeout

[cpuguy83]

Please answer these questions before submitting your issue. Thanks!

1. What version of Go are you using (go version)?
   1.6
2. What operating system and processor architecture are you using (go env)?
   linux/amd64
3. What did you do?

go test timeouts are implemented in 2 ways:

There is the main timeout which just calls panic when the timeout is reached.
This timeout can be disabled when you set a value less than 1

A secondary timeout is set as a failsafe which calls SIGQUIT to dump a stacktrace and then exits.
This is set to the main timeout value + 1 minute:

go/src/cmd/go/test.go

Line 409 in 9323de3

if dt, err := time.ParseDuration(testTimeout); err == nil && dt > 0 {

If the main timeout is not > 0, it just stays at the default 10m.

1. What did you expect to see?
   The test timeout should be able to be be disabled.
2. What did you see instead?
   The secondary timeout is in place even when no timeout is set and there is no way to disable.

We use a test helper that wraps testing.T to provide its own functionality, like setup/teardown tests.
This means the entire suite uses a single testing.T. With that, means the only way to not have go kill our integration tests is to set an really high value for the timeout (which I think also defeats the purpose of the timeout to begin with).

[ianlancetaylor]

What is the difference between disabling the timeout and using a very high value? That is, why doesn't setting a very high value solve your problem?

[cpuguy83]

It works, but doesn't feel right.
And then I either have to set an insanely high value and can leave it alone or set something more inline with the time it takes to run the suite and bump it as we add new tests.

[ianlancetaylor]

What are you suggesting that we do?

Note that I would vote against anything that involves adding another option to go test. Additional knobs carry a cost, and require a corresponding benefit. That benefit needs to be more than "it feels more right."

[cpuguy83]

I agree on not adding another option. Currently the code checks if the timeout is zero and then sets the fallback if it's not.
If it is zero (or perhaps rather just explicitly set to 0) it can disable the fallback as well?

[ianlancetaylor]

I suppose that is OK with me. Want to send a patch?

[cpuguy83]

Sorry yes, I've been putting this off until I have a chance to digest the contribution process.

[gopherbot]

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

[rsc]

I don't see any argument for changing the meaning of -timeout=0 when -timeout=1000h already does what you want and is much clearer about the effect.

[ianlancetaylor]

@rsc The -test.timeout flag is documented as accepting 0 to override the timeout in the -help output of any test binary and also in https://tip.golang.org/cmd/go/#hdr-Description_of_testing_flags . So I think this is either a bug in the implementation or a bug in the documentation.

[rsc]

@ianlancetaylor, that text about timeout=0 does not appear in Go 1.8 docs. I will revert whatever introduced it in the Go 1.9 cycle. Thanks.

[gopherbot]

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

[vcabbage]

@rsc I introduced the incorrect text about timeout=0 while trying to make the docs consistent with:

flag.Duration("test.timeout", 0, "panic test binary after duration `d` (0 means unlimited)")

go/src/testing/testing.go

Line 268 in bb3be40

timeout = flag.Duration("test.timeout", 0, "panic test binary after duration `d` (0 means unlimited)")

Is the text in the flag also incorrect or have I misunderstood something here?

[vcabbage]

I think I've answered my own question by running some tests.

• When executing a test binary (built with go test -c) -test.timeout=0 does mean no timeout, which is also the default.
• When running tests with go test -timeout=0 the default timeout of 10m is used.

The asymmetry seems confusing.

[rsc]

Yes, running under the go command is different from running a test binary by hand. I left the issue open (the CL did not say Fix) because I want to think more about this in Go 1.10. The thing is, no one would ever say -test.timeout=0 when invoking a binary by hand, because that's the default. It may be that the right fix to the binary doc is to change

-test.timeout d
	panic test binary after duration d (0 means unlimited)

to

-test.timeout d
	panic test binary after duration d (default unlimited)

There's still the fact that the go command default is different from a test binary's default, but that seems at least plausibly defensible.

Anyway, at least the docs are correct for Go 1.9.

[gopherbot]

Change https://golang.org/cl/81499 mentions this issue: cmd/go: honor -timeout=0 to mean no timeout