diff --git a/cmd/mesh-controller/batches_test.go b/cmd/mesh-controller/batches_test.go index a5010630..90a5ac8a 100644 --- a/cmd/mesh-controller/batches_test.go +++ b/cmd/mesh-controller/batches_test.go @@ -119,6 +119,24 @@ func everyHeadWaits(t *testing.T) { t.Cleanup(func() { deliveryGroupOf = was }) } +// groupStates stubs mesh-delivery's word on one group: its members and their states, which a test changes as the +// members merge. +func groupStates(t *testing.T, head string, states map[string]string) { + t.Helper() + was := deliveryGroupOf + deliveryGroupOf = func(_ context.Context, _ bool, asked string) ([]groupMember, bool, error) { + if asked != head { + return nil, false, nil + } + var members []groupMember + for repository, state := range states { + members = append(members, groupMember{Repository: repository, State: state}) + } + return members, true, nil + } + t.Cleanup(func() { deliveryGroupOf = was }) +} + // fromHead is a merge from a head branch a test names: merges from one head are members of one delivery group, // related by it (novox/hq ADR 0304 decision 1), so they share a batch whatever else they share. func fromHead(m link.SourceMoved, head string) link.SourceMoved { diff --git a/cmd/mesh-controller/check_siblings.go b/cmd/mesh-controller/check_siblings.go index b2e5626e..df08f3f5 100644 --- a/cmd/mesh-controller/check_siblings.go +++ b/cmd/mesh-controller/check_siblings.go @@ -127,10 +127,15 @@ func siblingsFor(p link.PullUpdated, entries []inventory.Entry, beside map[strin // could not be learned (ADR 0382 §1): a check never waits on mesh-delivery past groupAskWithin and never fails for it, // so a fix to mesh-delivery is still checked without it. func withSiblings(ctx context.Context, request *link.BuildRequest, p link.PullUpdated, entries []inventory.Entry) { + lines, err := groupsAsked(ctx, deliverySeatHeld(entries)) + siblingsFrom(request, p, entries, lines, err) +} + +// siblingsFrom is withSiblings from mesh-delivery's answer already had. +func siblingsFrom(request *link.BuildRequest, p link.PullUpdated, entries []inventory.Entry, lines []groupsLine, err error) { if request.Check == nil { return } - lines, err := groupsAsked(ctx, deliverySeatHeld(entries)) if err != nil { request.Check.SiblingsUnknown = fmt.Sprintf("mesh-delivery did not say the group's pull requests (%v): "+ "the check reads no siblings", err) @@ -138,3 +143,23 @@ func withSiblings(ctx context.Context, request *link.BuildRequest, p link.PullUp } request.Check.Siblings, request.Check.SiblingsUnknown = siblingsFor(p, entries, request.Check.Beside, lines) } + +// composedMember says the pull request is a member of a delivery group whose gate is the composed check's (novox/hq +// ADR 0403 clauses 1 and 7): the group has two members or more and mesh-delivery says so on its line. The group is +// found as siblingsFor finds it, by its head branch name, or, for a recheck that carries none, by the pull request's +// repository and number — not by siblings, which name only what the check reads beside it. +func composedMember(p link.PullUpdated, lines []groupsLine) bool { + own := strings.ToLower(p.Owner + "/" + p.Repo) + for _, l := range lines { + found := p.Head != "" && l.ID == p.Head + for _, m := range l.Details { + if p.Head == "" && p.Number != 0 && m.Number == p.Number && strings.ToLower(m.Repository) == own { + found = true + } + } + if found { + return l.ComposedGate && max(len(l.Details), len(l.Members)) >= 2 + } + } + return false +} diff --git a/cmd/mesh-controller/checks.go b/cmd/mesh-controller/checks.go index d501d49a..e20f7737 100644 --- a/cmd/mesh-controller/checks.go +++ b/cmd/mesh-controller/checks.go @@ -154,6 +154,11 @@ func (f following) PullUpdated(ctx context.Context, p link.PullUpdated) error { return err } scope := pullScope(p, entries, read, edges) + // **A member of a group whose gate is the composed check's runs its repository's check alone** (novox/hq ADR 0403 + // clause 1): mesh-delivery is asked once, and says so on the group's line. Told nothing, the pull request is + // checked alone, gate included (clause 6). + lines, groupsErr := groupsAsked(ctx, deliverySeatHeld(entries)) + composed := groupsErr == nil && composedMember(p, lines) // The change plan of the commit at hand (ADR 0238): what a merge of it would build and send, posted // with the verdict whatever the verdict is. plan := changePlanOf(p.Owner+"/"+p.Repo, p.Base, p.Commit, scope.Reach, entries, func(module string) (inventory.Upgrade, bool) { @@ -162,6 +167,10 @@ func (f following) PullUpdated(ctx context.Context, p link.PullUpdated) error { }) fmt.Print(planText(plan)) direct := directVerdict(p, plan) + if composed { + scope.Modules, scope.Dependents, scope.New, scope.Manifests = nil, nil, nil, nil + composedGate(&direct) + } switch { case !scope.gated() && !scope.Mesh: fmt.Printf("%s/%s#%d (%.8s): %s, and the repository is not the mesh's: said, nothing run\n", @@ -179,8 +188,9 @@ func (f following) PullUpdated(ctx context.Context, p link.PullUpdated) error { return err } request.Check.Plan = &plan + request.Check.Composed = composed // **The group's other open pull requests are read beside it** (novox/hq ADR 0382 §1). - withSiblings(ctx, &request, p, entries) + siblingsFrom(&request, p, entries, lines, groupsErr) seat := buildSeatHeld(ctx) ask, err := askOverOn(seat) if err != nil { @@ -194,6 +204,9 @@ func (f following) PullUpdated(ctx context.Context, p link.PullUpdated) error { // issue 974): the check queue keeps one request a pull request, and a running check watches its key. Nothing is asked // of the forge here (issue 726). what := "its own merge-check.sh alone: " + noModuleTouched + if composed { + what = "its own merge-check.sh alone: the gate is its group's composed check's" + } if scope.gated() { what = fmt.Sprintf("the gate over %s (a merge would build %d module(s) in %d tier(s))", strings.Join(append(append([]string{}, scope.Modules...), prefixedAll("new:", scope.New)...), ", "), @@ -204,6 +217,20 @@ func (f following) PullUpdated(ctx context.Context, p link.PullUpdated) error { return nil } +// composedWords is what a group member's own verdict says of its gate layer (novox/hq ADR 0403 clause 1). +const composedWords = "judged by its group's composed check" + +// gateComposed is a gate layer's verdict when no gate was run for the member alone: never a pass. Mesh-delivery reads it +// as the composed check's to judge, and the forge's holder sets no mesh/merge-gate from it. +const gateComposed = "composed" + +// composedGate says a group member's own verdict has no gate of its own: its gate layer, and the verdict that is +// the gate's, read "composed" in place of a pass. +func composedGate(c *link.Checked) { + c.Verdict, c.Summary = gateComposed, composedWords + c.Gate = &link.CheckLayer{Verdict: gateComposed, Summary: composedWords} +} + // directVerdict is the verdict of a change no build judges — it touches no module, and either the repository is not // the mesh's or the head holds no merge-check.sh. **It names the trunk commit it holds for** (novox/hq ADR 0323 §3): // the base branch's head the forge announced with the head, so the merge tool can weigh it against the trunk's head @@ -518,6 +545,12 @@ func checkedOf(result link.BuildResult) link.Checked { c.Gate = &link.CheckLayer{Verdict: c.Verdict, Summary: c.Summary} c.RepoCheck = &link.CheckLayer{Verdict: c.Verdict, Summary: c.Summary} } + // **A group member's own check has no gate of its own** (novox/hq ADR 0403 clause 1): what ran was the repository's + // check alone, and its gate layer is "composed", never a pass. A failure or an error stays what it was. + if result.Checked != nil && result.Checked.Composed && result.Check != nil && c.Gate != nil && + (c.Gate.Verdict == "pass" || c.Gate.Verdict == "warning") { + composedGate(&c) + } if result.Checked != nil && c.Gate != nil && len(c.Gate.Modules) == 0 { c.Gate.Modules = append(append([]string{}, result.Checked.Modules...), prefixedAll("new:", result.Checked.New)...) } diff --git a/cmd/mesh-controller/composed_member_check_test.go b/cmd/mesh-controller/composed_member_check_test.go new file mode 100644 index 00000000..f5bbc15b --- /dev/null +++ b/cmd/mesh-controller/composed_member_check_test.go @@ -0,0 +1,68 @@ +//go:build integration + +package main + +import ( + "context" + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/catalogue" + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" +) + +// A member of a group whose gate is the composed check's is asked of the build seat with no module: its repository's +// check alone (novox/hq ADR 0403 clause 1). With no word from mesh-delivery, or one that does not say the gate is +// composed, it is checked alone with its gate (clause 6). + +func askedForMember(t *testing.T, answer []groupsLine, answerErr error) link.CheckRequest { + t.Helper() + open := aMesh(t) + ctx := t.Context() + js, _ := aBuildQueue(t) + servingBus.Store(js) + t.Cleanup(func() { servingBus.Store(nil) }) + if err := open.inventory.RegisterModule(ctx, catalogue.Manifest{Module: "mesh-controller", Version: "1"}, + inventory.Source{Repository: "novox/mesh-controller", Ref: "main", BuiltFrom: "c0", Head: "c0"}); err != nil { + t.Fatal(err) + } + was := groupsAsked + groupsAsked = func(context.Context, bool) ([]groupsLine, error) { return answer, answerErr } + t.Cleanup(func() { groupsAsked = was }) + p := link.PullUpdated{Owner: "novox", Repo: "mesh-controller", Number: 346, Base: "main", Head: "feat/x", Commit: headController, + Paths: []string{"main.go"}, ModuleDirsSaid: true, MergeCheck: true, MergeCheckSaid: true} + if err := (following{open: open, oneShot: true}).PullUpdated(ctx, p); err != nil { + t.Fatal(err) + } + q, err := link.ReadQueue(ctx, js, link.TheBuildMachine) + if err != nil || len(q.Asks) != 1 { + t.Fatalf("asks %d: %v", len(q.Asks), err) + } + return *q.Asks[0].Request.Check +} + +func TestAComposedMembersCheckIsItsRepositoryChecksAlone(t *testing.T) { + line := group("feat/x", member("novox/mesh-controller", 346, headController, "main", true), member("novox/mesh-catalog", 350, headHost, "main", true)) + line.ComposedGate = true + c := askedForMember(t, []groupsLine{line}, nil) + if len(c.Modules) != 0 || len(c.New) != 0 || len(c.Dependents) != 0 || !c.Composed { + t.Fatalf("the member's check is asked as %+v; want no module and composed", c) + } +} + +func TestAMemberOfAGroupOfAnOlderMeshDeliveryKeepsItsOwnGate(t *testing.T) { + line := group("feat/x", member("novox/mesh-controller", 346, headController, "main", true), member("novox/mesh-catalog", 350, headHost, "main", true)) + c := askedForMember(t, []groupsLine{line}, nil) + if !strings.Contains(strings.Join(c.Modules, ","), "mesh-controller") || c.Composed { + t.Fatalf("an older mesh-delivery's member is asked as %+v; want its own gate", c) + } +} + +func TestAMemberWhoseGroupCannotBeLearnedIsCheckedAloneWithItsGate(t *testing.T) { + c := askedForMember(t, nil, context.DeadlineExceeded) + if !strings.Contains(strings.Join(c.Modules, ","), "mesh-controller") || c.Composed || + !strings.Contains(c.SiblingsUnknown, "reads no siblings") { + t.Fatalf("the member is asked as %+v; want its gate and why its siblings are unknown", c) + } +} diff --git a/cmd/mesh-controller/composed_verdict_test.go b/cmd/mesh-controller/composed_verdict_test.go new file mode 100644 index 00000000..f85b9850 --- /dev/null +++ b/cmd/mesh-controller/composed_verdict_test.go @@ -0,0 +1,136 @@ +package main + +import ( + "context" + "encoding/json" + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/catalogue" + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" +) + +// A delivery group's gate is the composed check's, and its members' own checks run their repository's check alone +// (novox/hq ADR 0403 clauses 1 and 7). Pure: no store, no bus. + +// A member of a group whose line says composed_gate is a composed member; one whose group says nothing of it (an older +// mesh-delivery), one of a group of a single pull request and a lone pull request are not. +func TestAMemberOfAComposedGroupIsKnownByTheGroupsLine(t *testing.T) { + two := func(composed bool) groupsLine { + l := group("feat/x", member("novox/mesh-host", 114, headHost, "main", true), member("novox/mesh-controller", 346, headController, "main", true)) + l.ComposedGate = composed + return l + } + p := link.PullUpdated{Owner: "novox", Repo: "mesh-host", Number: 114, Base: "main", Head: "feat/x", Commit: headHost} + recheck := link.PullUpdated{Owner: "novox", Repo: "mesh-host", Number: 114, Base: "main", Commit: headHost} + for name, c := range map[string]struct { + p link.PullUpdated + lines []groupsLine + want bool + }{ + "composed": {p, []groupsLine{two(true)}, true}, + "a recheck, by its number": {recheck, []groupsLine{two(true)}, true}, + "an older mesh-delivery": {p, []groupsLine{two(false)}, false}, + "another group": {p, []groupsLine{group("feat/y", member("novox/mesh-host", 1, headHost, "main", true))}, false}, + "no group": {p, nil, false}, + } { + if got := composedMember(c.p, c.lines); got != c.want { + t.Errorf("%s: composed %v, want %v", name, got, c.want) + } + } + alone := group("feat/x", member("novox/mesh-host", 114, headHost, "main", true)) + alone.ComposedGate = true + if composedMember(p, []groupsLine{alone}) { + t.Error("a group of a single pull request is composed") + } +} + +// A composed member's verdict never says its gate passed: the gate layer, and the verdict that is the gate's, read +// "composed"; a failure or an error stays what it was. +func TestAComposedMembersVerdictHasNoPassingGate(t *testing.T) { + asked := &link.CheckRequest{Owner: "novox", Repo: "mesh-host", Number: 114, Composed: true} + pass := link.BuildResult{ID: "b", Ref: headHost, Checked: asked, Check: &link.CheckOutcome{Verdict: "pass", Summary: "no module touched", + Gate: &link.CheckLayer{Verdict: "pass", Summary: "no module touched"}, RepoCheck: &link.CheckLayer{Verdict: "pass", Summary: "ok"}}} + c := checkedOf(pass) + if c.Gate == nil || c.Gate.Verdict != "composed" || c.Verdict != "composed" || c.RepoCheck == nil || c.RepoCheck.Verdict != "pass" { + t.Fatalf("the member's verdict says %+v gate %+v", c, c.Gate) + } + asked.Composed = false + if c := checkedOf(pass); c.Gate.Verdict != "pass" || c.Verdict != "pass" { + t.Fatalf("a member of no composed group has its own gate: %+v", c.Gate) + } + asked.Composed = true + failing := pass + failing.Check = &link.CheckOutcome{Verdict: "fail", Summary: "conflict", Gate: &link.CheckLayer{Verdict: "fail", Summary: "conflict"}, + RepoCheck: &link.CheckLayer{Verdict: "fail", Summary: "conflict"}} + if c := checkedOf(failing); c.Gate.Verdict != "fail" || c.Verdict != "fail" { + t.Fatalf("a failure was read as composed: %+v", c.Gate) + } + broken := link.BuildResult{ID: "b", Ref: headHost, Checked: asked, Failed: "the facts snapshot cannot be read"} + if c := checkedOf(broken); c.Verdict != "error" || c.Gate.Verdict != "error" { + t.Fatalf("an error was read as composed: %+v", c) + } +} + +// The composed verdict on the bus carries every member, each with its trunk commit, for the forge's holder to set each +// member's mesh/merge-gate from it. +func TestTheComposedVerdictCarriesEveryMember(t *testing.T) { + asked := &link.CheckRequest{Owner: "novox", Repo: "mesh-host", Number: 114, Group: "feat/x", + Members: []link.GroupMember{{Owner: "novox", Repo: "mesh-controller", Number: 346, Ref: headController}}} + c := checkedOf(link.BuildResult{ID: "g", Ref: headHost, Checked: asked, Check: &link.CheckOutcome{Verdict: "pass", Summary: "ok", + Trunk: "aaaaaaaaaaaa", Members: []link.CheckedMember{{Owner: "novox", Repo: "mesh-controller", Commit: headController[:12], Trunk: "bbbbbbbbbbbb"}}}}) + body, _ := json.Marshal(c) + for _, want := range []string{`"group":"feat/x"`, `"members":[{"owner":"novox","repo":"mesh-host","number":114,"commit":"` + headHost + `","trunk":"aaaaaaaaaaaa"}`, + `{"owner":"novox","repo":"mesh-controller","number":346,"commit":"` + headController + `","trunk":"bbbbbbbbbbbb"}`} { + if !strings.Contains(string(body), want) { + t.Fatalf("the composed verdict lacks %s:\n%s", want, body) + } + } +} + +// Members whose manifests order their modules in a cycle fail the composed check, naming every module and edge, and no +// ask is made of the build seat. +func TestAGroupInACycleFailsItsComposedCheckWithoutABuild(t *testing.T) { + members := []orderMember{ + {ID: "novox/x@" + headHost[:12], Repository: "novox/x", Base: "main", Head: headHost, Number: 7, Paths: []string{"module.json"}}, + {ID: "novox/y@" + headController[:12], Repository: "novox/y", Base: "main", Head: headController, Number: 9, Paths: []string{"module.json"}}, + } + reach := map[string]orderReach{members[0].ID: {Moved: []string{"mx"}}, members[1].ID: {Moved: []string{"my"}}} + was, wasParent := manifestAt, parentOf + t.Cleanup(func() { manifestAt, parentOf = was, wasParent }) + parentOf = func(context.Context, string, string, string) (string, error) { return "", nil } + manifestAt = func(_ context.Context, _, repo, _, ref string) (catalogue.Manifest, bool, error) { + if ref == "main" { + return catalogue.Manifest{}, false, nil // what the pull requests merge into holds no such manifest + } + if repo == "x" { + return catalogue.Manifest{Module: "mx", DefinesSeats: []catalogue.SeatDeclaration{{Name: "s", Serves: []catalogue.Verb{{Name: "a"}}}}, + Invokes: []string{"seat:s.b"}}, true, nil + } + return catalogue.Manifest{Module: "my", DefinesSeats: []catalogue.SeatDeclaration{{Name: "s", Serves: []catalogue.Verb{{Name: "b"}}}}, + Invokes: []string{"seat:s.a"}}, true, nil + } + c, cyclic, err := cycleVerdict(context.Background(), "feat/cycle", members, reach, []inventory.Edge(nil)) + if err != nil || !cyclic { + t.Fatalf("no cycle found: %v", err) + } + if c.Verdict != "fail" || c.Group != "feat/cycle" || len(c.Members) != 2 || c.Gate == nil || c.Gate.Verdict != "fail" { + t.Fatalf("the verdict is %+v", c) + } + for _, w := range []string{"mx", "my", "s.a", "s.b"} { + if !strings.Contains(c.Summary, w) { + t.Errorf("the cycle does not name %s: %s", w, c.Summary) + } + } + // Members with no edge between them are no cycle. + manifestsReadMu.Lock() + clear(manifestsRead) + manifestsReadMu.Unlock() + manifestAt = func(context.Context, string, string, string, string) (catalogue.Manifest, bool, error) { + return catalogue.Manifest{}, false, nil + } + if _, cyclic, err := cycleVerdict(context.Background(), "feat/cycle", members, reach, nil); err != nil || cyclic { + t.Fatalf("a cycle where there is none: %v %v", cyclic, err) + } +} diff --git a/cmd/mesh-controller/delivery.go b/cmd/mesh-controller/delivery.go index 33459102..3fc13979 100644 --- a/cmd/mesh-controller/delivery.go +++ b/cmd/mesh-controller/delivery.go @@ -1014,6 +1014,43 @@ func theGraph(ctx context.Context, inv *inventory.Inventory) ([]inventory.Entry, return entries, read, edges, nil } +// cycleVerdict is the failing verdict of a group whose modules are ordered in a cycle, and whether there is one: the +// group's composed check, said without a build. The manifests' change is read from the forge; an answer it cannot give +// is an error, never read as no cycle. +func cycleVerdict(ctx context.Context, group string, members []orderMember, reach map[string]orderReach, + edges []inventory.Edge) (link.Checked, bool, error) { + changes, err := changesOf(ctx, changeSourcesOf(members)) + if err != nil { + return link.Checked{}, false, fmt.Errorf("what the group's members say of their manifests cannot be read: %w", err) + } + var moved []string + for _, m := range members { + moved = appendNew(moved, reach[m.ID].Moved...) + } + said := orderEdges(moved, changes, edges).CycleSaid() + if said == "" { + return link.Checked{}, false, nil + } + first := members[0].pull() + c := link.Checked{ID: link.NewBuildID(time.Now()), Owner: first.Owner, Repo: first.Repo, Number: first.Number, + Commit: first.Commit, Verdict: "fail", Summary: said, Report: said, Group: group, + Gate: &link.CheckLayer{Verdict: "fail", Summary: said}} + for _, m := range members { + p := m.pull() + c.Members = append(c.Members, link.CheckedMember{Owner: p.Owner, Repo: p.Repo, Number: p.Number, Commit: p.Commit, + Trunk: twelve(p.BaseSHA)}) + } + return c, true, nil +} + +// twelve is a commit as twelve characters. +func twelve(c string) string { + if len(c) > 12 { + return c[:12] + } + return c +} + // groupPrimary is the head a group's composed check is run from: the one moving the controller, which then // judges the group by itself; else the one moving the node-engine, whose validator judges; else the first. func groupPrimary(members []orderMember, reach map[string]orderReach) int { @@ -1036,6 +1073,14 @@ func askGroupCheck(ctx context.Context, open *stores, group string, members []or return "", err } reach := reachOfMembers(members, entries, read, edges) + // **A cycle of edges fails the group's composed check, naming every module and edge** (novox/hq ADR 0419 clause 6), + // before the build seat is asked: no composition of the heads has an order to be sent in. + if failing, cyclic, err := cycleVerdict(ctx, group, members, reach, edges); err != nil { + return "", err + } else if cyclic { + sayChecked(ctx, failing) + return failing.ID, nil + } primary := groupPrimary(members, reach) p := members[primary].pull() scope := pullScope(p, entries, read, edges) diff --git a/cmd/mesh-controller/group_one_batch_test.go b/cmd/mesh-controller/group_one_batch_test.go index e792e40e..a4a55025 100644 --- a/cmd/mesh-controller/group_one_batch_test.go +++ b/cmd/mesh-controller/group_one_batch_test.go @@ -1,7 +1,8 @@ +//go:build integration + package main import ( - "context" "strings" "testing" "time" @@ -13,24 +14,6 @@ import ( // member still to merge does not close by the merge window, however long that member takes, and a group with a // member of each kind is one batch whatever order its members are heard in. -// groupStates stubs mesh-delivery's word on one group: its members and their states, which a test changes as the -// members merge. -func groupStates(t *testing.T, head string, states map[string]string) { - t.Helper() - was := deliveryGroupOf - deliveryGroupOf = func(_ context.Context, _ bool, asked string) ([]groupMember, bool, error) { - if asked != head { - return nil, false, nil - } - var members []groupMember - for repository, state := range states { - members = append(members, groupMember{Repository: repository, State: state}) - } - return members, true, nil - } - t.Cleanup(func() { deliveryGroupOf = was }) -} - func TestAGroupsBatchWaitsForItsLastMemberHoweverLongItTakes(t *testing.T) { open := windowed(t) asksWithPaths(t) diff --git a/cmd/mesh-controller/group_wait.go b/cmd/mesh-controller/group_wait.go index b2384cf8..c8a62f3f 100644 --- a/cmd/mesh-controller/group_wait.go +++ b/cmd/mesh-controller/group_wait.go @@ -44,6 +44,10 @@ type groupsLine struct { ID string `json:"id"` Members []string `json:"members"` Details []memberLine `json:"member_details,omitempty"` + // ComposedGate says the group's gate is the composed check's (novox/hq ADR 0403): a member's own check runs the + // repository's check alone. Said by a mesh-delivery that merges a group in one pass; absent from an older one, whose + // members keep their own gate. + ComposedGate bool `json:"composed_gate,omitempty"` } // memberLine is one member of a group, as `groups` says it beside its text. diff --git a/cmd/mesh-controller/non_trunk_merge_test.go b/cmd/mesh-controller/non_trunk_merge_test.go index 3e7c190d..be0ace88 100644 --- a/cmd/mesh-controller/non_trunk_merge_test.go +++ b/cmd/mesh-controller/non_trunk_merge_test.go @@ -1,3 +1,5 @@ +//go:build integration + package main import ( diff --git a/cmd/mesh-controller/one_group_one_plan_test.go b/cmd/mesh-controller/one_group_one_plan_test.go new file mode 100644 index 00000000..84c7af6c --- /dev/null +++ b/cmd/mesh-controller/one_group_one_plan_test.go @@ -0,0 +1,177 @@ +//go:build integration + +package main + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/novox/mesh-controller/internal/catalogue" + "github.com/novox/mesh-controller/internal/inventory" +) + +// Acceptance 1 of design 47, the controller's part: a feature across the controller and the catalogue with a new seat +// verb is delivered as one group, with no `after:` line — its merges are one batch, each module is built once and sent +// once, its commits are carried by that one plan, and the plan names the edge that placed each tier. The Approve and +// the one-pass merge are mesh-delivery's. + +// manifestsHeld stubs the forge for a test: the manifest each (repository, path, commit) holds, and none anywhere else. +func manifestsHeld(t *testing.T, held map[[3]string]catalogue.Manifest) { + t.Helper() + was, wasParent := manifestAt, parentOf + t.Cleanup(func() { manifestAt, parentOf = was, wasParent }) + manifestsReadMu.Lock() + clear(manifestsRead) + manifestsReadMu.Unlock() + parentOf = func(context.Context, string, string, string) (string, error) { return "", nil } + manifestAt = func(_ context.Context, _, repo, file, ref string) (catalogue.Manifest, bool, error) { + m, ok := held[[3]string{repo, file, ref}] + return m, ok, nil + } +} + +func TestAFeatureWithANewSeatVerbIsOneBatchAndOnePlanInOrderWithNoAfterLine(t *testing.T) { + for name, controllerFirst := range map[string]bool{"controller_first": true, "catalogue_first": false} { + t.Run(name, func(t *testing.T) { + open := windowed(t) + asked := asksWithPaths(t) + ctx := t.Context() + const head = "feat/one-group-one-plan" + // mesh-delivery holds its seat, so a catalogue plan would wait for its word; this plan carries the controller. + if err := open.inventory.RegisterModule(ctx, catalogue.Manifest{Module: "mesh-delivery", Version: "1", + Claims: []catalogue.Claim{{Name: catalogue.DeliverySeat, Scope: catalogue.ScopeMesh}}}, + inventory.Source{Repository: "novox/mesh-catalog", Seat: "git", Path: "modules/mesh-delivery", Ref: "main", + BuiltFrom: "c0", Head: "c0"}); err != nil { + t.Fatal(err) + } + if _, err := open.inventory.Assign(ctx, "anchor", "mesh-delivery"); err != nil { + t.Fatal(err) + } + controller := fromHead(repoMerge("mesh-controller", "k1", t0), head) + controller.Paths = []string{"module.json", "cmd/mesh-controller/main.go"} + catalog := fromHead(catalogueMerge("cat1aaaaaaaa", "app", t0), head) + manifestsHeld(t, map[[3]string]catalogue.Manifest{ + {"mesh-controller", "module.json", "k1"}: {Module: "mesh-controller", + DefinesSeats: []catalogue.SeatDeclaration{{Name: "seat", Serves: []catalogue.Verb{{Name: "verb"}}}}}, + {"mesh-catalog", "modules/app/module.json", "cat1aaaaaaaa"}: {Module: "app", Invokes: []string{"seat:seat.verb"}}, + }) + states := map[string]string{"novox/mesh-controller": "ready", "novox/mesh-catalog": "ready"} + groupStates(t, head, states) + first, second := controller, catalog + if !controllerFirst { + first, second = catalog, controller + } + // The first member merges; the group waits for the second, however long it takes. + hear(t, open, first, t0) + cutAt(t, open, t0.Add(30*time.Minute)) + if ws, bs := walks(t, open); len(ws) != 0 || len(bs) != 1 { + t.Fatalf("a member of a group alone: %d walk(s), %d batch(es)", len(ws), len(bs)) + } + states["novox/"+second.Repo] = "merged" + second.MergedAt = t0.Add(30 * time.Minute).Format(time.RFC3339Nano) + hear(t, open, second, t0.Add(30*time.Minute)) + cutAt(t, open, t0.Add(30*time.Minute+90*time.Second)) + ws, bs := walks(t, open) + if len(ws) != 1 || len(bs) != 0 { + t.Fatalf("after the window: %d walk(s), %d batch(es); want exactly one plan", len(ws), len(bs)) + } + plan := ws[0] + if plan.CommitOf("novox/mesh-controller") != "k1" || plan.CommitOf("novox/mesh-catalog") != "cat1aaaaaaaa" { + t.Fatalf("the plan carries %+v", plan.Commits) + } + if len(plan.Tiers) != 2 || strings.Join(plan.Tiers[0], ",") != "mesh-controller" || strings.Join(plan.Tiers[1], ",") != "app" { + t.Fatalf("the plan's tiers are %v; want the controller, then the caller of its verb", plan.Tiers) + } + if placed := plan.Modules["app"].Placed; !strings.Contains(placed, "verb before caller: seat.verb") || + !strings.Contains(placed, "after mesh-controller") { + t.Fatalf("the plan does not name the edge that placed the caller: %q", placed) + } + if plan.Modules["mesh-controller"].Placed != "" { + t.Fatalf("the first tier is placed by %q", plan.Modules["mesh-controller"].Placed) + } + if plan.Delivery != nil && (plan.Delivery.Awaits != "" || plan.Waiting()) { + t.Fatalf("a plan carrying the controller waits for %q's word", plan.Delivery.Awaits) + } + if plan.Modules["mesh-controller"].Build == "" || len(*asked) != 1 { + t.Fatalf("the first tier is asked once: build %q, %d ask(s) %v", plan.Modules["mesh-controller"].Build, len(*asked), *asked) + } + // Every merge is answered by that one plan. + for _, repository := range []string{"novox/mesh-controller", "novox/mesh-catalog"} { + merges, err := open.inventory.MergesOf(ctx, plan.ID) + if err != nil || len(merges) != 2 { + t.Fatalf("the plan answers %d merge(s) of %s: %v", len(merges), repository, err) + } + } + }) + } +} + +// Issue 1049: a move delivered half before its second merge. The controller adds a module the catalogue removes, in one +// group: the merges are one batch and one plan in either order, the module is built once and from the controller's +// repository, and the catalogue's removal does not forget it. +func TestAMoveOfAModuleAcrossTwoRepositoriesIsOnePlanAndBuildsItOnceFromTheNewOne(t *testing.T) { + for name, controllerFirst := range map[string]bool{"controller_first": true, "catalogue_first": false} { + t.Run(name, func(t *testing.T) { + open := windowed(t) + asked := asksWithPaths(t) + ctx := t.Context() + const head = "feat/whole-modules" + // The module runs on a machine from the catalogue's repository, so its removal there cannot forget it. + if err := open.inventory.RegisterModule(ctx, catalogue.Manifest{Module: "route-proxy", Version: "1"}, + inventory.Source{Repository: "novox/mesh-catalog", Seat: "git", Path: "modules/route-proxy", Ref: "main", + BuiltFrom: "c0", Head: "c0"}); err != nil { + t.Fatal(err) + } + if _, err := open.inventory.Assign(ctx, "anchor", "route-proxy"); err != nil { + t.Fatal(err) + } + manifestsHeld(t, nil) + controller := fromHead(repoMerge("mesh-controller", "k1", t0), head) + controller.Paths = []string{"modules/route-proxy/module.json", "modules/route-proxy/main.go"} + controller.ModuleDirs, controller.ModuleDirsSaid = []string{"modules/route-proxy"}, true + catalog := fromHead(catalogueMerge("cat1aaaaaaaa", "route-proxy", t0), head) + catalog.Removed = []string{"modules/route-proxy/module.json"} + catalog.ModuleDirs, catalog.ModuleDirsSaid = nil, true + states := map[string]string{"novox/mesh-controller": "ready", "novox/mesh-catalog": "ready"} + groupStates(t, head, states) + first, second := controller, catalog + if !controllerFirst { + first, second = catalog, controller + } + hear(t, open, first, t0) + states["novox/"+second.Repo] = "merged" + second.MergedAt = t0.Add(20 * time.Second).Format(time.RFC3339Nano) + hear(t, open, second, t0.Add(20*time.Second)) + cutAt(t, open, t0.Add(20*time.Second+90*time.Second)) + ws, bs := walks(t, open) + if len(ws) != 1 || len(bs) != 0 { + t.Fatalf("a move: %d walk(s), %d batch(es); want exactly one plan", len(ws), len(bs)) + } + var fromController int + for _, a := range *asked { + if strings.Contains(a[0], "mesh-controller") && a[1] == "modules/route-proxy" { + fromController++ + } + if strings.Contains(a[0], "mesh-catalog") && a[1] == "modules/route-proxy" { + t.Errorf("route-proxy was built from the repository it left: %v", a) + } + } + if fromController != 1 { + t.Fatalf("route-proxy was built %d time(s) from the controller's repository: %v", fromController, *asked) + } + entries, err := open.inventory.Catalogued(ctx) + if err != nil { + t.Fatal(err) + } + kept := false + for _, e := range entries { + kept = kept || e.Manifest.Module == "route-proxy" + } + if !kept { + t.Fatal("the catalogue's removal forgot route-proxy, which a machine runs") + } + }) + } +} diff --git a/internal/catalogue/delivery_seat.go b/internal/catalogue/delivery_seat.go index 19a2d51b..840c4916 100644 --- a/internal/catalogue/delivery_seat.go +++ b/internal/catalogue/delivery_seat.go @@ -84,15 +84,6 @@ func deliveryVerbs() []Verb { "dry": "\"true\": what it would retire, changing nothing"}, []string{"why"}, "dry"), // Added after the seat's holder shipped (novox/hq issue 313): optional until mesh-delivery serves it. Optional: true}, - {Name: "mark-ready", Description: "A reviewer's word that a delivery's head is approved: the holder puts the " + - "label mesh/ready on its pull request and then writes \"ready at \", the order the forge counts, " + - "as the mesh's own forge account, naming who reviewed it and why. Refused when the delivery's commit is " + - "no longer its pull request's head. mesh-delivery then merges it once its checks pass (novox/hq ADR 0364).", - Input: schema(map[string]string{"id": "the delivery's id (owner/repository@commit): the head reviewed", - "reviewed-by": "who reviewed it, as who@node", "why": "the review's verdict in a line"}, - []string{"id", "reviewed-by", "why"}), - // Added after the seat's holder shipped (novox/hq issue 913): optional until mesh-delivery serves it. - Optional: true}, } } diff --git a/internal/catalogue/delivery_seat_test.go b/internal/catalogue/delivery_seat_test.go index 783ec06e..5fe8daf2 100644 --- a/internal/catalogue/delivery_seat_test.go +++ b/internal/catalogue/delivery_seat_test.go @@ -133,32 +133,3 @@ func TestTheDeliverySeatPromisesTimesOptionally(t *testing.T) { t.Fatalf("a holder serving times: %v", err) } } - -// The seat marks a reviewed head ready (novox/hq issue 913): `mark-ready`, with the delivery's id, who reviewed it -// and why, so a reviewer marks a pull request through one audited verb, label and comment in the order the forge -// counts, instead of two raw forge calls. Added after the holder shipped, it is optional. -func TestTheDeliverySeatPromisesMarkReadyOptionally(t *testing.T) { - seat, _ := SeatNamed(DeliverySeat) - var mark *Verb - for i := range seat.Serves { - if seat.Serves[i].Name == "mark-ready" { - mark = &seat.Serves[i] - } - } - if mark == nil || !mark.Optional { - t.Fatalf("the delivery seat promises %v, mark-ready optionally", VerbNames(seat.Serves)) - } - if req, _ := mark.Input["required"].([]string); !reflect.DeepEqual(req, []string{"id", "reviewed-by", "why"}) { - t.Errorf("mark-ready requires %v, wanted id, reviewed-by and why", req) - } - var without []string - for _, v := range VerbNames(seat.Serves) { - if v != "mark-ready" { - without = append(without, v) - } - } - m := Manifest{Module: "mesh-delivery", Claims: []Claim{{Name: DeliverySeat, Scope: ScopeMesh, Serves: without}}} - if err := CanHold(m, seat); err != nil { - t.Fatalf("a holder without mark-ready yet: %v", err) - } -} diff --git a/internal/link/build.go b/internal/link/build.go index 6ba3231f..ffccf720 100644 --- a/internal/link/build.go +++ b/internal/link/build.go @@ -150,6 +150,9 @@ type CheckRequest struct { Trunk string `json:"trunk,omitempty"` // Ready says the pull request is marked ready (ADR 0406 §3): its check is queued before every other check. Ready bool `json:"ready,omitempty"` + // Composed says the pull request is a member of a delivery group whose gate is the composed check's (novox/hq ADR + // 0403 clause 1): this check runs the repository's own merge-check.sh alone, and its gate layer is not run. + Composed bool `json:"composed,omitempty"` // Paths are the files the pull request changes, for the width of its rebuild. Paths []string `json:"paths,omitempty"` // Beside are the repositories the check reads next to this one, each cloned at the ref given — the diff --git a/internal/link/events.go b/internal/link/events.go index 883ef587..84b41651 100644 --- a/internal/link/events.go +++ b/internal/link/events.go @@ -294,8 +294,9 @@ type Checked struct { // and send, posted with the verdict. Plan *ChangePlan `json:"plan,omitempty"` // Group is set on a delivery group's composed check (novox/hq ADR 0239): every member's head composed - // together as one future state. Members are the heads judged. A forge's holder sets no merge-gate - // status from it — the group's verdict is its owner's, mesh-delivery, to say on each member's head. + // together as one future state. Members are the heads judged, each with the trunk commit it was merged into. **The + // forge's holder sets every member's mesh/merge-gate from it** (novox/hq ADR 0403 clauses 1 and 5): the group's + // verdict is each member's gate, and a member's own verdict, whose gate layer is "composed", sets none. Group string `json:"group,omitempty"` Members []CheckedMember `json:"members,omitempty"` // Trunk is the trunk commit the head was merged into and judged as, twelve characters (novox/hq ADR 0323