Let a long step's exec runner be bound by its context alone (issue 1497), part 8 of 8

go/forgekey/runner.go

@@ -23,14 +23,30 @@ type Ran struct {
 // Runner runs one command with extra environment words, so every tool can be tested without ssh.
 type Runner func(ctx context.Context, env []string, name string, args ...string) Ran
 
-// CallTimeout bounds one command: below the runtime's thirty-second call limit.
-const CallTimeout = 20 * time.Second
+// CallTimeout bounds one command of ExecRunner: below the runtime's thirty-second call limit. A variable so a
+// test can lower it.
+var CallTimeout = 20 * time.Second
 
 // ExecRunner runs a command on this machine, with no terminal and nothing on its stdin, bounded by
 // CallTimeout.
 func ExecRunner(ctx context.Context, env []string, name string, args ...string) Ran {
-	ctx, cancel := context.WithTimeout(ctx, CallTimeout)
-	defer cancel()
+	return ExecRunnerFor(CallTimeout)(ctx, env, name, args...)
+}
+
+// ExecRunnerFor is ExecRunner with its own bound (issue 1497): a build module's long step passes 0, so only the
+// step's own context ends a command, and a fetch may outlast a tool call's thirty seconds.
+func ExecRunnerFor(bound time.Duration) Runner {
+	return func(ctx context.Context, env []string, name string, args ...string) Ran {
+		return execRun(ctx, bound, env, name, args...)
+	}
+}
+
+func execRun(ctx context.Context, bound time.Duration, env []string, name string, args ...string) Ran {
+	if bound > 0 {
+		var cancel context.CancelFunc
+		ctx, cancel = context.WithTimeout(ctx, bound)
+		defer cancel()
+	}
 	cmd := exec.CommandContext(ctx, name, args...)
 	cmd.Env = append(os.Environ(), env...)
 	var out, errb bytes.Buffer
@@ -40,7 +56,7 @@ func ExecRunner(ctx context.Context, env []string, name string, args ...string)
 	var exitErr *exec.ExitError
 	switch {
 	case errors.Is(ctx.Err(), context.DeadlineExceeded):
-		r.Status, r.Err = 124, fmt.Sprintf("no answer within %d s", int(CallTimeout/time.Second))
+		r.Status, r.Err = 124, fmt.Sprintf("no answer within %d s", int(bound/time.Second))
 	case errors.Is(err, exec.ErrNotFound):
 		r.Status, r.Err = 127, "ENOENT"
 	case errors.As(err, &exitErr):

go/forgekey/runner_test.go

@@ -0,0 +1,42 @@
+package forgekey
+
+import (
+	"context"
+	"testing"
+	"time"
+)
+
+func lowered(t *testing.T) {
+	was := CallTimeout
+	CallTimeout = 50 * time.Millisecond
+	t.Cleanup(func() { CallTimeout = was })
+}
+
+// 1497/AC1: ExecRunnerFor(0), no deadline: a command longer than the lowered CallTimeout is not cut.
+func TestExecRunnerForZeroIsNotCutByCallTimeout(t *testing.T) {
+	lowered(t)
+	r := ExecRunnerFor(0)(context.Background(), nil, "sleep", "0.2")
+	if r.Status != 0 || r.Err != "" {
+		t.Errorf("ran = %+v, want status 0", r)
+	}
+}
+
+// 1497/AC2: ExecRunner is unchanged: cut at CallTimeout with status 124.
+func TestExecRunnerIsStillCutAtCallTimeout(t *testing.T) {
+	lowered(t)
+	r := ExecRunner(context.Background(), nil, "sleep", "0.2")
+	if r.Status != 124 {
+		t.Errorf("ran = %+v, want status 124", r)
+	}
+}
+
+// 1497/AC3: ExecRunnerFor(0) is cut when its context is cancelled.
+func TestExecRunnerForZeroIsCutByItsContext(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
+	defer cancel()
+	began := time.Now()
+	r := ExecRunnerFor(0)(ctx, nil, "sleep", "2")
+	if r.Status == 0 || time.Since(began) > time.Second {
+		t.Errorf("ran = %+v after %v, want it cut", r, time.Since(began))
+	}
+}