fix(quality): count mutation timeouts as killed up to a cap

go/private/gomutants.bzl

@@ -6,6 +6,7 @@
 load(
     "@tbhb_rules//quality:defs.bzl",
     "MUTATION_CHANGES_ATTRS",
+    "MUTATION_RUNNER_ENV",
     "MutationReportInfo",
     "mutation_changes_slice",
     "mutation_gate_test",
@@ -21,10 +22,9 @@ visibility(["//", "//private/...", "//tests/..."])
 # test. The flags attribute follows these, so a consumer's value wins.
 _DEFAULT_FLAGS = ["--timeout-min=20s", "--test-flags=-failfast"]
 
-# A timed-out mutant is unstable, so a slow Windows runner fails the gate when
-# the ceiling (the baseline times the coefficient) is too tight. A Windows
-# execution platform gets a higher floor and ceiling; a timeout still never
-# counts as killed.
+# Timeouts count as killed up to a cap, so a slow Windows runner fails the gate
+# when the ceiling (the baseline times the coefficient) is too tight. A Windows
+# execution platform gets a higher floor and ceiling.
 _WINDOWS_DEFAULT_FLAGS = ["--timeout-min=60s", "--timeout-coefficient=40", "--test-flags=-failfast"]
 
 def default_flags(exec_goos):
@@ -73,6 +73,9 @@ def _gomutants_report_impl(ctx):
         ),
         tools = [ctx.attr._gomutants[DefaultInfo].files_to_run, gomutants],
         outputs = [report, log],
+        # The runner exits non-zero over the timeout cap, so Bazel does not
+        # cache a starved run; the gate reads the same cap.
+        env = MUTATION_RUNNER_ENV,
         mnemonic = "Gomutants",
         progress_message = "Mutation-testing %{label}",
         # gomutants times each mutant's tests against their recorded
@@ -96,6 +99,8 @@ module, and runs gomutants on the package with the toolchain's go, offline
 (modules come from `proxy`). gomutants runs a mutant against the tests of
 its own package. So a change to a package reruns its unit and the units of
 the packages that depend on it, and every other unit is a cache hit.
