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

proposal: testing: allow per-test timeouts #58549

Closed
seebs opened this issue Feb 15, 2023 · 6 comments
Closed

proposal: testing: allow per-test timeouts #58549

seebs opened this issue Feb 15, 2023 · 6 comments

Comments

@seebs
Copy link
Contributor

seebs commented Feb 15, 2023

A lot of test cases could be most cleanly expressed by specifying a weak timeout estimate and doing something that may or may not successfully complete. In practice, tests that are going to hit a deadlock on failure manifest as the entire test run getting killed at the global test timeout, and dumping all of its goroutines, which can be a lot to sort through. It also means that at most one such test actually reports a failure.

It would be really convenient if you could write tests that expect a thing to happen in some time window, and if it doesn't, the test fails WITHOUT killing the entire go test process, dumping goroutines, and so on.

Proposed API addition:

func (*T) Timeout(time.Time)

This specifies a per-test timeout. If the timeout is reached before the test completes, the test is considered to have failed, as though by t.Fatalf("test duration (%v) exceeded", duration).

Right now, if I really, really, want to write a test that tries to check for a deadlock, I'm probably doing something messy with channels and goroutines. Because t.Fatalf and friends can't be called from other goroutines, I can't use that for both failures within the execution path of the test, and timeout-style failures; one or the other of them is in a different goroutine. There's no really good way to express this, but I think the testing internals could do it significantly more cleanly. And even if it's hard to get right, the testing package only has to get it right once.

@seebs seebs added the Proposal label Feb 15, 2023
@gopherbot gopherbot added this to the Proposal milestone Feb 15, 2023
@ianlancetaylor
Copy link
Contributor

CC @bcmills

@seankhliao
Copy link
Member

Isn't this likely to leak resources on timeout?
Sounds to me it would be better to execute the individual test as a separate process with a timeout, a bit like fizzing

@bcmills
Copy link
Contributor

bcmills commented Feb 15, 2023

If a test deadlocks, in my experience you do want to dump its goroutines (because that's generally the most useful way to diagnose it).

Moreover, I have seen an awful lot of tests with hard-coded timeouts that don't scale appropriately. For example, if a test is run on a single-core ARM machine with a small physical RAM and swapfile, that test might take two or three orders of magnitude longer than on a many-core AMD64 workstation. Generally a user testing on a very slow machine will run go test with a more generous -timeout flag, but I don't see a clean way to incorporate that into this API.

I agree with @seankhliao's recommendation to run the test as a subprocess if you want to get multiple failures in a run. You can also explicitly set -test.timeout to whatever you like for that subprocess to obtain the desired timeout behavior. (I wonder if at some point we should export internal/testenv.CommandContext, which is intended for exactly that sort of use-case, but that's a matter for a separate proposal.)

@cespare
Copy link
Contributor

cespare commented Feb 15, 2023

Isn't this covered by #48157 (which was accepted but hasn't been implemented)?

@bcmills
Copy link
Contributor

bcmills commented Feb 15, 2023

Um... Yes!

@bcmills
Copy link
Contributor

bcmills commented Feb 15, 2023

Duplicate of #48157

@bcmills bcmills marked this as a duplicate of #48157 Feb 15, 2023
@bcmills bcmills closed this as not planned Won't fix, can't repro, duplicate, stale Feb 15, 2023
@golang golang locked and limited conversation to collaborators Feb 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants