As motivated in the commit message of CL 504495:
Since go test has a default top-level timeout, individual tests don't
need to impose their own timeouts too. It's possible to adjust it via
the -timeout flag or ^C if something stalled and should be considered
to have failed sooner.
Any constant timeout we choose will either be too long to ever trigger,
or not long enough to handle rare cases where the system is overloaded
or just slow. The only winning move is not to play.
For automated tests we might not be able to get away from having a hard constant timeout in at least one layer, but we probably don't benefit from cases where a test has 2 or more overlapping constant timeouts. Whenever a system is overloaded and happens to exceed the shorter of the two overlapping timeouts, it will will cause test failures that produce little useful new information. Recent examples are in issue #78392, the timeout in https://ci.chromium.org/ui/test-investigate/invocations/build-8679277370961616529/tests, and many matches for panic: test timed out after 3m0s or similar.
In recent times when adding new tests to cmd/dist, we've been preferring to avoid adding custom timeouts unless there's a concrete need. For instance:
// newer test, no custom timeout (that is, it inherits the default test timeout imposed by go test)
t.registerTest("API release note check", &goTest{variant: "check", pkg: "cmd/relnote", testFlags: []string{"-check"}})
// older test, still has a custom timeout of 5 minutes from long ago
t.registerTest("API check", &goTest{variant: "check", pkg: "cmd/api", timeout: 5 * time.Minute, testFlags: []string{"-check"}})From what I can tell the "no custom timeout" has generally been working well. Custom timeouts may still be needed for tests that try to go beyond the go test's default:
timeout: 1200 * time.Second, // longer timeout for race with benchmarks
Otherwise, this is a tracking issue to drop the unhelpful custom timeouts from cmd/dist.
It's worth noting that proposal #48157 is accepted. If there are cases where we want to explicitly annotate a given test with a custom per-test timeout, implementing that proposal and using it is probably the way to go.
CC @golang/release, @golang/command-line.
As motivated in the commit message of CL 504495:
For automated tests we might not be able to get away from having a hard constant timeout in at least one layer, but we probably don't benefit from cases where a test has 2 or more overlapping constant timeouts. Whenever a system is overloaded and happens to exceed the shorter of the two overlapping timeouts, it will will cause test failures that produce little useful new information. Recent examples are in issue #78392, the timeout in https://ci.chromium.org/ui/test-investigate/invocations/build-8679277370961616529/tests, and many matches for panic: test timed out after 3m0s or similar.
In recent times when adding new tests to cmd/dist, we've been preferring to avoid adding custom timeouts unless there's a concrete need. For instance:
From what I can tell the "no custom timeout" has generally been working well. Custom timeouts may still be needed for tests that try to go beyond the go test's default:
Otherwise, this is a tracking issue to drop the unhelpful custom timeouts from cmd/dist.
It's worth noting that proposal #48157 is accepted. If there are cases where we want to explicitly annotate a given test with a custom per-test timeout, implementing that proposal and using it is probably the way to go.
CC @golang/release, @golang/command-line.