+The action fails (and Bazel does not cache it) on an errored or interrupted
+run and on timeouts over MUTATION_MAX_TIMEOUT_PERCENT of the mutants.
 
 `changes` (quality's `changes` flag by default) sets the scope. With no
 change set the package runs in full. With one, the action reads the unit's
@@ -123,16 +128,17 @@ slice (the `changes` output group).""",
 )
 
 def gomutants_test(name, tests, go_mod, go_sum = None, proxy = None, data = [], flags = [], cpus = 0, changes = None, require_scope = "any", mutation_oses = None, **kwargs):
-    """Mutation-tests Go packages with gomutants and fails unless every mutant was killed.
+    """Mutation-tests Go packages with gomutants and fails unless every mutant was killed or timed out (within a cap).
 
     Creates one gomutants_report per package that a test in `tests` is in,
     `<name>.<package>` (one cached gomutants run each), and `<name>`, a
     `mutation_gate_test` over their reports. Fails on a mutant that lived
-    (its tests passed), one no test covers, a timeout, an infrastructure
-    error, a failed run, a diff-scope selection that does not match what
-    gomutants tested, and a full run that found nothing to mutate. A mutant
-    that does not compile is skipped, not killed. Account for a mutant that
-    must stay alive with a reviewed
+    (its tests passed), one no test covers, timeouts over
+    MUTATION_MAX_TIMEOUT_PERCENT of the mutants, an infrastructure error,
+    a failed run, a diff-scope selection that does not match what gomutants
+    tested, and a full run that found nothing to mutate. A mutant that does
+    not compile is skipped, not killed. Account for a mutant that must stay
+    alive with a reviewed
     `// gomutants:disable-next-line <MUTATOR> reason="..."` directive.
 
     Args:

go/private/gomutants/main.go

@@ -16,8 +16,14 @@
 // through a fake git (this binary under the name git). Every mutant
 // reported must be one the selection holds and vice versa; a mismatch is
 // reported as mutant.unmapped. It writes the report mutation_gate_test
-// reads and exits 1 after writing one with a timeout, errored or
-// interrupted count, so Bazel never caches it.
+// reads, then exits 1 for an errored or interrupted count, for a diff run
+// whose selected mutants are not the mutants it reported (selected other than
+// total, or any mutant.unmapped line: a selected mutant with no outcome), or
+// for timeouts above MUTATION_MAX_TIMEOUT_PERCENT of the mutants (set by the
+// action from quality's constant, which the gate reads too). So Bazel never
+// caches a run that machine load may have spoiled or that the gate rejects as
+// incomplete: such a run is rerun, not replayed. Timeouts within the cap exit
+// 0; the gate counts them as killed.
 //
 // Invoked as git, it answers the two commands gomutants' --changed-since
 // runs from GOMUTANTS_FAKE_GIT_ROOT and GOMUTANTS_FAKE_GIT_DIFF.
@@ -51,6 +57,10 @@ import (
 
 var mainExit = os.Exit
 
+// maxTimeoutEnv holds the most timed-out mutants, in percent of the total,
+// that still count as killed. The action sets it, so it has no default.
+const maxTimeoutEnv = "MUTATION_MAX_TIMEOUT_PERCENT"
+
 func main() {
 	if strings.TrimSuffix(filepath.Base(os.Args[0]), ".exe") == "git" {
 		mainExit(fakeGit(os.Args[1:]))
@@ -217,6 +227,11 @@ func runMain(args []string) int {
 		fmt.Fprintln(os.Stderr, "gomutants: -gomutants, -go, -manifest, -pkg, -report and -log are required")
 		return 2
 	}
+	maxTimeout, err := strconv.Atoi(os.Getenv(maxTimeoutEnv))
+	if err != nil || maxTimeout < 0 || maxTimeout > 100 {
+		fmt.Fprintf(os.Stderr, "gomutants: %s must be an integer from 0 to 100\n", maxTimeoutEnv)
+		return 2
+	}
 	r, log, err := run(c)
 	if err == nil {
 		err = runnerWriteFile(c.log, log, 0o644)
@@ -228,7 +243,7 @@ func runMain(args []string) int {
 		fmt.Fprintf(os.Stderr, "gomutants: %v\n", err)
 		return 1
 	}
-	if found := r.unstable(); len(found) > 0 {
+	if found := r.unstable(maxTimeout); len(found) > 0 {
 		fmt.Fprintf(os.Stderr, "gomutants: not cacheable: %s\n%s", strings.Join(found, ", "), log)
 		if len(r.mutants) > 0 {
 			sort.Strings(r.mutants)
@@ -263,20 +278,42 @@ func (r result) format() string {
 	return b.String()
 }
 
-func (r result) unstable() []string {
+// unstable names the verdicts the action must not cache: those machine load
+// may have caused (errored or interrupted mutants, timeouts above maxTimeout
+// percent of the total) and the incomplete diff reports the gate rejects (a
+// selection that is not the total, and any unmapped mutant).
+func (r result) unstable(maxTimeout int) []string {
 	var found []string
-	if r.timeout > 0 {
-		found = append(found, "timeout")
-	}
 	if r.errored > 0 {
 		found = append(found, "errored")
 	}
 	if r.interrupted > 0 {
 		found = append(found, "interrupted")
 	}
+	if r.timeout*100 > r.total*maxTimeout {
+		found = append(found, fmt.Sprintf("%d timeouts of %d, above %d%%", r.timeout, r.total, maxTimeout))
+	}
+	if r.scope == "diff" && r.selected != r.total {
+		found = append(found, fmt.Sprintf("selected %d, total %d", r.selected, r.total))
+	}
+	if n := r.unmapped(); n > 0 {
+		found = append(found, fmt.Sprintf("%d unmapped", n))
+	}
 	return found
 }
 
+// unmapped counts the mutant.unmapped lines: mutants the selection and the
+// report disagree on.
+func (r result) unmapped() int {
+	n := 0
+	for _, m := range r.mutants {
+		if strings.HasPrefix(m, "mutant.unmapped=") {
+			n++
+		}
+	}
+	return n
+}
+
 func run(c runConfig) (result, []byte, error) {
 	var log bytes.Buffer
 	entries, err := scratch.ReadManifest(c.manifest)

go/private/gomutants/main_test.go

@@ -42,6 +42,8 @@ func TestMain(m *testing.M) {
 		_ = os.Setenv("GOCOVERDIR", os.Getenv("FAKE_GOCOVERDIR"))
 		os.Exit(fakeGomutants(rc))
 	}
+	// The action sets the timeout cap; the runner has no default.
+	_ = os.Setenv(maxTimeoutEnv, "10")
 	os.Exit(m.Run())
 }
 
@@ -291,7 +293,7 @@ mutant.timed_out=mod/p/p.go:13:9 X (p/p.go:Double:X#1)
 	if got := r.format(); got != want {
 		t.Errorf("report:\n%s\nwant:\n%s", got, want)
 	}
-	if got := strings.Join(r.unstable(), ","); got != "timeout,errored" {
+	if got := strings.Join(r.unstable(100), ","); got != "errored" {
 		t.Errorf("unstable = %s", got)
 	}
 	if err := (&result{}).read([]byte(`{"files":[{"mutations":[{"status":"ODD"}]}]}`), ""); err == nil {
@@ -436,10 +438,6 @@ func TestRunUnstable(t *testing.T) {
 	for _, tc := range []struct {
 		name, rc, js, reason, mutants string
 	}{
-		{
-			"timeout", "0", strings.Replace(oneLived, "LIVED", "TIMED OUT", 1), "timeout",
-			"mutant.timed_out=p/p.go:7:15 CONDITIONALS_BOUNDARY (p/p.go:Pass:CONDITIONALS_BOUNDARY#1)\n",
-		},
 		{
 			"errored", "0", `{"files":[{"file_name":"p.go","mutations":[
 				{"id":"p/p.go:Pass:Z#1","type":"Z","status":"INFRA ERROR","line":7,"column":15},
@@ -449,6 +447,12 @@ func TestRunUnstable(t *testing.T) {
 			"mutant.infra_error=p/p.go:13:9 A (p/p.go:Double:A#1)\n" +
 				"mutant.infra_error=p/p.go:7:15 Z (p/p.go:Pass:Z#1)\n",
 		},
+		{
+			"one errored", "0", `{"files":[{"file_name":"p.go","mutations":[
+				{"id":"p/p.go:Pass:Z#1","type":"Z","status":"INFRA ERROR","line":7,"column":15}
+			]}]}`, "errored",
+			"mutant.infra_error=p/p.go:7:15 Z (p/p.go:Pass:Z#1)\n",
+		},
 		{"no report", "0", "", "interrupted", ""},
 		{"run failed", "1", "", "interrupted", ""},
 	} {
@@ -488,6 +492,153 @@ func TestRunUnstable(t *testing.T) {
 	}
 }
 
+// manyMutants is a report of n mutants, the first timedOut of them TIMED OUT
+// and the rest KILLED.
+func manyMutants(n, timedOut int) string {
+	var mutations []string
+	for i := 1; i <= n; i++ {
+		status := "KILLED"
+		if i <= timedOut {
+			status = "TIMED OUT"
+		}
+		mutations = append(mutations, fmt.Sprintf(`{"id":"p/p.go:Pass:CONDITIONALS_BOUNDARY#%d","type":"CONDITIONALS_BOUNDARY","status":%q,"line":7,"column":15}`, i, status))
+	}
+	return `{"files":[{"mutations":[` + strings.Join(mutations, ",") + `]}]}`
+}
+
+// A timeout counts as killed only up to the cap: the action exits 0 within it,
+// and above it exits 1 after writing the report, so Bazel never caches a
+// starved run (a rerun replays a cached report, not a fresh one).
+func TestRunTimeoutCap(t *testing.T) {
+	for _, tc := range []struct {
+		name            string
+		cap             string
+		total, timedOut int
+		code            int
+	}{
+		{"below the cap", "10", 20, 1, 0},
+		{"at the cap", "10", 10, 1, 0},
+		{"above the cap", "10", 10, 2, 1},
+		{"every mutant at a cap of 100", "100", 1, 1, 0},
+		{"one timeout at a cap of 0", "0", 1, 1, 1},
+		{"none at a cap of 0", "0", 1, 0, 0},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Setenv(maxTimeoutEnv, tc.cap)
+			js := manyMutants(tc.total, tc.timedOut)
+			code, report := runFake(t, "", "0", &js)
+			want := fmt.Sprintf("total=%d\nkilled=%d\nlived=0\nnot_covered=0\ntimeout=%d\n", tc.total, tc.total-tc.timedOut, tc.timedOut)
+			if code != tc.code || !strings.HasPrefix(report, want) {
+				t.Errorf("exit %d, want %d, report:\n%s", code, tc.code, report)
+			}
+		})
+	}
+}
+
+func TestRunTimeoutListsTheMutants(t *testing.T) {
+	t.Setenv(maxTimeoutEnv, "0")
+	js := strings.Replace(oneLived, "LIVED", "TIMED OUT", 1)
+	code, report := captureStderr(t, func() int { c, _ := runFake(t, "", "0", &js); return c })
+	want := "gomutants: not cacheable: 1 timeouts of 1, above 0%\n"
+	if code != 1 || !strings.HasPrefix(report, want) || !strings.Contains(report, "mutant.timed_out=p/p.go:7:15 CONDITIONALS_BOUNDARY (p/p.go:Pass:CONDITIONALS_BOUNDARY#1)\n") {
+		t.Errorf("exit %d, stderr:\n%s", code, report)
+	}
+}
+
+func TestUnstableTimeoutCap(t *testing.T) {
+	for _, tc := range []struct {
+		total, timeout, cap int
+		want                string
+	}{
+		{100, 5, 10, ""},
+		{100, 10, 10, ""},
+		{100, 11, 10, "11 timeouts of 100, above 10%"},
+		{1000, 100, 10, ""},
+		{1000, 101, 10, "101 timeouts of 1000, above 10%"},
+		{0, 0, 10, ""},
+		{0, 1, 10, "1 timeouts of 0, above 10%"},
+		{100, 0, 0, ""},
+		{100, 1, 0, "1 timeouts of 100, above 0%"},
+		{4, 4, 100, ""},
+	} {
+		got := strings.Join(result{total: tc.total, timeout: tc.timeout}.unstable(tc.cap), ",")
+		if got != tc.want {
+			t.Errorf("%d of %d at %d%%: %q, want %q", tc.timeout, tc.total, tc.cap, got, tc.want)
+		}
+	}
+}
+
+// A diff report must hold an outcome for every selected mutant: the gate
+// rejects any other, so the runner must not let Bazel cache one.
+func TestUnstableDiffSelection(t *testing.T) {
+	unmapped := "mutant.unmapped=p/p.go:7:15 X (selected, not reported)"
+	for _, tc := range []struct {
+		name string
+		r    result
+		want string
+	}{
+		{"selected is total", result{scope: "diff", selected: 2, total: 2}, ""},
+		{"selected above total", result{scope: "diff", selected: 2, total: 1}, "selected 2, total 1"},
+		{"selected below total", result{scope: "diff", selected: 1, total: 2}, "selected 1, total 2"},
+		{"nothing selected", result{scope: "diff"}, ""},
+		{"full scope selects nothing", result{scope: "full", total: 3}, ""},
+		{
+			"unmapped pair",
+			result{scope: "diff", selected: 1, total: 1, mutants: []string{
+				unmapped, "mutant.unmapped=p/p.go:9:3 Y (reported, not selected)", "mutant.lived=p/p.go:1:1 Z (id)",
+			}},
+			"2 unmapped",
+		},
+		{
+			"one unmapped",
+			result{scope: "diff", selected: 1, total: 1, mutants: []string{unmapped}},
+			"1 unmapped",
+		},
+		{
+			"unmapped and selection",
+			result{scope: "diff", selected: 1, total: 0, mutants: []string{unmapped}},
+			"selected 1, total 0,1 unmapped",
+		},
+		{
+			"other mutants only",
+			result{scope: "diff", selected: 1, total: 1, lived: 1, mutants: []string{"mutant.lived=p/p.go:1:1 Z (id)"}},
+			"",
+		},
+	} {
+		if got := strings.Join(tc.r.unstable(10), ","); got != tc.want {
+			t.Errorf("%s: unstable = %q, want %q", tc.name, got, tc.want)
+		}
+	}
+}
+
+// A selected mutant missing from a successful but empty report once left
+// selected=1, total=0 and exit 0, which Bazel cached.
+func TestRunDiffEmptyReportIsNotCached(t *testing.T) {
+	js := `{"files":[]}`
+	code, report := runFake(t, `{"version":1,"files":{"p/p.go":[[7,7]]}}`, "0", &js)
+	if code != 1 {
+		t.Errorf("exit %d, want 1; report:\n%s", code, report)
+	}
+	for _, want := range []string{"total=0\n", "selected=1\n", "mutant.unmapped=p/p.go:7:15 CONDITIONALS_BOUNDARY (selected, not reported)\n"} {
+		if !strings.Contains(report, want) {
+			t.Errorf("report lacks %q:\n%s", want, report)
+		}
+	}
+}
+
+func TestRunMainNeedsTheTimeoutCap(t *testing.T) {
+	for _, value := range []string{"", "ten", "-1", "101", "1.5"} {
+		t.Run(value, func(t *testing.T) {
+			t.Setenv(maxTimeoutEnv, value)
+			code, diagnostic := captureStderr(t, func() int { return runMain(runnerArgs(runnerConfig(t))) })
+			want := "gomutants: MUTATION_MAX_TIMEOUT_PERCENT must be an integer from 0 to 100\n"
+			if code != 2 || diagnostic != want {
+				t.Fatalf("exit %d, stderr %q", code, diagnostic)
+			}
+		})
+	}
+}
+
 func TestFakeGitRejectsOtherCommands(t *testing.T) {
 	if fakeGit([]string{"log"}) == 0 {
 		t.Error("git log answered")