diff --git a/cmd/mesh-controller/checks.go b/cmd/mesh-controller/checks.go index 1cca29e7..bbd6edc3 100644 --- a/cmd/mesh-controller/checks.go +++ b/cmd/mesh-controller/checks.go @@ -163,9 +163,9 @@ func (f following) pullUpdated(ctx context.Context, p link.PullUpdated, w io.Wri } scope := pullScope(p, entries, read, edges) // **The merge gate's checks stop** (novox/hq ADR 0503 §3, §5; issue 1299): the checker judges every repository whose - // trunk requires `mesh/check`, and the controller asks nothing of it and says nothing. Only a repository whose trunk - // requires `mesh/repo-check` alone (hq) has its own merge-check.sh asked of build-agent, and said with that layer - // alone; what the forge's protection requires is read, never listed (lead decision L11). + // trunk requires `mesh/check`. A repository whose trunk requires `mesh/repo-check` (hq), whatever else it requires + // (ADR 0513 §4), has its own merge-check.sh asked of build-agent, and said with that layer alone; any other is asked + // nothing and said nothing. What the forge's protection requires is read, never listed (lead decision L11). // Mesh-config is still told what the head declares (ADR 0502), whatever judges it. this := pullAtOf(p) if this.Truncated { @@ -180,20 +180,31 @@ func (f following) pullUpdated(ctx context.Context, p link.PullUpdated, w io.Wri fmt.Fprintf(w, "%s/%s#%d (%.8s): what %s's protection requires cannot be read, so nothing is asked and nothing is "+ "said: %v\n", p.Owner, p.Repo, p.Number, p.Commit, p.Base, err) return "", nil - case slices.Contains(required, sdkdelivery.StatusContextCheck): - fmt.Fprintf(w, "%s/%s#%d (%.8s): %s requires %s: the checker judges it; nothing is asked\n", p.Owner, p.Repo, - p.Number, p.Commit, p.Base, sdkdelivery.StatusContextCheck) - return "", nil - case !slices.Contains(required, sdkdelivery.StatusContextRepoCheck): - fmt.Fprintf(w, "%s/%s#%d (%.8s): %s requires neither %s nor %s: nothing is asked and nothing is said\n", p.Owner, - p.Repo, p.Number, p.Commit, p.Base, sdkdelivery.StatusContextCheck, sdkdelivery.StatusContextRepoCheck) + case !asksRepoCheck(required): + fmt.Fprintf(w, "%s/%s#%d (%.8s): %s does not require %s, and %s; nothing is asked\n", p.Owner, p.Repo, p.Number, + p.Commit, p.Base, sdkdelivery.StatusContextRepoCheck, checkerWords(required)) return "", nil } return f.repoCheckAlone(ctx, w, p, scope, entries) } +// asksRepoCheck says what a trunk's protection has the controller ask: its repository's own merge-check.sh whenever the +// trunk requires `mesh/repo-check`, whatever else it requires (ADR 0513 §4: a trunk requiring both has the checker judge +// `mesh/check` and the repository check asked beside it), and nothing otherwise. +func asksRepoCheck(required []string) bool { + return slices.Contains(required, sdkdelivery.StatusContextRepoCheck) +} + +// checkerWords says who judges a trunk that requires no `mesh/repo-check`. +func checkerWords(required []string) string { + if slices.Contains(required, sdkdelivery.StatusContextCheck) { + return "the checker judges its " + sdkdelivery.StatusContextCheck + } + return "does not require " + sdkdelivery.StatusContextCheck + " either, so nothing is said" +} + // repoCheckAlone asks build-agent for a repository's own merge-check.sh alone, for a pull request of a repository whose -// trunk requires `mesh/repo-check` and not `mesh/check` (ADR 0503 §5): no gate, no module built, its verdict said with the +// trunk requires `mesh/repo-check`, whether or not it also requires `mesh/check` (ADR 0503 §5, ADR 0513 §4): no gate, no module built, its verdict said with the // repo-check layer alone. func (f following) repoCheckAlone(ctx context.Context, w io.Writer, p link.PullUpdated, scope checkScope, entries []inventory.Entry) (string, error) { diff --git a/cmd/mesh-controller/checks_stop_integration_test.go b/cmd/mesh-controller/checks_stop_integration_test.go index df8a2e64..d0496f3a 100644 --- a/cmd/mesh-controller/checks_stop_integration_test.go +++ b/cmd/mesh-controller/checks_stop_integration_test.go @@ -97,4 +97,13 @@ func TestThePullRequestChecksAreAskedOnlyForARepositoryTheCheckerDoesNotJudge(t if c.Gate != nil || c.RepoCheck == nil || c.RepoCheck.Verdict != "warning" { t.Fatalf("0503 §5: the verdict said names a gate layer, or no repo-check layer: %+v", c) } + // A trunk requiring both (hq, ADR 0513 §4): the checker judges mesh/check, and the repository check is still asked. + required["novox/hq"] = []string{"mesh/check", "mesh/repo-check"} + if err := f.PullUpdated(ctx, pull("hq", 11, true)); err != nil { + t.Fatal(err) + } + if q, err := link.ReadQueue(ctx, js, link.TheBuildMachine); err != nil || len(q.Asks) != 2 || + q.Asks[1].Request.Check == nil || q.Asks[1].Request.Check.Repo != "hq" { + t.Fatalf("0513 §4: a trunk requiring both had no repository check asked: %+v %v", q.Asks, err) + } } diff --git a/cmd/mesh-controller/checks_test.go b/cmd/mesh-controller/checks_test.go index d7e821eb..f804eb06 100644 --- a/cmd/mesh-controller/checks_test.go +++ b/cmd/mesh-controller/checks_test.go @@ -316,3 +316,21 @@ func TestARecheckedHeadsDirectVerdictNamesItsTrunk(t *testing.T) { // A check stopped by its pull request's key moving on says no verdict (issue 974, review 2): an error said for it would name the // live head, and the newer request is the one that judges. A check that failed for any other reason still says its error. + +// C2 (ADR 0513 §4, ADR 0503 §5; issue 1299): a trunk requiring mesh/repo-check has its repository check asked, whether or +// not it also requires mesh/check (hq requires both); a trunk requiring mesh/check alone, or neither, is asked nothing. +func TestARepositoryCheckIsAskedWheneverTheTrunkRequiresIt(t *testing.T) { + for _, c := range []struct { + required []string + asks bool + }{ + {[]string{"mesh/check", "mesh/repo-check"}, true}, + {[]string{"mesh/repo-check"}, true}, + {[]string{"mesh/check"}, false}, + {nil, false}, + } { + if got := asksRepoCheck(c.required); got != c.asks { + t.Fatalf("a trunk requiring %v asks its repository check: %v, want %v", c.required, got, c.asks) + } + } +}