cmd/go: treat cached test results as satisfying any timeout

We want test caching to work even for people with scripts
that set a non-default test timeout. But then that raises the
question of what to do about runs with different timeouts:
is a cached success with one timeout available for use when
asked to run the test with a different timeout?

This CL answers that question by saying that the timeout applies
to the overall execution of either running the test or displaying
the cached result, and displaying a cached result takes no time.
So it's always OK to record a cached result, regardless of timeout,
and it's always OK to display a cached result, again regardless of timeout.

Fixes #22633.

Change-Id: Iaef3602710e3be107602267bbc6dba9a2250796c
Reviewed-on: https://go-review.googlesource.com/76552
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: roger peppe <rogpeppe@gmail.com>
Reviewed-by: David Crawshaw <crawshaw@golang.org>

src/cmd/go/go_test.go

@@ -4823,7 +4823,9 @@ func TestTestCache(t *testing.T) {
 	tg.setenv("GOPATH", tg.tempdir)
 	tg.setenv("GOCACHE", filepath.Join(tg.tempdir, "cache"))
 
-	tg.run("test", "-x", "errors")
+	// timeout here should not affect result being cached
+	// or being retrieved later.
+	tg.run("test", "-x", "-timeout=10s", "errors")
 	tg.grepStderr(`[\\/]compile|gccgo`, "did not run compiler")
 	tg.grepStderr(`[\\/]link|gccgo`, "did not run linker")
 	tg.grepStderr(`errors\.test`, "did not run test")
@@ -4835,6 +4837,10 @@ func TestTestCache(t *testing.T) {
 	tg.grepStderrNot(`errors\.test`, "incorrectly ran test")
 	tg.grepStderrNot("DO NOT USE", "poisoned action status leaked")
 
+	// Even very low timeouts do not disqualify cached entries.
+	tg.run("test", "-timeout=1ns", "-x", "errors")
+	tg.grepStderrNot(`errors\.test`, "incorrectly ran test")
+
 	// The -p=1 in the commands below just makes the -x output easier to read.
 
 	t.Log("\n\nINITIAL\n\n")

src/cmd/go/internal/test/test.go

@@ -105,7 +105,9 @@ go test will redisplay the previous output instead of running the test
 binary again. In the summary line, go test prints '(cached)' in place of
 the elapsed time. To disable test caching, use any test flag or argument
 other than the cacheable flags. The idiomatic way to disable test caching
-explicitly is to use -count=1.
+explicitly is to use -count=1. A cached result is treated as executing in
+no time at all, so a successful package test result will be cached and reused
+regardless of -timeout setting.
 
 ` + strings.TrimSpace(testFlag1) + ` See 'go help testflag' for details.
 
@@ -1346,6 +1348,7 @@ func (c *runCache) tryCacheWithID(b *work.Builder, a *work.Action, id string) bo
 		return false
 	}
 
+	var cacheArgs []string
 	for _, arg := range testArgs {
 		i := strings.Index(arg, "=")
 		if i < 0 || !strings.HasPrefix(arg, "-test.") {
@@ -1362,6 +1365,12 @@ func (c *runCache) tryCacheWithID(b *work.Builder, a *work.Action, id string) bo
 			// These are cacheable.
 			// Note that this list is documented above,
 			// so if you add to this list, update the docs too.
+			cacheArgs = append(cacheArgs, arg)
+
+		case "-test.timeout":
+			// Special case: this is cacheable but ignored during the hash.
+			// Do not add to cacheArgs.
+
 		default:
 			// nothing else is cacheable
 			c.disableCache = true
@@ -1375,7 +1384,7 @@ func (c *runCache) tryCacheWithID(b *work.Builder, a *work.Action, id string) bo
 	}
 
 	h := cache.NewHash("testResult")
-	fmt.Fprintf(h, "test binary %s args %q execcmd %q", id, testArgs, work.ExecCmd)
+	fmt.Fprintf(h, "test binary %s args %q execcmd %q", id, cacheArgs, work.ExecCmd)
 	// TODO(rsc): How to handle other test dependencies like environment variables or input files?
 	// We could potentially add new API like testing.UsedEnv(envName string)
 	// or testing.UsedFile(inputFile string) to let tests declare what external inputs