diff --git a/modules/gitea/index.ts b/modules/gitea/index.ts index 9b9bd491..8ab610fd 100644 --- a/modules/gitea/index.ts +++ b/modules/gitea/index.ts @@ -42,7 +42,7 @@ import { commitStatusTool } from "./delivery.js"; import { announce, keepWatermark, readWatermark } from "./announce.js"; import { mergeTool } from "./merge.js"; import { GiteaAdmin, GiteaClient, movedSince } from "./client.js"; -import { CHECK_CONTEXT, REPO_CHECK_CONTEXT, commentFor, headsToAnnounce, statusesFor, type Announced, type Checked, +import { CHECK_CONTEXT, REPO_CHECK_CONTEXT, commentFor, groupGateStatus, headsToAnnounce, statusesFor, type Announced, type Checked, repoCheckFacts, reposToLook } from "./pulls.js"; import { GateWriter, clonedAs, type BuildStarted } from "./gate.js"; import { HOOK_PATH, HOOK_PORT, Pace, claimHead, ensureSystemHook, eventsTool, keptSecret, receiver, type ForgeEvent } from "./webhook.js"; @@ -361,8 +361,9 @@ async function setVerdict(client: GiteaClient, event: { body: unknown }): Promis { took: c.took, at: new Date().toISOString(), head: c.commit }); } if (c.group) { - // A delivery group's composed check (novox/hq ADR 0239): its verdict is the group owner's to say, on each - // member's head, as mesh/delivery-group — never this head's merge gate. + // A delivery group's composed check (novox/hq ADR 0239, ADR 0403 §1): its verdict is each member's merge gate, at the + // head it read; the group's own status, mesh/delivery-group, is its owner's to say. + await setGroupVerdict(client, c); return; } if (!c.owner || !c.repo || !c.commit || !c.verdict) { @@ -417,6 +418,30 @@ async function setVerdict(client: GiteaClient, event: { body: unknown }): Promis console.log(`[gitea] ${c.owner}/${c.repo}#${c.number ?? "?"} at ${c.commit.slice(0, 8)}: merge check ${c.verdict}`); } +/** **The composed verdict sets each member's gate** (novox/hq ADR 0403 §1, issue 978), in each head's own turn of the gate + * writer (gate.ts), judged against the trunk as a member's own verdict is: one that names a trunk commit that is no longer + * the trunk's head is out of date when it lands, and said so, pending. */ +async function setGroupVerdict(client: GiteaClient, c: Checked): Promise { + for (const m of c.members ?? []) { + if (!m.owner || !m.repo || !m.commit || !c.verdict) { + console.error(`[gitea] a composed verdict of ${c.group} names a member with no repository, commit or verdict; ignored`); + continue; + } + await gates.verdict(m.owner, m.repo, m.commit, async () => { + const pull = m.number ? await client.getPullRequest(m.owner, m.repo, m.number).catch(() => undefined) : undefined; + const target = pull?.html_url || undefined; + let status = groupGateStatus(c, m); + if (m.trunk && pull?.base) { + const head = await client.branchHead(m.owner, m.repo, pull.base).catch(() => undefined); + if (head && !sameCommit(m.trunk, head)) status = { state: "pending", context: CHECK_CONTEXT, description: outOfDateWords(m.trunk, head) }; + } + await client.setCommitStatus(m.owner, m.repo, m.commit, target ? { ...status, target_url: target } : status); + console.log(`[gitea] ${m.owner}/${m.repo}#${m.number ?? "?"} at ${m.commit.slice(0, 8)}: the group ${c.group}'s composed check ${c.verdict}, ` + + `its merge gate ${status.state}`); + }, c.id); + } +} + // **This module is the one writer of a head's mesh/merge-gate** (novox/hq issue 446): the announcement's pending words, // where the check is when a build machine takes it, and the verdict — each head's writes in one chain, the verdict last // (gate.ts). A second writer elsewhere raced the verdict and could leave a pass reading pending. diff --git a/modules/gitea/pulls.ts b/modules/gitea/pulls.ts index c6c92667..6dbf2040 100644 --- a/modules/gitea/pulls.ts +++ b/modules/gitea/pulls.ts @@ -58,8 +58,11 @@ export interface Checked { "repo-check"?: Layer; /** The change plan of the commit checked (novox/hq ADR 0238): what a merge of it would build and send. */ plan?: ChangePlan; - /** Set on a delivery group's composed check (novox/hq ADR 0239): not this head's merge gate. */ + /** Set on a delivery group's composed check (novox/hq ADR 0239): its verdict is each member's merge gate (ADR 0403 §1, + * issue 978), set in setGroupVerdict, never read as this head's own. */ group?: string; + /** The members a composed check read, each at its head and the trunk commit it was merged onto (ADR 0323 §6). */ + members?: { owner: string; repo: string; number?: number; commit: string; trunk?: string }[]; /** The trunk commit the head was merged into and judged as, twelve characters (novox/hq ADR 0323 §3); absent from a * controller whose build seat judged the head alone. Beside are the other repositories' commits the check read, * by directory; took how long it ran. */ @@ -234,6 +237,17 @@ export function statusFor(c: Checked): CommitStatus { return judgedFirst(c, { state: stateOf(gate.verdict), context: CHECK_CONTEXT, description }); } +/** + * **A member's gate from its group's composed verdict** (novox/hq ADR 0403 §1, issue 978): the composed check, which + * composed every machine with all the members' heads, sets each member's `mesh/merge-gate` at the head it read. The + * description names what it judged first, as a member's own gate does (ADR 0323 §3), and the group. + */ +export function groupGateStatus(c: Checked, m: NonNullable[number]): CommitStatus { + const said = described(c.verdict, `group ${c.group} composed: ${c.summary}`); + const first = m.trunk ? `head ${m.commit.slice(0, 12)} merged into trunk ${m.trunk.slice(0, 12)} — ` : ""; + return { state: stateOf(c.verdict), context: CHECK_CONTEXT, description: clipped(first + said) }; +} + /** Every status a verdict sets: the gate's, and the repository's own check's when it was said. */ export function statusesFor(c: Checked, facts: RepoCheckFacts = {}): CommitStatus[] { const out = [statusFor(c)]; diff --git a/modules/gitea/test/pulls.test.ts b/modules/gitea/test/pulls.test.ts index e1396125..dc775e11 100644 --- a/modules/gitea/test/pulls.test.ts +++ b/modules/gitea/test/pulls.test.ts @@ -154,3 +154,21 @@ test("a head announced is pending in the build queue, since the local time it wa assert.equal(pendingWords(new Date(2026, 9, 11, 3, 8, 50)), "waiting in the build queue since 03:08"); assert.equal(pendingWords(new Date(2026, 9, 11, 14, 0, 0)), "waiting in the build queue since 14:00"); }); + +// **A group's composed verdict is each member's merge gate** (novox/hq ADR 0403 §1, issue 978): set at the head the +// composed check read, naming the trunk commit it was merged onto, the group, and what the check said. +test("a composed verdict sets every member's gate at its own head, naming its trunk and the group", async () => { + const { groupGateStatus, CHECK_CONTEXT } = await import("../pulls.ts"); + const c = { owner: "", repo: "", commit: "", id: "build-1", group: "feat/x", verdict: "pass", summary: "both compose", + members: [{ owner: "novox", repo: "mesh-controller", number: 1, commit: "c".repeat(40), trunk: "a".repeat(40) }, + { owner: "novox", repo: "mesh-catalog", number: 2, commit: "d".repeat(40) }] }; + const [first, second] = c.members.map((m) => groupGateStatus(c, m)); + assert.equal(first.context, CHECK_CONTEXT); + assert.equal(first.state, "success"); + assert.match(first.description, /^head cccccccccccc merged into trunk aaaaaaaaaaaa — pass: group feat\/x composed: both compose/); + assert.equal(second.state, "success"); + assert.match(second.description, /^pass: group feat\/x composed: both compose/, "no trunk named, none said"); + // A composed check that failed is every member's failure, never a success. + assert.equal(groupGateStatus({ ...c, verdict: "fail", summary: "a and b do not compose" }, c.members[0]).state, "failure"); + assert.equal(groupGateStatus({ ...c, verdict: "error", summary: "could not run" }, c.members[0]).state, "error"); +}); diff --git a/modules/mesh-delivery/cmd/mesh-delivery/answers.go b/modules/mesh-delivery/cmd/mesh-delivery/answers.go index 4d0cf2dc..9fb1a14d 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/answers.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/answers.go @@ -294,7 +294,7 @@ func (h *Holder) orphanLines(now time.Time) []StalledLine { continue } w := s.walk - if lapsed, behind := h.lapsedOf(w); len(lapsed) > 0 || len(behind) > 0 { + if len(h.lapsedOn(w)) > 0 { // Owed its word by stops that lapsed, and not given it past the grace (review of issue 753): said, why. says := fmt.Sprintf("the walk %s of %s at %s waits for this owner's word, which the stopped deliveries whose "+ "merge it answers owe it (novox/hq issue 753), and it has not started", w.ID, w.Repository, shortOf(w.Commit, 8)) @@ -402,35 +402,19 @@ func (h *Holder) endedSaid(w Walk) string { // a walk whose every other delivery ended is never left without one. A walk at the merge of a delivery stopped for // itself, or by a person, gets nothing from it. Under the lock. // -// In its group's order (review of issue 753): a member gives its word only once every member ordered before it reached -// the machines, or is itself a stop that lapsed whose merge this same walk answers — the walk then delivers both, in -// the group's order among its tiers, as members sharing one walk do. A member behind one still undelivered gives -// nothing; lapsedBehind names that one. +// No member is ordered before another (novox/hq ADR 0418 §2): each such delivery gives its word at once. func (h *Holder) lapsedOn(w Walk) []string { - out, _ := h.lapsedOf(w) - return out -} - -// lapsedOf is lapsedOn, and, by delivery, the member before it in its group that holds back the word of a stop that -// lapsed whose merge the walk answers. Under the lock. -func (h *Holder) lapsedOf(w Walk) ([]string, map[string]string) { if w.Release != nil || w.IsBatch() || !w.Open() || !w.Seen().Waits { - return nil, nil + return nil } var out []string - behind := map[string]string{} for _, d := range h.deliveries { - if !h.lapsedAnswered(w, d) { - continue + if h.lapsedAnswered(w, d) { + out = append(out, d.ID) } - if before := h.lapsedBehind(w, d); before != "" { - behind[d.ID] = before - continue - } - out = append(out, d.ID) } sort.Strings(out) - return out, behind + return out } // lapsedAnswered is whether a delivery's stop lapsed and the walk answers its merge, not left out. @@ -445,52 +429,17 @@ func (h *Holder) lapsedAnswered(w Walk, d *Delivery) bool { return !left } -// lapsedBehind is the member ordered before a delivery in its group that has not reached the machines and that this -// walk does not deliver with it, with its state; empty for none. -func (h *Holder) lapsedBehind(w Walk, d *Delivery) string { - g := h.groups[d.Group] - if d.Group == "" || g == nil { - return "" - } - members := h.membersOf(g) - byID := map[string]*Delivery{} - for _, m := range members { - byID[m.ID] = m - } - for _, m := range h.inOrder(g, members) { - if m.ID == d.ID { - return "" - } - o := byID[m.ID] - if o == nil || memberReached(o) || h.lapsedAnswered(w, o) { - continue - } - return fmt.Sprintf("%s (%s)", o.ID, o.State) - } - return "" -} - // lapsedSaid is why a walk whose word stops that lapsed give still waits: the members of their groups not yet // delivered before them, what the last look held it for, or the controller's refusal of the word. Under the lock. func (h *Holder) lapsedSaid(w Walk) string { var parts []string - _, behind := h.lapsedOf(w) - ids := make([]string, 0, len(behind)) - for id := range behind { - ids = append(ids, id) - } - sort.Strings(ids) - for _, id := range ids { - parts = append(parts, fmt.Sprintf("%s waits for %s, ordered before it in its group", id, behind[id])) - } if why := h.waitsWhy[w.ID]; why != "" { parts = append(parts, "not let go yet: "+why) } if err := h.deliverErr[w.ID]; err != "" { parts = append(parts, "the controller refused its word: "+err) } - lapsed, _ := h.lapsedOf(w) - for _, id := range lapsed { + for _, id := range h.lapsedOn(w) { if d := h.deliveries[id]; d != nil && strings.Contains(d.LetGoWhy, lapsedWord(w.ID)) { parts = append(parts, fmt.Sprintf("its word was given for %s at %s, and the controller has not started it", id, d.LetGo.Local().Format("15:04:05"))) @@ -499,10 +448,9 @@ func (h *Holder) lapsedSaid(w Walk) string { return strings.Join(parts, "; ") } -// lapsedOwed is whether a walk's word is owed by stops that lapsed: given now, or once a member before one is delivered. +// lapsedOwed is whether a walk's word is owed by stops that lapsed. func (h *Holder) lapsedOwed(w Walk) bool { - lapsed, behind := h.lapsedOf(w) - return len(lapsed) > 0 || len(behind) > 0 + return len(h.lapsedOn(w)) > 0 } // lapsedTurns registers, for every open walk waiting for this owner's word, the turn of each delivery whose stop @@ -679,7 +627,7 @@ func (h *Holder) WalksWaiting() []StalledLine { case !live && h.lapsedOwed(w): // Its word comes from the stopped deliveries whose merge it answers (novox/hq issue 753): said, never // unheld, while the controller has not taken it, with what holds it. - lapsed, _ := h.lapsedOf(w) + lapsed := h.lapsedOn(w) says := "its word is owed by the stopped deliveries whose merge it answers, each stopped for a member " + "delivered since (novox/hq issue 753)" if len(lapsed) > 0 { diff --git a/modules/mesh-delivery/cmd/mesh-delivery/approval_test.go b/modules/mesh-delivery/cmd/mesh-delivery/approval_test.go index e37d3f55..d440d1af 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/approval_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/approval_test.go @@ -159,12 +159,10 @@ func TestAnExpiredAskIsAskedAgainOnce(t *testing.T) { } } -func TestAGroupIsAskedOnceForAllItsHeadsAndMergedInOrderOnOneApprove(t *testing.T) { +func TestAGroupIsAskedOnceForAllItsHeadsAndMergedInOnePassOnOneApprove(t *testing.T) { w := newWorld(t) w.op.auto = false const ctlHead, catHead = "c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1", "d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2" - ctl, cat := IDOf("novox/mesh-controller", ctlHead), IDOf("novox/mesh-catalog", catHead) - w.ctl.order = func(ms []Member) Order { return Order{Order: []string{ctl, cat}} } w.h.PullUpdated(pr("novox/mesh-controller", 1, ctlHead, "feat/x", "internal/a.go")) w.h.PullUpdated(pr("novox/mesh-catalog", 2, catHead, "feat/x", "modules/app/module.json")) w.h.Checked(judgedAt("novox/mesh-controller", 1, ctlHead, "pass", "t1t1t1t1t1t1")) @@ -185,35 +183,17 @@ func TestAGroupIsAskedOnceForAllItsHeadsAndMergedInOrderOnOneApprove(t *testing. } w.op.answer(a.ID, OptionApprove) w.settleAll() - if len(w.forge.merges) != 1 || w.forge.merges[0] != "novox/mesh-controller#1@"+ctlHead { - t.Fatalf("the first member was not merged alone on the Approve: %v", w.forge.merges) - } - w.h.PullMerged(merged("novox/mesh-controller", 1, ctlHead, "m1m1m1m1m1m1")) - w.settleAll() - if len(w.forge.merges) != 2 || w.forge.merges[1] != "novox/mesh-catalog#2@"+catHead || w.op.count() != 1 { - t.Fatalf("the next member was not merged on the same Approve: merges %v, asks %d", w.forge.merges, w.op.count()) + if len(w.forge.merges) != 2 || w.op.count() != 1 { + t.Fatalf("both members were not merged in one pass on the one Approve: merges %v, asks %d", w.forge.merges, w.op.count()) } } // A push to a group member without a mark ends the group's ask (ADR 0398 §3, issue 948; review round 1): the Approve // that follows would be for heads the group no longer has. -func TestAPushToAnUnmarkedGroupMemberEndsTheGroupsAskAndApprove(t *testing.T) { +func TestAPushToAGroupMemberEndsTheGroupsAskAndApproveAndSaysSoOnEveryMember(t *testing.T) { w := newWorld(t) w.op.auto = false const ctlHead, catHead, catPushed = "c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1", "d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2", "e3e3e3e3e3e3e3e3e3e3e3e3e3e3e3e3e3e3e3e3" - ctl, cat := IDOf("novox/mesh-controller", ctlHead), IDOf("novox/mesh-catalog", catHead) - w.ctl.order = func(ms []Member) Order { - var o []string - for _, m := range ms { - if m.Repository == "novox/mesh-controller" { - o = append([]string{m.ID}, o...) - } else { - o = append(o, m.ID) - } - } - return Order{Order: o} - } - _, _ = ctl, cat w.h.PullUpdated(pr("novox/mesh-controller", 1, ctlHead, "feat/x", "internal/a.go")) w.h.PullUpdated(pr("novox/mesh-catalog", 2, catHead, "feat/x", "modules/app/module.json")) w.h.Checked(judgedAt("novox/mesh-controller", 1, ctlHead, "pass", "t1t1t1t1t1t1")) @@ -322,7 +302,6 @@ func TestIssue1025AMergeInAGroupCarriesTheGroupAndItsHeads(t *testing.T) { w.op.auto = false const ctlHead, catHead = "c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1", "d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2" ctl, cat := IDOf("novox/mesh-controller", ctlHead), IDOf("novox/mesh-catalog", catHead) - w.ctl.order = func(ms []Member) Order { return Order{Order: []string{ctl, cat}} } w.h.PullUpdated(pr("novox/mesh-controller", 1, ctlHead, "feat/x", "internal/a.go")) w.h.PullUpdated(pr("novox/mesh-catalog", 2, catHead, "feat/x", "modules/app/module.json")) w.h.Checked(judgedAt("novox/mesh-controller", 1, ctlHead, "pass", "t1t1t1t1t1t1")) @@ -340,14 +319,15 @@ func TestIssue1025AMergeInAGroupCarriesTheGroupAndItsHeads(t *testing.T) { w.op.answer(open[0], OptionApprove) w.settleAll() got := mergedSaid(w) - if len(got) != 1 { - t.Fatalf("announced %d; wanted the first member's merge once: %v", len(got), got) + if len(got) != 2 { + t.Fatalf("announced %d; wanted each member's merge once, from the one pass: %v", len(got), got) } - m := got[0] - hs, _ := m["heads"].(map[string]any) - if m["group"] != "feat/x" || m["ask_id"] != open[0] || m["option"] != OptionApprove || m["repo"] != "mesh-controller" || - hs[ctl] != ctlHead || hs[cat] != catHead || len(hs) != 2 { - t.Fatalf("the group's merge event: %v", m) + for _, m := range got { + hs, _ := m["heads"].(map[string]any) + if m["group"] != "feat/x" || m["ask_id"] != open[0] || m["option"] != OptionApprove || + hs[ctl] != ctlHead || hs[cat] != catHead || len(hs) != 2 { + t.Fatalf("the group's merge event: %v", m) + } } } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/bus_test.go b/modules/mesh-delivery/cmd/mesh-delivery/bus_test.go index 90b8a8b2..832b1ac9 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/bus_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/bus_test.go @@ -472,3 +472,93 @@ func TestAPassingVerdictAsksTheApproveAndItsApproveMergesWithinSecondsOverARealB t.Fatalf("%d asks of the mesh in a quiet second and a half after the merge: something polls", more) } } + +// **The acceptance of one group, one Approve** (novox/hq ADR 0418 §4, ADR 0421, design 47 milestone 3, issues 978 and +// 1022): a group of two pull requests on one branch name, in two repositories, with green checks, gets one ask of the +// operator — no label, comment or mark written by anyone — and on the Approve both merge in one pass, each handed the +// approved heads of both. +func TestAGroupOfTwoWithGreenChecksGetsOneAskAndOnItsApproveBothMergeInOnePassOverARealBus(t *testing.T) { + b := newTestBus(t) + rt := b.start(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + rt.deliverEvents(ctx) + rt.until("the module to subscribe to the events it follows", 20*time.Second, func() bool { + rt.mu.Lock() + defer rt.mu.Unlock() + return len(rt.subs) >= len(followed)+1 + }) + const ctlHead, catHead = "c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1", "d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2" + for _, p := range []PullEvent{pr("novox/mesh-controller", 1, ctlHead, "feat/x", "internal/a.go"), + pr("novox/mesh-catalog", 2, catHead, "feat/x", "modules/app/module.json")} { + p.BaseSHA = "t1t1t1t1t1t1" + p.Body = "Reviewed-by: lead@shanks at " + p.HeadSHA + b.publish(t, "mesh.mod.gitea.event.pull.updated", p) + } + b.publish(t, "mesh.seat."+ControllerSeat+".event.checked", judgedAt("novox/mesh-controller", 1, ctlHead, "pass", "t1t1t1t1t1t1")) + b.publish(t, "mesh.seat."+ControllerSeat+".event.checked", judgedAt("novox/mesh-catalog", 2, catHead, "pass", "t1t1t1t1t1t1")) + + // Both members green: the composed check is asked, once, for both. + rt.until("the composed check to be asked", 10*time.Second, func() bool { + return len(rt.asksOf("seat:"+ControllerSeat+".delivery-check")) >= 1 + }) + if n := len(rt.asksOf("seat:" + ControllerSeat + ".delivery-order")); n != 0 { + t.Fatalf("the controller's delivery-order was asked %d times: no order is asked for", n) + } + rt.mu.Lock() + nothing := len(rt.operatorAsks) + rt.mu.Unlock() + if nothing != 0 { + t.Fatalf("the operator was asked before the group's composed check passed") + } + // The composed check passes at both heads: the operator is asked once, for the group. + b.publish(t, "mesh.seat."+ControllerSeat+".event.checked", map[string]any{"group": "feat/x", "id": "check-1", "verdict": "pass", + "summary": "both compose", "members": []map[string]any{ + {"owner": "novox", "repo": "mesh-controller", "commit": ctlHead, "trunk": "t1t1t1t1t1t1"}, + {"owner": "novox", "repo": "mesh-catalog", "commit": catHead, "trunk": "t1t1t1t1t1t1"}}}) + rt.until("the operator to be asked once for the group", 10*time.Second, func() bool { + rt.mu.Lock() + defer rt.mu.Unlock() + return len(rt.operatorAsks) == 1 + }) + time.Sleep(500 * time.Millisecond) + rt.mu.Lock() + asked, ask := len(rt.operatorAsks), rt.operatorAsks[0] + rt.mu.Unlock() + if asked != 1 { + t.Fatalf("the operator was asked %d times for one group", asked) + } + if len(rt.asksOf("gitea.gitea_merge_pull_request")) != 0 { + t.Fatalf("merged before the Approve") + } + opt, _ := ask.Option(OptionApprove) + approved := b.publish(t, "mesh.seat."+asks.Seat+".event.decided."+Module, asks.Warrant{Ask: ask.ID, Asker: Module, + Outcome: asks.OutcomeChosen, Option: OptionApprove, Label: opt.Label, Level: opt.Level, + By: &asks.Person{Who: asks.Operator, Kind: "telegram", Verified: "user id verified"}, At: time.Now(), AskDigest: ask.Digest()}) + + // Both merge on that one Approve, in one pass: asked within seconds of the Approve's event, each with both heads. + rt.until("both merges to be asked of the forge", 5*time.Second, func() bool { return len(rt.asksOf("gitea.gitea_merge_pull_request")) >= 2 }) + merges := rt.asksOf("gitea.gitea_merge_pull_request") + if len(merges) != 2 { + t.Fatalf("the merge was asked %d times; wanted once for each member: %+v", len(merges), merges) + } + heads := map[string]bool{} + for _, m := range merges { + heads[fmt.Sprint(m.Body["repo"], "@", m.Body["head"])] = true + if got := fmt.Sprint(m.Body["approved_heads"]); got != "[novox/mesh-catalog@"+catHead+" novox/mesh-controller@"+ctlHead+"]" { + t.Fatalf("a merge was handed the approved heads %s", got) + } + if took := m.At.Sub(approved); took > 5*time.Second { + t.Fatalf("a merge was asked %s after the Approve's event; wanted within 5s", took) + } + } + if !heads["mesh-controller@"+ctlHead] || !heads["mesh-catalog@"+catHead] { + t.Fatalf("the merges were asked for %v", heads) + } + rt.mu.Lock() + again := len(rt.operatorAsks) + rt.mu.Unlock() + if again != 1 { + t.Fatalf("a second ask was made: %d", again) + } +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/covered_test.go b/modules/mesh-delivery/cmd/mesh-delivery/covered_test.go index ca989ff1..a7caf67d 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/covered_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/covered_test.go @@ -241,7 +241,6 @@ func TestACoveredGroupMemberLetsTheNextOneGo(t *testing.T) { const ctl, lab = "novox/mesh-controller", "novox/mesh-lab" const ctlHead, labHead = "c1c1c1c1c1c1c1c1", "d2d2d2d2d2d2d2d2" ctlID, labID := IDOf(ctl, ctlHead), IDOf(lab, labHead) - w.ctl.order = func(ms []Member) Order { return Order{Order: []string{ctlID, labID}} } pc, pl := pr(ctl, 1, ctlHead, "feat/g", "cmd/a.go"), pr(lab, 2, labHead, "feat/g", "replays/a.go") w.h.PullUpdated(pc) w.h.PullUpdated(pl) @@ -347,7 +346,6 @@ func TestHeldDeliveriesReadBackCloseByThemselvesOnce(t *testing.T) { return []*Delivery{d, other, coverer(cat, "systemd")} }(), &Group{ID: "feat/j", Members: []string{IDOf(cat, head), IDOf("novox/mesh-host", "abababababababab")}, Closed: true, - Order: []string{IDOf(cat, head), IDOf("novox/mesh-host", "abababababababab")}, Check: &GroupCheck{Asked: "check-1", Verdict: "pass", Commits: map[string]string{IDOf(cat, head): head, IDOf("novox/mesh-host", "abababababababab"): "abababababababab"}}}, Covered}, @@ -418,8 +416,15 @@ func TestAGroupMemberWaitsForItsComposedCheckToAnswer(t *testing.T) { } w.h.Checked(CheckedEvent{Group: "feat/k", ID: "check-1", Verdict: "pass", Summary: "both compose"}) w.settleAll() + if s := w.state(aID); s != Published || len(w.ctl.delivers) != 0 { + t.Fatalf("its composed check passed, but the other member is not merged: %s, let go %v", s, w.ctl.delivers) + } + // The other member merges: every member is on the trunk, and the walk is let go (ADR 0418 §2). + w.h.PullMerged(mergedAt(pb, "m2m2m2m2m2m2", t0)) + w.h.WalkMoved(w.ctl.put(aWalk("plan-b", b, "m2m2m2m2m2m2", true))) + w.settleAll() if s := w.state(aID); s != Delivering || !containsString(w.ctl.delivers, "plan-a") { - t.Fatalf("its composed check passed, and it is %s, let go %v", s, w.ctl.delivers) + t.Fatalf("every member merged, and it is %s, let go %v", s, w.ctl.delivers) } } @@ -448,16 +453,18 @@ func TestAMemberHeldForItsComposedCheckIsLiftedWhenItPasses(t *testing.T) { Plan: planMoving(a, "a"), Check: &Verdict{Gate: "pass"}, Walk: &WalkSeen{ID: "plan-a", Commit: "m1m1m1m1m1m1", State: walkBuilding, Waits: true, WaitedFor: "mesh-delivery"}} db := &Delivery{ID: bID, Repository: b, Commit: bHead, Number: 2, Base: "main", Branch: "feat/k", Group: "feat/k", - State: Ready, Since: t0, Created: t0.Add(-time.Hour), Plan: planMoving(b, "b"), Check: &Verdict{Gate: "pass"}} - g := &Group{ID: "feat/k", Members: []string{aID, bID}, Order: []string{aID, bID}, Closed: true, - OrderOf: map[string]string{aID: aHead, bID: bHead}, - Check: &GroupCheck{Asked: "check-old", At: t0, Commits: c.commits}} + State: Published, Since: t0, Created: t0.Add(-time.Hour), MergedAs: "m2m2m2m2m2m2", MergedAt: t0, + Plan: planMoving(b, "b"), Check: &Verdict{Gate: "pass"}, + Walk: &WalkSeen{ID: "plan-b", Commit: "m2m2m2m2m2m2", State: walkBuilding, Waits: true, WaitedFor: "mesh-delivery"}} + g := &Group{ID: "feat/k", Members: []string{aID, bID}, Closed: true, + Check: &GroupCheck{Asked: "check-old", At: t0, Commits: c.commits}} for _, d := range []*Delivery{da, db} { _ = store.PutDelivery(d) } _ = store.PutGroup(g) ctl := newFakeController() ctl.put(aWalk("plan-a", a, "m1m1m1m1m1m1", true)) + ctl.put(aWalk("plan-b", b, "m2m2m2m2m2m2", true)) w := newWorldOver(t, store, ctl, newFakeForge(), t0.Add(time.Minute)) w.settleAll() if s := w.state(aID); s != Held { diff --git a/modules/mesh-delivery/cmd/mesh-delivery/effects.go b/modules/mesh-delivery/cmd/mesh-delivery/effects.go index b74ae890..525a612d 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/effects.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/effects.go @@ -207,13 +207,7 @@ func ViewBody(d *Delivery, g *Group) string { b.WriteString("\n") } if g != nil { - fmt.Fprintf(&b, "**Group** `%s`, in order: %s\n", g.ID, strings.Join(g.Order, " → ")) - for _, p := range g.Pairs { - fmt.Fprintf(&b, "- %s\n", p.orderSaid()) - } - for _, c := range g.Contradictions { - fmt.Fprintf(&b, "- **refused**: %s\n", c.Said) - } + fmt.Fprintf(&b, "**Group** `%s`: %s\n", g.ID, strings.Join(g.Members, ", ")) if g.Check != nil && g.Check.Verdict != "" { fmt.Fprintf(&b, "- composed together: %s — %s\n", g.Check.Verdict, g.Check.Summary) } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go index 8e2b9a2a..da842249 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go @@ -89,7 +89,6 @@ type fakeController struct { mu sync.Mutex walks map[string]Walk held bool - order func([]Member) Order checks []string asked int delivers []string @@ -150,22 +149,6 @@ func (c *fakeController) Plan(repository, base, head string, paths, dirs, remove } return &DeliveryPlan{Repository: repository, Base: base, Head: head, Moved: []string{"app"}, Summary: "builds app"}, nil } -func (c *fakeController) Order(ms []Member) (Order, error) { - c.mu.Lock() - defer c.mu.Unlock() - if c.down { - return Order{}, errors.New("down") - } - if c.order != nil { - return c.order(ms), nil - } - var ids []string - for _, m := range ms { - ids = append(ids, m.ID) - } - sort.Strings(ids) - return Order{Order: ids}, nil -} func (c *fakeController) Check(group string, ms []Member) (string, error) { c.mu.Lock() defer c.mu.Unlock() @@ -395,7 +378,9 @@ type fakeForge struct { notes map[string][]string views map[string]string statuses map[string]string - down bool + // statusWrites counts the statuses set. + statusWrites int + down bool // viewErr is what View answers instead, when set: a pull request the forge no longer holds. viewErr error viewCalls int @@ -471,6 +456,7 @@ func (f *fakeForge) Status(owner, repo, commit, context, state, description, tar if f.down { return errors.New("the forge is away") } + f.statusWrites++ f.statuses[owner+"/"+repo+"@"+commit+" "+context] = state + " " + description + " → " + target return nil } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/group_test.go b/modules/mesh-delivery/cmd/mesh-delivery/group_test.go new file mode 100644 index 00000000..c9a34734 --- /dev/null +++ b/modules/mesh-delivery/cmd/mesh-delivery/group_test.go @@ -0,0 +1,190 @@ +package main + +import ( + "encoding/json" + "sort" + "testing" + "time" +) + +// **A group is merged in one pass on its one Approve, and delivered by one plan** (novox/hq ADR 0418, ADR 0419, ADR +// 0421, issues 978 and 1047): the members are not ordered, none waits for another's merge or delivery, and a trunk +// move does not ask the operator a second time. + +const ( + gCtl, gCat = "novox/mesh-controller", "novox/mesh-catalog" + gHc, gHk = "c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1", "d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2" + gTrunk0, gT1 = "a2c2c35000000000000000000000000000000000", "d28ca95000000000000000000000000000000000" +) + +// approvedGroup is a group of two pull requests on one branch, checks green, composed green and the operator's Approve +// asked and not yet answered (w.op.auto is off). +func approvedGroup(t *testing.T) *world { + w := newWorld(t) + w.op.auto = false + w.h.PullUpdated(pr(gCtl, 1, gHc, "feat/g", "internal/a.go")) + w.h.PullUpdated(pr(gCat, 2, gHk, "feat/g", "modules/app/module.json")) + w.h.Checked(judged(gCtl, 1, gHc, "pass", gTrunk0)) + w.h.Checked(judged(gCat, 2, gHk, "pass", gTrunk0)) + w.settleAll() + g := w.h.groups["feat/g"] + if g == nil || g.Check == nil { + t.Fatalf("no composed check was asked: %+v", g) + } + w.h.Checked(CheckedEvent{Group: "feat/g", ID: g.Check.Asked, Verdict: "pass", Summary: "composed", + Members: []struct { + Owner string `json:"owner"` + Repo string `json:"repo"` + Commit string `json:"commit"` + Trunk string `json:"trunk,omitempty"` + }{{Owner: "novox", Repo: "mesh-controller", Commit: gHc, Trunk: gTrunk0[:12]}, + {Owner: "novox", Repo: "mesh-catalog", Commit: gHk, Trunk: gTrunk0[:12]}}}) + w.settleAll() + if w.op.count() != 1 || len(w.op.open()) != 1 { + t.Fatalf("the green group is asked once: %d asks, open %v", w.op.count(), w.op.open()) + } + return w +} + +// Issue 1047 (the rule the lead settled, ADR 0421 §3): an approved group whose trunk moved through a merge that is not +// its member's has each out-of-date member's check asked at once, then the composed check, and merges on the same +// Approve — with no second one asked. +func TestAnApprovedGroupWhoseTrunkMovedIsCheckedAgainAndMergedOnTheSameApprove(t *testing.T) { + w := approvedGroup(t) + asked := len(w.ctl.checks) + w.h.PullMerged(merged(gCtl, 5, "5555555555555555", gT1)) + w.settleAll() + // The member whose trunk moved has its own check asked, in the same look; the other member's trunk did not move. + if got := w.ctl.checks[asked:]; len(got) != 1 || got[0] != ":"+IDOf(gCtl, gHc) { + t.Fatalf("the trunk's move asked %v; wanted the moved member's own check once", got) + } + if w.ctl.baseSHAs[len(w.ctl.baseSHAs)-1] != gT1[:12] && w.ctl.baseSHAs[len(w.ctl.baseSHAs)-1] != gT1 { + t.Fatalf("its check was not asked against the trunk's head: %v", w.ctl.baseSHAs) + } + // The Approve is given while the checks run: nothing merges on the verdict out of date. + w.op.answer(w.op.open()[0], OptionApprove) + w.settleAll() + if len(w.forge.merges) != 0 { + t.Fatalf("merged while the group's checks were out of date: %v", w.forge.merges) + } + // The member is judged again: the composed check is asked on the new verdicts. + w.later(time.Minute) + w.h.Checked(judged(gCtl, 1, gHc, "pass", gT1)) + w.settleAll() + g := w.h.groups["feat/g"] + last := w.ctl.checks[len(w.ctl.checks)-1] + if last != "feat/g:"+IDOf(gCat, gHk)+","+IDOf(gCtl, gHc) { + t.Fatalf("the composed check was not asked again: %v", w.ctl.checks) + } + w.h.Checked(CheckedEvent{Group: "feat/g", ID: g.Check.Asked, Verdict: "pass", Summary: "composed again"}) + w.settleAll() + sort.Strings(w.forge.merges) + if len(w.forge.merges) != 2 || w.forge.merges[0] != "novox/mesh-catalog#2@"+gHk || w.forge.merges[1] != "novox/mesh-controller#1@"+gHc { + t.Fatalf("both members were not merged on the new composed verdict: %v", w.forge.merges) + } + if w.op.count() != 1 { + t.Fatalf("a second Approve was asked: %d", w.op.count()) + } +} + +// ADR 0418 §4: a member the forge refuses is merged again on its fresh check, while the other merged in the same pass. +func TestAMemberTheForgeRefusesIsMergedAgainWhileTheOtherMergedInTheSamePass(t *testing.T) { + w := approvedGroup(t) + w.forge.refuse = errAskedAgain + w.op.answer(w.op.open()[0], OptionApprove) + w.settleAll() + if len(w.forge.merges) != 2 { + t.Fatalf("both members are asked in the same pass: %v", w.forge.merges) + } + w.forge.refuse = nil + w.later(2 * time.Minute) + w.settleAll() + if len(w.forge.merges) != 4 { + t.Fatalf("a refused member is asked again a minute on: %v", w.forge.merges) + } + if d := w.delivery(IDOf(gCat, gHk)); d.Ready.Merged == nil { + t.Fatalf("not merged: %+v", d.Ready) + } +} + +// Issue 978 (the pin): a feature's single pull request in one repository is a lone delivery, asked alone. +func TestAFeaturesSinglePullRequestIsALoneDeliveryAskedAlone(t *testing.T) { + w := newWorld(t) + w.op.auto = false + w.h.PullUpdated(pr("novox/app", 7, readyHead, "feat/x", "src/a.go")) + w.h.Checked(judgedAt("novox/app", 7, readyHead, "pass", "t1t1t1t1t1t1")) + w.settleAll() + d := w.delivery(IDOf("novox/app", readyHead)) + if d.Group != "" || len(w.h.groups) != 0 { + t.Fatalf("a single pull request made a group: %q %v", d.Group, w.h.groups) + } + if w.op.count() != 1 { + t.Fatalf("a lone delivery is asked once, alone: %d", w.op.count()) + } +} + +// A group answer says its members' gate is the composed check's (agreed with the controller, issue 977). +func TestTheGroupsAnswerSaysItsGateIsTheComposedChecks(t *testing.T) { + w := approvedGroup(t) + lines := w.h.Groups(false) + if len(lines) != 1 || !lines[0].ComposedGate { + t.Fatalf("groups: %+v", lines) + } + raw, _ := json.Marshal(lines[0]) + var got map[string]any + if err := json.Unmarshal(raw, &got); err != nil || got["composed_gate"] != true { + t.Fatalf("the group line says %s", raw) + } + if _, ordered := got["order"]; ordered { + t.Fatalf("a group line still says an order: %s", raw) + } +} + +// ADR 0403 §1: a member whose own gate the group's composed check judges instead (the controller says its gate layer +// "composed") counts as passed here once its repository's check did not fail, so the group's composed check is asked. +func TestAMemberWhoseGateIsTheComposedChecksCountsAsPassed(t *testing.T) { + w := newWorld(t) + w.op.auto = false + w.h.PullUpdated(pr(gCtl, 1, gHc, "feat/g", "internal/a.go")) + w.h.PullUpdated(pr(gCat, 2, gHk, "feat/g", "modules/app/module.json")) + for _, c := range []struct { + repo string + n int + head string + }{{gCtl, 1, gHc}, {gCat, 2, gHk}} { + v := judged(c.repo, c.n, c.head, "pass", gTrunk0) + v.Gate = &struct { + Verdict string `json:"verdict"` + Summary string `json:"summary"` + Modules []string `json:"modules,omitempty"` + Dependents []string `json:"dependents,omitempty"` + }{Verdict: GateComposed, Summary: "judged by its group's composed check"} + v.RepoCheck = &struct { + Verdict string `json:"verdict"` + Summary string `json:"summary"` + }{Verdict: "pass", Summary: "the repository's own check passed"} + w.h.Checked(v) + } + w.settleAll() + if len(w.ctl.checks) != 1 || w.ctl.checks[0] != "feat/g:"+IDOf(gCat, gHk)+","+IDOf(gCtl, gHc) { + t.Fatalf("the composed check was not asked for members judged by it: %v", w.ctl.checks) + } + // A member whose repository check failed is not passed, whatever the composed check will say. + w2 := newWorld(t) + w2.h.PullUpdated(pr(gCtl, 1, gHc, "feat/g", "internal/a.go")) + v := judged(gCtl, 1, gHc, "pass", gTrunk0) + v.Gate = &struct { + Verdict string `json:"verdict"` + Summary string `json:"summary"` + Modules []string `json:"modules,omitempty"` + Dependents []string `json:"dependents,omitempty"` + }{Verdict: GateComposed} + v.RepoCheck = &struct { + Verdict string `json:"verdict"` + Summary string `json:"summary"` + }{Verdict: "fail", Summary: "a test failed"} + w2.h.Checked(v) + if w2.state(IDOf(gCtl, gHc)) == Ready { + t.Fatalf("a member whose repository check failed is ready") + } +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index 04216762..8b2ea3a1 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -344,6 +344,11 @@ func (h *Holder) PullUpdated(p PullEvent) (isNew bool) { d.ReviewedBy, d.ReviewedAt = by, at h.keep(d) } + // An `after:` line written into the description later is told about too (novox/hq ADR 0419 §2). + if after := AfterIn(p.Body); p.Body != "" && !slices.Equal(after, d.After) { + d.After = after + h.keep(d) + } if d.State == Proposed && d.outOfDate() != "" && d.AskedAgain == nil { d.AskedAgain = &now h.keep(d) @@ -422,24 +427,15 @@ func (h *Holder) takePlaceOf(old string, d *Delivery) bool { if old == "" { return false } - for _, g := range h.groups { - _, ordered := g.OrderOf[old] - if !g.Closed || !ordered && !containsString(g.Order, old) { - continue - } - if ordered { - delete(g.OrderOf, old) - g.OrderOf[d.ID] = d.Commit - } + was := h.deliveries[old] + if was == nil || was.Group == "" { + return false + } + if g := h.groups[was.Group]; g != nil && g.Closed { if !containsString(g.Members, d.ID) { g.Members = append(g.Members, d.ID) sort.Strings(g.Members) } - for i, id := range g.Order { - if id == old { - g.Order[i] = d.ID - } - } d.Group = g.ID g.Updated = h.Now() h.keep(d) @@ -848,7 +844,7 @@ func (h *Holder) WalkMoved(w Walk) { func (h *Holder) walkMoved(w Walk) { for _, d := range h.deliveries { if d.Group != "" { - h.mergeAgainNow(d) // a walk moved: a group member's turn to merge may have come, asked at once + h.mergeAgainNow(d) // a walk moved: a group member's merge may be decided by it, asked at once } } if w.Release != nil { @@ -1093,7 +1089,7 @@ func (h *Holder) Reconcile() error { // --- the look: groups, turns, adoption -------------------------------------------------------------------- -// look takes every step the facts allow that needs another's word: a group's order and composed check, a +// look takes every step the facts allow that needs another's word: a group's composed check, a // delivery's turn, a walk with no delivery made one. The asking happens outside the lock. func (h *Holder) look() { for _, ask := range h.plan() { @@ -1330,7 +1326,7 @@ func (h *Holder) plan() []func() { } } - // Groups: membership, order, composed check, and their turns. + // Groups: membership, composed check, and their word. h.turning = map[string]*turnWant{} grouped := map[string]bool{} for _, g := range h.groups { @@ -1839,34 +1835,7 @@ func (h *Holder) planGroup(g *Group, members []*Delivery, now time.Time) []func( return nil } commits := commitsOf(members) - // The order, worked out again whenever a member's head moved. - if !sameMembers(g.OrderOf, commits) && !g.Closed { - ms := make([]Member, 0, len(members)) - for _, d := range members { - ms = append(ms, MemberOf(d)) - } - id := g.ID - asks = append(asks, func() { - o, err := h.Controller.Order(ms) - h.mu.Lock() - defer h.mu.Unlock() - g := h.groups[id] - if g == nil { - return - } - if err != nil { - h.Logf("[mesh-delivery] group %s's order cannot be asked yet: %v", id, err) - h.again("a group's order that could not be asked") - return - } - g.Order, g.Pairs, g.Cycle, g.Contradictions, g.OrderOf = o.Order, o.Pairs, o.Cycle, o.Contradictions, commits - g.Check = nil // a check of another set of heads is no check of these - g.Updated = h.Now() - h.keepGroup(g) - }) - return asks - } - // The composed check, once every member is ready and the order holds. Asked again for new heads, even once the + // The composed check, once every member is ready. Asked again for new heads, even once the // group is closed: a member rebased or pushed is a new head, and the group waits for its composed check rather // than being held for a person (novox/hq ADR 0323 §6, issue 406). One out of date is asked again once a member // was judged again after it went out of date: a recheck, or the merge tool's ask. @@ -1878,8 +1847,8 @@ func (h *Holder) planGroup(g *Group, members []*Delivery, now time.Time) []func( } } again := g.Check != nil && (!sameMembers(g.Check.Commits, commits) || g.Check.OutOfDate != "" && rejudgedSince(members, g.Check.OutOfDateAt)) - if allReady && len(g.Cycle) == 0 && (g.Check == nil && !g.Closed || again) { - ms := h.inOrder(g, members) + if allReady && (g.Check == nil && !g.Closed || again) { + ms := membersOfAsk(members) id := g.ID asks = append(asks, func() { asked, err := h.Controller.Check(id, ms) @@ -1934,425 +1903,120 @@ func (h *Holder) planGroup(g *Group, members []*Delivery, now time.Time) []func( Since: now, Last: d.Repository + "#" + fmt.Sprint(d.Number) + " " + d.HTMLURL}) } g.Owed = append(g.Owed, Effect{Kind: EffectEmit, Line: string(mustJSON(map[string]any{"group": g.ID, - "state": state, "why": why, "members": g.Members, "order": g.Order})), Since: now}) + "state": state, "why": why, "members": g.Members})), Since: now}) g.Updated = now h.keepGroup(g) } - // Its turns, in its order. + // Each member's mesh/merge-gate is the composed check's (ADR 0403 §1, §6): pending, saying why, while no composed + // verdict stands at these heads. The verdict itself sets it, success or failure, in the forge module. + if why := composedPending(g, members); why != "" { + if g.SaidGate == nil { + g.SaidGate = map[string]string{} + } + for _, d := range members { + if d.MergedAs != "" || d.State.Final() || g.SaidGate[d.ID] == why { + continue + } + g.SaidGate[d.ID] = why + g.Owed = append(g.Owed, Effect{Kind: EffectStatus, Commit: d.Commit, Context: "mesh/merge-gate", State: "pending", + Line: clip("group "+g.ID+": "+why, 140), Since: now, Last: d.Repository + "#" + fmt.Sprint(d.Number) + " " + d.HTMLURL}) + g.Updated = now + h.keepGroup(g) + } + } else if len(g.SaidGate) > 0 { + g.SaidGate = nil + h.keepGroup(g) + } + // Its word, once, when every member is merged (ADR 0418 §2). if g.Closed { - asks = append(asks, h.groupTurns(g, members)...) + asks = append(asks, h.groupWord(g, members)...) } return asks } -// inOrder are a group's members as the controller ordered them. -func (h *Holder) inOrder(g *Group, members []*Delivery) []Member { - byID := map[string]*Delivery{} - for _, d := range members { - byID[d.ID] = d +// composedPending is why a group member's gate stands pending: the composed check has given no verdict at the members' +// heads as they are, or its verdict is out of date; empty once one stands, which the forge module then sets as every +// member's gate (novox/hq ADR 0403 §6). +func composedPending(g *Group, members []*Delivery) string { + c := g.Check + switch { + case c == nil || c.Verdict == "": + return "its group's composed check has not given a verdict at the members' heads yet" + case !sameMembers(c.Commits, commitsOf(members)): + return "composed again for the group's new heads" + case c.OutOfDate != "": + return c.OutOfDate } - var out []Member - for _, id := range g.Order { - if d := byID[id]; d != nil { - out = append(out, MemberOf(d)) - delete(byID, id) - } - } - var rest []string - for id := range byID { - rest = append(rest, id) - } - sort.Strings(rest) - for _, id := range rest { - out = append(out, MemberOf(byID[id])) + return "" +} + +// membersOfAsk are a group's members as the controller is asked about them: by id. No order is asked for or kept; the +// plan's tiers take it from the module graph (novox/hq ADR 0418 §2, ADR 0419). +func membersOfAsk(members []*Delivery) []Member { + sorted := append([]*Delivery(nil), members...) + sort.Slice(sorted, func(i, j int) bool { return sorted[i].ID < sorted[j].ID }) + out := make([]Member, 0, len(sorted)) + for _, d := range sorted { + out = append(out, MemberOf(d)) } return out } -// groupTurns walks a delivering group in its order: the next member is let go only when every member before -// it is delivered; a member that failed or was stopped stops every member after it, naming it. What was -// delivered before it stays, each judged at its own gate (ADR 0239 decision 4); the failed member's builds -// were put back at its gate by the controller. A superseded member counts as delivered once a later delivered -// walk carried it, holds the members after it while a later merge can still carry it, and stops them, saying -// why, only when none can (novox/hq issue 445). -func (h *Holder) groupTurns(g *Group, members []*Delivery) []func() { - waits := map[string]carryWait{} - asks := h.turnsOf(g, members, waits) - // What each member's turn waits on a carrier for is kept on it, and cleared once it no longer does, so `show` - // and `stalled` name the carrier and until when (ADR 0276 decision 9, novox/hq issue 445). +// groupWord gives a closed group's deliveries their turn for the plan's word, once every member is merged (novox/hq +// ADR 0418 §2, issue 978): no member has a turn of its own, and none waits for another's delivery. The word itself is +// asked once per plan by words, when every live delivery the plan carries is at its turn. A member that failed or was +// stopped stops no other: a group's plan that fails fails as every plan does (ADR 0418 §5), and a stop is a person's. +func (h *Holder) groupWord(g *Group, members []*Delivery) []func() { for _, d := range members { - w := waits[d.ID] - if d.State != Published || !d.LetGo.IsZero() { - w = carryWait{} // only a member still waiting for its turn waits for a carrier + if d.MergedAs == "" && !d.State.Final() { + return nil // a member not yet on the trunk: the plan waits for it, nothing is let go } - var until *time.Time - if !w.until.IsZero() { - u := w.until - until = &u - h.lookAt(u, "a group member's turn that waits for a later merge to carry it, until the wait's bound") - } - if d.TurnAwaits != w.said || (d.TurnAwaitsUntil == nil) != (until == nil) || - until != nil && !d.TurnAwaitsUntil.Equal(*until) { - d.TurnAwaits, d.TurnAwaitsUntil = w.said, until - h.keep(d) - } - } - return asks -} - -// turnsOf is groupTurns' walk through the order; waits receives, by member, the carrier its turn waits for. -func (h *Holder) turnsOf(g *Group, members []*Delivery, waits map[string]carryWait) []func() { - ordered := h.inOrder(g, members) - byID := map[string]*Delivery{} - for _, d := range members { - byID[d.ID] = d } // Composed for these very heads: a verdict on another set of heads is no verdict on these. composed := g.Check != nil && (g.Check.Verdict == "pass" || g.Check.Verdict == "warning") && sameMembers(g.Check.Commits, commitsOf(members)) && g.Check.OutOfDate == "" - // A member's turn registered, the members after it that the same walk carries take theirs too: one walk is let - // go once, for all of them (ADR 0276 decision 5). A member on another walk, or not at its turn, waits. + sorted := append([]*Delivery(nil), members...) + sort.Slice(sorted, func(i, j int) bool { return sorted[i].ID < sorted[j].ID }) var asks []func() - sharing := "" - for i, m := range ordered { - d := byID[m.ID] - // A superseded member a later delivered walk carried counts as reached, as in GroupState: a group counts a - // covered member as delivered, so the member after it gets its turn (ADR 0270 decision 2, novox/hq - // issues 442 and 445). - // A stop or failure a later walk delivered since counts as reached too (novox/hq issues 480 and 747). - if memberReached(d) { - continue - } - if sharing != "" && (d.Walk == nil || d.Walk.ID != sharing) { - return asks - } + for _, d := range sorted { switch { - case d.State == Superseded: - // Superseded is no failure: the merge that took its walk over may still deliver its change. While a later - // merge into its trunk that can carry it is open, within that merge's own bound, the members after it - // wait, as for a member still delivering (ADR 0239 decision 4: the next starts when the one before is - // delivered). Only when none can carry it are they stopped, saying why (novox/hq issue 445). - w := h.carryAwaited(d, h.Now()) - // A delivered carrier is asked of the forge here too: coverOf asks only once every module has a - // delivered carrier, and while another module waits on an open one, this answer is what is awaited. - for _, in := range w.ask { - if a := h.askContains(d, in, h.Now()); a != nil { - asks = append(asks, a) - } - } - if w.covered() { - continue // every module delivered by a later merge the forge says contains it: reached - } - if w.why != "" { - for _, later := range ordered[i+1:] { - if a := h.stopInGroup(byID[later.ID], d.ID, fmt.Sprintf("its group %s stopped: %s superseded, and %s", g.ID, - d.ID, w.why)); a != nil { - asks = append(asks, a) - } - } - return asks - } - w.said = fmt.Sprintf("its turn in group %s waits for %s, superseded, to be carried by %s, until %s", g.ID, - d.ID, w.said, w.until.UTC().Format(time.RFC3339)) - for _, later := range ordered[i+1:] { - waits[later.ID] = w - } - // Under ADR 0276 a member is superseded when its waiting walk was folded into a successor that does not - // answer its merge, and the member after it was folded into the same successor: the carrier is then in - // the next member's own walk, which waits for that member's turn. Waiting there would wait for itself - // until the carrier's bound. That walk delivers both, in the group's order among its tiers (decision 5), - // so the members on it take their turn as members sharing one walk do; a member on another walk waits. - // It is shared only when that one walk carries every module of the member not yet carried, each by a - // merge the forge said contains the member's: a carrier of some modules on it, others elsewhere, would - // let the next member go on a walk that cannot deliver the member before it (review of #203). - shared, ask := h.sharedWalk(d, w, h.Now()) - asks = append(asks, ask...) - if shared != "" && sharing == "" { - sharing = shared - continue - } - return asks - case h.stopLapsed(d): - // Stopped for a member whose change reached the machines since: it stops nothing more. Its own change is - // delivered by a later walk of its repository that contains it, which then covers it (novox/hq issue 747). + case memberReached(d), h.stopLapsed(d): continue - case d.State == Failed || d.State == Stopped: - for _, later := range ordered[i+1:] { - if a := h.stopInGroup(byID[later.ID], d.ID, fmt.Sprintf("its group %s stopped: %s %s", g.ID, d.ID, d.State)); a != nil { - asks = append(asks, a) - } - } - return asks case d.State == Held: - // Held for a composed check that has since passed for the heads that merged: lifted, published again, - // and its turn taken in order (novox/hq issue 316). Any other hold is a person's. + // Held for a composed check that has since passed for the heads that merged: lifted, published again + // (novox/hq issue 316). Any other hold is a person's. if composed && heldForGroup(d, g) && walkCanGoOn(d) { d.HeldWhy = "" d.Lifted = fmt.Sprintf("its group %s's composed check said %s for the heads that merged: %s", g.ID, g.Check.Verdict, g.Check.Summary) h.settleAndKeep(d) if d.State == Published { - if a := h.turn(d, fmt.Sprintf("its turn in group %s (%d of %d)", g.ID, i+1, len(ordered))); a != nil { - asks, sharing = append(asks, a), d.Walk.ID - continue + if a := h.turn(d, "its group "+g.ID+": every member is merged"); a != nil { + asks = append(asks, a) } } } - return asks case d.State == Published: // Asked and not answered is not "did not pass": it waits, published, for the answer (issue 316). Nor is a - // composed check of other heads — a member rebased or pushed — or one out of date: it runs again for these - // heads, and the member waits for it (novox/hq ADR 0323 §6, issue 406). + // composed check of other heads, or one out of date: it runs again for these heads, and the member waits + // for it (novox/hq ADR 0323 §6, issue 406). if !composed && d.HeldWhy == "" && g.Check != nil && (g.Check.Verdict == "" || g.Check.OutOfDate != "" || !sameMembers(g.Check.Commits, commitsOf(members))) { - return asks + continue } if !composed && d.HeldWhy == "" { d.HeldWhy = fmt.Sprintf("its group %s's composed check did not pass for the heads that merged; a person "+ "decides that it goes on", g.ID) d.Unwalked = false h.settleAndKeep(d) - return asks - } - if a := h.turn(d, fmt.Sprintf("its turn in group %s (%d of %d)", g.ID, i+1, len(ordered))); a != nil { - asks, sharing = append(asks, a), d.Walk.ID - delete(waits, d.ID) continue } - return asks - default: - return asks // not on the trunk yet, held, or delivering: the members after it wait - } - } - return asks -} - -// containsGrace is how long a later merge whose walk was delivered may still be awaited as a superseded member's -// carrier while the forge has not said whether its merge commit contains the member's: as long as a published -// delivery may wait for its turn (Bounds[Published]). A forge that cannot answer means "not yet covered" (ADR -// 0270), and without this bound the members after it would wait on that answer for ever (novox/hq issue 445). -const containsGrace = 30 * time.Minute - -// readyCarrierBound is how long a merged carrier with no walk yet may be awaited from its merge. Its state, ready, -// has no bound of its own: it waits in a batch, which closes at the latest ten minutes after its first merge -// (ADR 0276 decision 1, `merge-window-at-most`), and then behind at most the one open walk, within that walk's -// bound (decisions 4 and 9). A merge no walk answers is held after `unwalked`, and a held one is no carrier. -const readyCarrierBound = 10*time.Minute + 2*time.Hour - -// carryWait is what a superseded member's group waits for: the later merges that may still carry it, said, and -// the moment the wait ends at the latest; or why none can carry it. Neither means every module it moves was -// delivered by a later merge the forge says contains it: covered. -type carryWait struct { - by []*Delivery - // byModule are the carriers awaited for each module not yet carried. - byModule map[string][]*Delivery - said string - until time.Time - why string - // ask are the delivered carriers' merge commits the forge has not yet said contain the member's. - ask []string -} - -func (w carryWait) covered() bool { return len(w.by) == 0 && w.why == "" } - -// sharedWalk is the walk, not started and waiting for this owner's word, that carries every module of a superseded -// member not yet carried, each by a carrier the forge said contains the member's merge: the walk the members after -// it may take their turn on. Whether a carrier on it contains the merge, when not yet known, is asked of the forge, -// and the members wait for the answer within the carrier's bound (ADR 0270, novox/hq issue 445). -func (h *Holder) sharedWalk(d *Delivery, w carryWait, now time.Time) (string, []func()) { - var walks []string - for _, c := range w.by { - if !c.State.Final() && c.Walk != nil && c.Walk.Waits && !c.Walk.Started() && !containsString(walks, c.Walk.ID) { - walks = append(walks, c.Walk.ID) - } - } - sort.Strings(walks) - var asks []func() - for _, walk := range walks { - every, said := true, true - var unknown []string - for _, carriers := range w.byModule { - on, yes := false, false - for _, c := range carriers { - if c.State.Final() || c.Walk == nil || c.Walk.ID != walk || !c.Walk.Waits || c.Walk.Started() { - continue - } - on = true - contained, known := d.ContainedIn[c.MergedAs] - switch { - case known && contained: - yes = true - case !known && !containsString(unknown, c.MergedAs): - unknown = append(unknown, c.MergedAs) - } - } - every = every && on - said = said && yes - } - if !every { - continue // a module is carried elsewhere, or by nothing on this walk: it cannot deliver the member - } - if said { - return walk, asks - } - for _, in := range unknown { - if a := h.askContains(d, in, now); a != nil { + if a := h.turn(d, "its group "+g.ID+": every member is merged"); a != nil { asks = append(asks, a) } } } - return "", asks -} - -// carryAwaited says which later merges into a superseded member's trunk may still carry every module it moves — -// the cover GroupState counts as reached (ADR 0270's covered test, novox/hq issue 442) — and until when, or, -// when none can, why. A later merge of the same repository into the same trunk on a passing check, whose plan -// moves the module and whose merge commit the forge has not said leaves this one out, can carry it while it is -// merged and not yet ended, not held for a person, and within its bound (ADR 0239 decision 9, ADR 0276 decision -// 9: every wait has a bound): its state's, or readyCarrierBound from its merge while no walk carries it yet; once -// delivered, while the forge has not yet said whether it contains this merge, within containsGrace. A carrier -// superseded in turn ends; the later merge that took it over is a carrier of its own (novox/hq issue 445). -func (h *Holder) carryAwaited(d *Delivery, now time.Time) carryWait { - if d.MergedAs == "" || d.MergedAt.IsZero() || d.Plan == nil || len(d.Plan.Moved) == 0 { - return carryWait{why: "nothing records which modules it moves, so no later merge can carry it"} - } - var w carryWait - seen := map[string]bool{} -modules: - for _, module := range d.Plan.Moved { - found := false - var latest time.Time // the last moment a carrier of this module may still be awaited - var not []string - for _, c := range h.deliveries { - if !mayCarry(c, d, module) { - continue - } - contained, known := d.ContainedIn[c.MergedAs] - var until time.Time - switch { - case known && !contained: - not = append(not, c.ID+" does not contain its merge") - continue - case c.State == Delivered && carries(c, d, module) && known: - continue modules // delivered and contained: this module is carried - case c.State == Delivered && carries(c, d, module): - until = c.Since.Add(containsGrace) - if now.After(until) { - not = append(not, fmt.Sprintf("the forge did not say within %s whether %s contains its merge", containsGrace, c.ID)) - continue - } - if !containsString(w.ask, c.MergedAs) { - w.ask = append(w.ask, c.MergedAs) - } - case c.State.Final() || c.State == Held: - continue // ended without carrying it, or held for a person: no carrier to wait for - case c.State == Ready: - until = c.MergedAt.Add(readyCarrierBound) - if now.After(until) { - not = append(not, fmt.Sprintf("%s is merged and no walk carried it within %s", c.ID, readyCarrierBound)) - continue - } - default: - b, bounded := boundOf(c) - if pastBound(c, now) { - not = append(not, fmt.Sprintf("%s is past its bound (%s %s)", c.ID, c.State, b.For)) - continue - } - if until = now.Add(Bounds[Delivering].For); bounded && b.For > 0 { - until = boundSince(c).Add(b.For) - } - } - found = true - if w.byModule == nil { - w.byModule = map[string][]*Delivery{} - } - w.byModule[module] = append(w.byModule[module], c) - if until.After(latest) { - latest = until - } - if !seen[c.ID] { - seen[c.ID] = true - w.by = append(w.by, c) - } - } - if !found { - why := fmt.Sprintf("no later merge into %s can carry %s", d.Base, module) - if len(not) > 0 { - sort.Strings(not) - why += ": " + strings.Join(not, "; ") - } - return carryWait{why: why} - } - // The wait ends when the first module runs out of carriers, whatever the others still have. - if w.until.IsZero() || latest.Before(w.until) { - w.until = latest - } - } - sort.Slice(w.by, func(i, j int) bool { return w.by[i].ID < w.by[j].ID }) - var said []string - for _, c := range w.by { - s := fmt.Sprintf("%s (%s", c.ID, c.State) - if c.Walk != nil { - s += ", walk " + c.Walk.ID - } - said = append(said, s+")") - } - w.said = strings.Join(said, ", ") - return w -} - -// mayCarry is whether c is a later merge that could carry module for d, by what is known before its walk ends: of -// the same repository, merged later into the same trunk, on its own passing check, its plan moving the module. -func mayCarry(c, d *Delivery, module string) bool { - return c.ID != d.ID && strings.EqualFold(c.Repository, d.Repository) && strings.EqualFold(c.Base, d.Base) && - c.MergedAs != "" && !sameCommit(c.MergedAs, d.MergedAs) && c.MergedAt.After(d.MergedAt) && c.Check.Passes() && - c.Plan != nil && slices.Contains(c.Plan.Moved, module) -} - -// stopInGroup stops a member after one that failed, and leaves its merge out of its walk if it has one open. -// -// **Only its own change** (novox/hq issue 459, ADR 0299): one walk carries every merge of its batch, so ending the -// member's walk ended every other delivery on it, which nobody stopped. Its merge is left out instead: the -// controller ends the walk and walks the other merges again without it, and without each later merge of its -// repository, which contains it. Every member a pass stops on one walk is gathered first, and the walk is asked -// once, by the first member's ask, after the pass: a member is never walked again because its stop came second. -func (h *Holder) stopInGroup(d *Delivery, cause, why string) func() { - if d == nil || d.State.Final() { - return nil - } - t, err := Apply(d, EvStop, true, Facts{Now: h.Now(), By: byOwner, Why: why}) - if err != nil { - h.refuse(err) - return nil - } - d.StoppedFor = cause // the member it was stopped for: once that one is delivered, the stop lapses (novox/hq issue 747) - h.moved(d, []Transition{t}) - // Its open walk; or, its walk stopped for another merge, the batch or walk that walks its merge again (review - // of #221): a merge walked again and then stopped by its group is left out there too. - var walk string - switch { - case d.Walk == nil: - return nil - case d.Walk.State == walkBuilding || d.Walk.State == walkRolling: - walk = d.Walk.ID - case d.walkedAgain(): - walk = d.Walk.WalkedAgainBy - default: - return nil - } - if !h.leave(walk, d) { - return nil // the first member left out of this walk asks for every one - } - return func() { - without, shared := h.leaving459(walk) - if !shared { - if err := h.Controller.Stop(walk, why, byOwner); err != nil { - h.Logf("[mesh-delivery] the walk %s could not be stopped: %v", walk, err) - } - return - } - h.Logf("[mesh-delivery] %s", h.stopLeaving(walk, why, byOwner, without)) - } + return asks } // stopLeaving asks the controller to stop a walk or batch without merges (ADR 0299) and says what came of it. A @@ -2657,8 +2321,7 @@ func againWords(a StopAnswer) string { // 0270's covered test); a closed group whose members all ended is then delivered, and otherwise stopped, naming // the member. A group with nothing in progress never reads delivering. // -// groupTurns agrees: the members after a carried member take their turn, and they wait while a later merge can still -// carry it (novox/hq issue 445). +// groupWord agrees: a group's plan is let go once, when every member is merged (novox/hq ADR 0418 §2). func GroupState(g *Group, members []*Delivery) (string, string) { count := map[State]int{} reached, ended := 0, 0 @@ -2713,8 +2376,6 @@ func GroupState(g *Group, members []*Delivery) (string, string) { return "rejected", "composed together: " + g.Check.Summary case g.Closed: return "delivering", fmt.Sprintf("%d of %d delivered", reached, n) - case len(g.Cycle) > 0: - return "rejected", cycleSaid(g) case count[Rejected] > 0: return "rejected", "a member's own check failed" case g.Check != nil && g.Check.OutOfDate != "": @@ -2764,31 +2425,6 @@ func containsString(xs []string, x string) bool { return false } -// cycleSaid is why a group's order is refused: each contradiction no precedence resolves, with the rules on -// both sides and how to declare the order — or, for a cycle through more members, the pairs in it. -func cycleSaid(g *Group) string { - why := "its order contradicts itself: " - if len(g.Contradictions) > 0 { - var said []string - for _, c := range g.Contradictions { - said = append(said, c.Said) - } - return why + strings.Join(said, "; ") - } - var pairs []string - for _, p := range g.Pairs { - if slices.Contains(g.Cycle, p.Before) && slices.Contains(g.Cycle, p.After) { - pairs = append(pairs, p.orderSaid()) - } - } - why += strings.Join(g.Cycle, ", ") - if len(pairs) > 0 { - why += " (" + strings.Join(pairs, "; ") + "); declare the order with a line `after: ` in " + - "the description of the pull request that goes later" - } - return why -} - // rejudgedSince is whether a member was judged again after a moment: a verdict of its own heard since. func rejudgedSince(members []*Delivery, at time.Time) bool { for _, d := range members { diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder_test.go b/modules/mesh-delivery/cmd/mesh-delivery/holder_test.go index ad010978..7fe4beef 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder_test.go @@ -350,16 +350,13 @@ func TestAMergeWithoutAPassingCheckIsHeld(t *testing.T) { } } -// A delivery group (ADR 0239 decision 4): two heads sharing a branch name across repositories, ordered by the -// controller, checked together, ready only when both are and their composition passed — and delivered in -// order, whatever order they merged in. -func TestAGroupIsOrderedCheckedTogetherAndDeliveredInOrder(t *testing.T) { +// A delivery group (ADR 0239 decision 4, ADR 0418): two heads sharing a branch name across repositories, checked +// together, ready only when both are and their composition passed — and let go for delivery once, when every member is +// merged, whatever order they merged in. No member waits for another's delivery. +func TestAGroupIsCheckedTogetherAndLetGoOnceEveryMemberIsMerged(t *testing.T) { w := newWorld(t) const ctlHead, catHead = "c1c1c1c1c1c1c1c1", "d2d2d2d2d2d2d2d2" ctl, cat := IDOf("novox/mesh-controller", ctlHead), IDOf("novox/mesh-catalog", catHead) - w.ctl.order = func(ms []Member) Order { - return Order{Order: []string{ctl, cat}, Pairs: []Pair{{Before: ctl, After: cat, Why: "version skew"}}} - } w.h.PullUpdated(pr("novox/mesh-controller", 1, ctlHead, "feat/x", "internal/a.go")) w.h.PullUpdated(pr("novox/mesh-catalog", 2, catHead, "feat/x", "modules/app/module.json")) if w.delivery(ctl).Group != "feat/x" || w.delivery(cat).Group != "feat/x" { @@ -372,8 +369,8 @@ func TestAGroupIsOrderedCheckedTogetherAndDeliveredInOrder(t *testing.T) { } w.h.Checked(verdict("novox/mesh-catalog", 2, catHead, "pass")) w.settleAll() - if len(w.ctl.checks) != 1 || w.ctl.checks[0] != "feat/x:"+ctl+","+cat { - t.Fatalf("the composed check asked %v, wanted the members in order", w.ctl.checks) + if len(w.ctl.checks) != 1 || w.ctl.checks[0] != "feat/x:"+cat+","+ctl { + t.Fatalf("the composed check asked %v, wanted the members by id", w.ctl.checks) } if state, _ := GroupState(w.h.groups["feat/x"], w.h.membersOf(w.h.groups["feat/x"])); state != "checking" { t.Fatalf("before its composed verdict the group is %s", state) @@ -387,34 +384,34 @@ func TestAGroupIsOrderedCheckedTogetherAndDeliveredInOrder(t *testing.T) { t.Fatalf("the group's status on a member's head: %v", w.forge.statuses) } - // The catalogue merges first; its walk waits — the controller has not been delivered. + // The catalogue merges first; its walk waits — the controller's member is not merged yet, so nothing is let go. w.h.PullMerged(merged("novox/mesh-catalog", 2, catHead, "m2m2m2m2m2m2")) w.h.WalkMoved(w.ctl.put(aWalk("plan-cat", "novox/mesh-catalog", "m2m2m2m2m2m2", true))) w.settleAll() if w.state(cat) != Published || len(w.ctl.delivers) != 0 { - t.Fatalf("a member merged before the one it goes after was let go: %s %v", w.state(cat), w.ctl.delivers) + t.Fatalf("a member was let go before every member merged: %s %v", w.state(cat), w.ctl.delivers) } - // The controller merges: its walk is on the controller's own path, started at the merge. + // The controller merges: its walk is on the controller's own path, started at the merge. Every member is merged, + // so the catalogue's walk is let go at once, without waiting for the controller's delivery. w.h.PullMerged(merged("novox/mesh-controller", 1, ctlHead, "m1m1m1m1m1m1")) ctlWalk := w.ctl.put(aWalk("plan-ctl", "novox/mesh-controller", "m1m1m1m1m1m1", false)) w.h.WalkMoved(ctlWalk) w.settleAll() - if w.state(ctl) != Delivering || len(w.ctl.delivers) != 0 { - t.Fatalf("the controller's member is %s, let go %v", w.state(ctl), w.ctl.delivers) - } - ctlWalk.State = walkDone - w.h.WalkMoved(w.ctl.put(ctlWalk)) - w.settleAll() - if w.state(ctl) != Delivered || len(w.ctl.delivers) != 1 || w.ctl.delivers[0] != "plan-cat" { - t.Fatalf("once the controller was delivered, the catalogue was not let go: %s %v", w.state(ctl), w.ctl.delivers) + if w.state(ctl) != Delivering || len(w.ctl.delivers) != 1 || w.ctl.delivers[0] != "plan-cat" { + t.Fatalf("every member merged, the catalogue was not let go once: %s %v", w.state(ctl), w.ctl.delivers) } if w.state(cat) != Delivering { t.Fatalf("the catalogue's member is %s", w.state(cat)) } + w.settleAll() + if len(w.ctl.delivers) != 1 { + t.Fatalf("the word was given more than once: %v", w.ctl.delivers) + } } -// A member that fails stops every member after it, naming it; what was delivered before it stays. -func TestAFailedMemberStopsTheMembersAfterItAndLeavesThoseBefore(t *testing.T) { +// A member that fails stops no other (ADR 0418 §5): the plan that fails fails as every plan does, and what was delivered +// stays. Nothing is stopped member by member. +func TestAFailedMemberStopsNoOtherMember(t *testing.T) { w := newWorld(t) heads := map[string]string{"novox/a": "a1a1a1a1a1a1a1", "novox/b": "b2b2b2b2b2b2b2", "novox/c": "c3c3c3c3c3c3c3"} ids := map[string]string{} @@ -431,107 +428,108 @@ func TestAFailedMemberStopsTheMembersAfterItAndLeavesThoseBefore(t *testing.T) { w.h.WalkMoved(w.ctl.put(aWalk("plan-"+repo[6:], repo, mergeCommit, true))) } w.settleAll() + if len(w.ctl.delivers) != 3 { + t.Fatalf("every member merged, yet the walks were let go %v", w.ctl.delivers) + } a := w.ctl.walk("plan-a") a.State = walkDone w.h.WalkMoved(w.ctl.put(a)) - w.settleAll() b := w.ctl.walk("plan-b") - if b.Delivery == nil || b.Delivery.Go == nil { - t.Fatalf("b was not let go after a was delivered: %v", w.ctl.delivers) - } b.State = walkFailed b.Note = "b failed its gate on anchor" w.h.WalkMoved(w.ctl.put(b)) w.settleAll() - if w.state(ids["novox/a"]) != Delivered || w.state(ids["novox/b"]) != Failed || w.state(ids["novox/c"]) != Stopped { + if w.state(ids["novox/a"]) != Delivered || w.state(ids["novox/b"]) != Failed || w.state(ids["novox/c"]) == Stopped { t.Fatalf("a %s, b %s, c %s", w.state(ids["novox/a"]), w.state(ids["novox/b"]), w.state(ids["novox/c"])) } - c := w.delivery(ids["novox/c"]) - if why := c.Transitions[len(c.Transitions)-1].Why; !strings.Contains(why, ids["novox/b"]) { - t.Fatalf("c's stop does not name the member that failed: %q", why) - } - if len(w.ctl.stops) != 1 || w.ctl.stops[0] != "plan-c" { - t.Fatalf("c's walk was not ended: %v", w.ctl.stops) + if len(w.ctl.stops) != 0 || len(w.ctl.withouts) != 0 { + t.Fatalf("a walk was stopped because another failed: %v %v", w.ctl.stops, w.ctl.withouts) } if state, _ := GroupState(w.h.groups["feat/y"], w.h.membersOf(w.h.groups["feat/y"])); state != "failed" { t.Fatalf("the group is %s", state) } } -// A group whose declared order contradicts its inferred one is rejected, and checked never. -func TestAGroupWithACycleIsRejected(t *testing.T) { +// ADR 0419 §2, issue 978: a description's `after:` line is not read, and says so once on its pull request. The +// composed check is asked as for any other group: no order is asked of the controller first, and none gates it. +func TestAnAfterLineIsNotReadAndSaidOnceAndNothingWaitsForAnOrder(t *testing.T) { w := newWorld(t) - w.ctl.order = func(ms []Member) Order { return Order{Cycle: []string{ms[0].ID, ms[1].ID}} } - w.h.PullUpdated(pr("novox/a", 1, "a1a1a1a1a1a1a1", "feat/z")) + w.op.auto = false + pa := pr("novox/a", 1, "a1a1a1a1a1a1a1", "feat/z") + pa.Body = "Goes after the other.\n\nafter: novox/b\n" + w.h.PullUpdated(pa) w.h.PullUpdated(pr("novox/b", 2, "b2b2b2b2b2b2b2", "feat/z")) w.h.Checked(verdict("novox/a", 1, "a1a1a1a1a1a1a1", "pass")) w.h.Checked(verdict("novox/b", 2, "b2b2b2b2b2b2b2", "pass")) w.settleAll() - g := w.h.groups["feat/z"] - if state, why := GroupState(g, w.h.membersOf(g)); state != "rejected" || !strings.Contains(why, "contradicts") { - t.Fatalf("a cycle left the group %s: %s", state, why) - } - if len(w.ctl.checks) != 0 { - t.Fatal("a group with a cycle was checked") - } -} - -// novox/hq issue 309: a group whose rules order one pair both ways at one rank is refused with both rules -// and how to declare the order; a pair a rule won by precedence says what it won over, and the group is -// checked as any other. -func TestAGroupSaysWhichOrderRuleWonOrWhyNoneDid(t *testing.T) { - w := newWorld(t) - said := "novox/a@a1a1a1a1a1a1 and novox/b@b2b2b2b2b2b2: built by puts novox/a@a1a1a1a1a1a1 first, built by puts " + - "novox/b@b2b2b2b2b2b2 first; neither outranks the other: declare the order with a line `after: novox/a` in " + - "novox/b#2's description, or `after: novox/b` in novox/a#1's" - w.ctl.order = func(ms []Member) Order { - return Order{Cycle: []string{ms[0].ID, ms[1].ID}, Contradictions: []Contradiction{{ - Members: []string{ms[0].ID, ms[1].ID}, Said: said, - Rules: []Pair{{Before: ms[0].ID, After: ms[1].ID, Why: "built by"}, {Before: ms[1].ID, After: ms[0].ID, Why: "built by"}}}}} - } - w.h.PullUpdated(pr("novox/a", 1, "a1a1a1a1a1a1a1", "feat/z")) - w.h.PullUpdated(pr("novox/b", 2, "b2b2b2b2b2b2b2", "feat/z")) - w.h.Checked(verdict("novox/a", 1, "a1a1a1a1a1a1a1", "pass")) - w.h.Checked(verdict("novox/b", 2, "b2b2b2b2b2b2b2", "pass")) + w.later(time.Minute) w.settleAll() - g := w.h.groups["feat/z"] - if state, why := GroupState(g, w.h.membersOf(g)); state != "rejected" || !strings.Contains(why, said) { - t.Fatalf("a contradiction was said %s: %s", state, why) + if len(w.ctl.checks) != 1 { + t.Fatalf("the composed check was not asked once: %v", w.ctl.checks) } - if lines := w.h.Groups(false); len(lines) != 1 || len(lines[0].Contradictions) != 1 { - t.Fatalf("groups does not list the contradiction: %+v", lines) + if got := w.ctl.checks[0]; got != "feat/z:"+IDOf("novox/a", "a1a1a1a1a1a1a1")+","+IDOf("novox/b", "b2b2b2b2b2b2b2") { + t.Fatalf("the members asked: %s", got) } - - // Resolved by precedence: ordered, the pair saying what it won over, and checked. - w2 := newWorld(t) - w2.ctl.order = func(ms []Member) Order { - return Order{Order: []string{ms[0].ID, ms[1].ID}, - Pairs: []Pair{{Before: ms[0].ID, After: ms[1].ID, Why: "built by", Over: []string{"engine before controller"}}}} - } - w2.h.PullUpdated(pr("novox/a", 1, "a1a1a1a1a1a1a1", "feat/z")) - w2.h.PullUpdated(pr("novox/b", 2, "b2b2b2b2b2b2b2", "feat/z")) - w2.h.Checked(verdict("novox/a", 1, "a1a1a1a1a1a1a1", "pass")) - w2.h.Checked(verdict("novox/b", 2, "b2b2b2b2b2b2b2", "pass")) - w2.settleAll() - g = w2.h.groups["feat/z"] - if len(g.Cycle) > 0 || len(w2.ctl.checks) != 1 { - t.Fatalf("a group ordered by precedence was not checked: cycle %v, checks %v", g.Cycle, w2.ctl.checks) - } - if got := g.Pairs[0].orderSaid(); !strings.HasSuffix(got, ": built by, over engine before controller") { - t.Fatalf("the pair is said %q", got) - } -} - -// A cycle through members with no contradiction of its own names the pairs in it. -func TestACycleWithoutAContradictionNamesItsPairs(t *testing.T) { - g := &Group{Cycle: []string{"a", "b", "c"}, Pairs: []Pair{{Before: "a", After: "b", Why: "built by"}, - {Before: "b", After: "c", Why: "declared"}, {Before: "c", After: "a", Why: "version skew"}}} - why := cycleSaid(g) - for _, w := range []string{"a before b: built by", "b before c: declared", "c before a: version skew", "`after: `"} { - if !strings.Contains(why, w) { - t.Errorf("the cycle does not say %q: %s", w, why) + said := 0 + for _, c := range w.forge.comments { + if strings.Contains(c, "not read") && strings.Contains(c, "novox/a#1") { + said++ } } + if said != 1 { + t.Fatalf("the after: line is not said once on its pull request: %v", w.forge.comments) + } + // A new head of the same pull request, with the line still there, is not told again. + pa2 := pr("novox/a", 1, "a9a9a9a9a9a9a9", "feat/z") + pa2.Body = pa.Body + w.h.PullUpdated(pa2) + w.settleAll() + said = 0 + for _, c := range w.forge.comments { + if strings.Contains(c, "not read") { + said++ + } + } + if said != 1 { + t.Fatalf("said again for a new head: %v", w.forge.comments) + } +} + +// A group's members have their mesh/merge-gate said pending, saying why, until a composed verdict stands at their heads +// (ADR 0403 §1, §6); the verdict then sets the gate in the forge module. +func TestAGroupMembersGateIsPendingUntilItsComposedVerdictStands(t *testing.T) { + w := newWorld(t) + w.op.auto = false + const ha, hb = "a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1", "b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2" + w.h.PullUpdated(pr("novox/a", 1, ha, "feat/z")) + w.h.PullUpdated(pr("novox/b", 2, hb, "feat/z")) + w.h.Checked(verdict("novox/a", 1, ha, "pass")) + w.h.Checked(verdict("novox/b", 2, hb, "pass")) + w.settleAll() + for _, at := range []string{"novox/a@" + ha, "novox/b@" + hb} { + if s := w.forge.statuses[at+" mesh/merge-gate"]; !strings.HasPrefix(s, "pending") || !strings.Contains(s, "composed check") { + t.Fatalf("the member's gate on %s is %q, wanted pending with its reason", at, s) + } + } + // Said once while the reason stands. + w.later(time.Minute) + before := w.forge.statusWrites + w.settleAll() + if w.forge.statusWrites != before { + t.Fatalf("the pending gate was written again with no change") + } + // A composed verdict stands: mesh-delivery says nothing more of the gate (the forge module sets it). + w.h.Checked(CheckedEvent{Group: "feat/z", ID: "check-1", Verdict: "pass", Summary: "compose"}) + w.settleAll() + if g := w.h.groups["feat/z"]; len(g.SaidGate) != 0 { + t.Fatalf("a standing verdict left a gate pending: %v", g.SaidGate) + } + // A member's push puts it back to pending. + w.h.PullUpdated(pr("novox/a", 1, "a9a9a9a9a9a9a9a9a9a9a9a9a9a9a9a9a9a9a9a9", "feat/z")) + w.settleAll() + if s := w.forge.statuses["novox/b@"+hb+" mesh/merge-gate"]; !strings.HasPrefix(s, "pending") { + t.Fatalf("the sibling's gate after a push is %q", s) + } } // Stalled: a delivery past its state's bound is listed with what H2 may do; close takes only that. diff --git a/modules/mesh-delivery/cmd/mesh-delivery/issue341_test.go b/modules/mesh-delivery/cmd/mesh-delivery/issue341_test.go index e335d52f..afdaea8a 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/issue341_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/issue341_test.go @@ -255,15 +255,13 @@ func TestARebasedMemberIsComposedAgainNotHeld(t *testing.T) { const hc, hk, hc2 = "c1c1c1c1c1c1c1c1", "k1k1k1k1k1k1k1k1", "c2c2c2c2c2c2c2c2" idc, idk, idc2 := IDOf(ctl, hc), IDOf(cat, hk), IDOf(ctl, hc2) w := newWorld(t) + w.op.auto = false // the merges are heard as events below, as when the forge refused one w.h.PullUpdated(pr(ctl, 1, hc, "feat/g")) w.h.PullUpdated(pr(cat, 2, hk, "feat/g")) w.h.Checked(judged(ctl, 1, hc, "pass", trunk0)) w.h.Checked(judged(cat, 2, hk, "pass", trunk0)) w.settleAll() g := w.h.groups["feat/g"] - if len(g.Order) != 2 || g.Order[0] != idk { - t.Fatalf("the group's order is %v: the catalogue's member first is what this replays", g.Order) - } w.h.Checked(CheckedEvent{Group: "feat/g", ID: g.Check.Asked, Verdict: "pass", Summary: "composed"}) w.settleAll() @@ -283,8 +281,8 @@ func TestARebasedMemberIsComposedAgainNotHeld(t *testing.T) { if d := w.delivery(idc2); d.Group != "feat/g" { t.Fatalf("the rebased head did not take its place in its closed group: %q", d.Group) } - if containsString(g.Members, idc) || !containsString(g.Members, idc2) || len(g.Order) != 2 || g.Order[1] != idc2 { - t.Fatalf("the group's members are %v in the order %v", g.Members, g.Order) + if containsString(g.Members, idc) || !containsString(g.Members, idc2) || len(g.Members) != 2 { + t.Fatalf("the group's members are %v", g.Members) } if state, why := GroupState(g, w.h.membersOf(g)); state != "checking" { t.Fatalf("the group reads %s (%s), not checking", state, why) @@ -390,59 +388,6 @@ func TestAnOutOfDateDeliveryAskedAgainIsBoundFromTheAsk(t *testing.T) { } } -// The same, with the first member already delivered when the second is rebased: a delivered member counts for the -// composed check of the new heads, which is asked, and the rebased member is never held (review of ADR 0323 §6). -func TestARebasedMemberAfterAnotherWasDeliveredIsComposedAgain(t *testing.T) { - const ctl, cat = "novox/mesh-controller", "novox/mesh-catalog" - const hc, hk, hc2 = "c1c1c1c1c1c1c1c1", "k1k1k1k1k1k1k1k1", "c2c2c2c2c2c2c2c2" - idk, idc2 := IDOf(cat, hk), IDOf(ctl, hc2) - w := newWorld(t) - w.h.PullUpdated(pr(ctl, 1, hc, "feat/g")) - w.h.PullUpdated(pr(cat, 2, hk, "feat/g")) - w.h.Checked(judged(ctl, 1, hc, "pass", trunk0)) - w.h.Checked(judged(cat, 2, hk, "pass", trunk0)) - w.settleAll() - g := w.h.groups["feat/g"] - w.h.Checked(CheckedEvent{Group: "feat/g", ID: g.Check.Asked, Verdict: "pass", Summary: "composed"}) - w.settleAll() - - // The catalogue's member merges and its walk is done: delivered. - mk := merged(cat, 2, hk, merge1) - mk.Head = "feat/g" - w.h.PullMerged(mk) - walk := w.ctl.put(aWalk("plan-cat", cat, merge1, true)) - w.h.WalkMoved(walk) - w.settleAll() - walk = w.ctl.walk("plan-cat") - walk.State = walkDone - w.h.WalkMoved(w.ctl.put(walk)) - w.settleAll() - if s := w.state(idk); s != Delivered { - t.Fatalf("the catalogue's member is %s, not delivered", s) - } - - // The controller's member is rebased, and judged. - w.h.PullUpdated(pr(ctl, 1, hc2, "feat/g")) - w.h.Checked(judged(ctl, 1, hc2, "pass", trunk0)) - w.settleAll() - asked := w.ctl.checks[len(w.ctl.checks)-1] - if !strings.HasPrefix(asked, "feat/g:") || !strings.Contains(asked, idc2) { - t.Fatalf("the composed check was not asked again for the new heads: %v", w.ctl.checks) - } - if state, why := GroupState(g, w.h.membersOf(g)); state != "checking" { - t.Fatalf("the group reads %s (%s), not checking", state, why) - } - w.h.Checked(CheckedEvent{Group: "feat/g", ID: g.Check.Asked, Verdict: "pass", Summary: "composed again"}) - mc := merged(ctl, 1, hc2, "e0e0e0e0e0e0e0e0") - mc.Head = "feat/g" - w.h.PullMerged(mc) - w.h.WalkMoved(w.ctl.put(aWalk("plan-ctl", ctl, "e0e0e0e0e0e0e0e0", true))) - w.settleAll() - if d := w.delivery(idc2); d.State == Held { - t.Fatalf("the rebased member is held: %s", d.HeldWhy) - } -} - // A check this owner asks — a recheck — sends the base head the forge last announced the head with, so the // controller's verdict for a head no build judges names its trunk commit (ADR 0323 §3, review of mesh-controller#271). func TestARecheckSendsTheAnnouncedBaseHead(t *testing.T) { diff --git a/modules/mesh-delivery/cmd/mesh-delivery/issue442_test.go b/modules/mesh-delivery/cmd/mesh-delivery/issue442_test.go index 2863aca4..b6f9e899 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/issue442_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/issue442_test.go @@ -50,8 +50,7 @@ func TestAGroupWhoseMembersEndedEnds(t *testing.T) { return d } group := func(id string, order ...string) *Group { - return &Group{ID: id, Members: append([]string(nil), order...), Closed: true, Order: order, Said: "delivering", - OrderOf: map[string]string{}} + return &Group{ID: id, Members: append([]string(nil), order...), Closed: true, Said: "delivering"} } const checks, assign = "feat/delivery-checks-verb", "fix/assign-says-why-a-module-is-not-there" diff --git a/modules/mesh-delivery/cmd/mesh-delivery/issue445_test.go b/modules/mesh-delivery/cmd/mesh-delivery/issue445_test.go deleted file mode 100644 index 89ba0119..00000000 --- a/modules/mesh-delivery/cmd/mesh-delivery/issue445_test.go +++ /dev/null @@ -1,446 +0,0 @@ -package main - -import ( - "strings" - "testing" - "time" -) - -// novox/hq issue 445: in a delivery group ordered [A, B], A's walk was superseded by the walk of a later merge C -// of A's repository that does not answer A's merge. groupTurns stopped B at once, although C's walk could still -// carry A's modules and A then counts as reached (ADR 0270 decision 2, issue 442). While a later merge that can -// carry A is open, B waits, saying for which merge and until when; once that merge's walk is delivered and covers -// A, B takes its turn; when C's walk is B's own (ADR 0276 folds both into one successor), B takes its turn on it; -// when no later merge can carry A, or the one that could is past its own bound, B is stopped, saying why. -// -// The shape is the live group fix/assign-says-why-a-module-is-not-there as mesh-delivery showed it on -// 2026-10-11 (issue 442's test): the controller's member superseded first in the order, the lab's after it. - -const ( - g445 = "fix/assign-says-why-a-module-is-not-there" - ctl445, lab445 = "novox/mesh-controller", "novox/mesh-lab" - mergedA, mergedB, mergedC = "a5f53ef9ebdf3650", "77777777aaaaaaaa", "3f68a49500000000" - mergedD = "4d4d4d4d00000000" - headA, headB, headC, headD = "8adb7f1a051941e9", "33e63ee5b47a7552", "3f68a495c0c0c0c0", "4d4d4d4dc0c0c0c0" -) - -var ( - at445 = time.Date(2026, 10, 7, 19, 4, 10, 0, time.UTC) - modules445 = []string{"build-agent", "mesh-controller", "route-proxy"} - aID, bID, cID, dID445 = IDOf(ctl445, headA), IDOf(lab445, headB), IDOf(ctl445, headC), IDOf(ctl445, headD) -) - -// shape445 is A superseded, B published on its walk, and C, the later merge of A's repository, as each case sets it. -type shape445 struct { - a, b, c *Delivery - extra []*Delivery - walks []Walk - forge map[string]bool // what the forge says of containment from the start -} - -func newShape445() *shape445 { - at := at445 - s := &shape445{} - s.a = &Delivery{ID: aID, Repository: ctl445, Commit: headA, Number: 109, Base: "main", Branch: g445, Group: g445, - State: Superseded, Since: at.Add(4 * time.Minute), Created: at.Add(-time.Hour), MergedAs: mergedA, MergedAt: at, - Plan: planMoving(ctl445, modules445...), Check: &Verdict{Gate: "pass"}, - Walk: &WalkSeen{ID: "plan-a5f53e", Commit: mergedA, State: walkSuperseded, Branch: "main", - Note: "superseded at tier 0 by plan-later (" + ctl445 + " at 3f68a495); planned there again"}, - Transitions: []Transition{{At: at.Add(4 * time.Minute), From: Published, To: Superseded, Event: EvSuperseded, - Why: "a newer delivery to the same trunk took over its walk"}}} - s.b = &Delivery{ID: bID, Repository: lab445, Commit: headB, Number: 117, Base: "main", Branch: g445, Group: g445, - State: Published, Since: at.Add(3 * time.Minute), Created: at.Add(-2 * time.Hour), MergedAs: mergedB, - MergedAt: at.Add(2 * time.Minute), Plan: planMoving(lab445, "mesh-lab"), Check: &Verdict{Gate: "pass"}, - Walk: &WalkSeen{ID: "plan-lab", Commit: mergedB, State: walkBuilding, Waits: true, WaitedFor: "mesh-delivery", - Branch: "main", Created: at.Add(3 * time.Minute)}} - s.c = &Delivery{ID: cID, Repository: ctl445, Commit: headC, Number: 110, Base: "main", Branch: "fix/later", - State: Delivering, Since: at.Add(5 * time.Minute), Created: at.Add(-time.Hour), MergedAs: mergedC, - MergedAt: at.Add(time.Minute), Plan: planMoving(ctl445, modules445...), Check: &Verdict{Gate: "pass"}, - Walk: &WalkSeen{ID: "plan-later", Commit: mergedC, State: walkRolling, WaitedFor: "mesh-delivery", GoBy: byOwner, - Branch: "main", Created: at.Add(4 * time.Minute)}} - went := at.Add(5 * time.Minute) - s.walks = []Walk{ - {ID: "plan-a5f53e", Repository: ctl445, Branch: "main", Commit: mergedA, Created: at.Add(30 * time.Second), - State: walkSuperseded, Note: s.a.Walk.Note, Modules: map[string]*WalkModule{}}, - {ID: "plan-lab", Repository: lab445, Branch: "main", Commit: mergedB, Created: at.Add(3 * time.Minute), - State: walkBuilding, Tiers: [][]string{{"mesh-lab"}}, Modules: map[string]*WalkModule{"mesh-lab": {}}, - Delivery: &WalkDelivery{Awaits: "mesh-delivery"}}, - ctlWalk("plan-later", mergedC, at.Add(4*time.Minute), walkRolling, &WalkDelivery{Awaits: "mesh-delivery", - Go: &went, By: byOwner}), - } - return s -} - -// ctlWalk is a walk of the controller's repository at a merge commit, moving A's modules. -func ctlWalk(id, commit string, created time.Time, state string, wd *WalkDelivery) Walk { - w := Walk{ID: id, Repository: ctl445, Branch: "main", Commit: commit, Created: created, State: state, - Tiers: [][]string{modules445}, Modules: map[string]*WalkModule{}, Delivery: wd} - for _, m := range modules445 { - w.Modules[m] = &WalkModule{} - } - return w -} - -// open is the world over the shape, read back and ticked ten minutes after A's merge. -func (s *shape445) open(t *testing.T) *world { - t.Helper() - store := newMemStore() - for _, d := range append([]*Delivery{s.a, s.b, s.c}, s.extra...) { - if d == nil { - continue - } - if err := store.PutDelivery(d); err != nil { - t.Fatal(err) - } - } - g := &Group{ID: g445, Members: []string{aID, bID}, Closed: true, Order: []string{aID, bID}, Said: "delivering", - OrderOf: map[string]string{}, Check: &GroupCheck{Asked: "check-1", At: at445.Add(-time.Hour), Verdict: "pass", - Summary: "both compose", Commits: commitsOf([]*Delivery{s.a, s.b})}} - if err := store.PutGroup(g); err != nil { - t.Fatal(err) - } - ctlr := newFakeController() - for _, w := range s.walks { - ctlr.put(w) - } - forge := newFakeForge() - for k, v := range s.forge { - forge.contains[k] = v - } - w := newWorldOver(t, store, ctlr, forge, at445.Add(10*time.Minute)) - if err := w.h.Reconcile(); err != nil { - t.Fatal(err) - } - w.settleAll() - return w -} - -// end ends a walk in the state given, every module sent to every machine running it. -func end445(w *world, walk, state string) { - r := w.ctl.walk(walk) - sent := w.now - for m := range r.Modules { - r.Modules[m] = &WalkModule{State: "built", SentAt: &sent} - } - r.State = state - w.h.WalkMoved(w.ctl.put(r)) - w.settleAll() -} - -func waits445(t *testing.T, w *world, on string) { - t.Helper() - d := w.delivery(bID) - if d.State != Published || containsString(w.ctl.delivers, "plan-lab") { - t.Fatalf("B is %s, let go %v; want it published and waiting while %s can still carry A", d.State, w.ctl.delivers, on) - } - if !strings.Contains(d.TurnAwaits, on) || !strings.Contains(d.TurnAwaits, aID) || d.TurnAwaitsUntil == nil { - t.Fatalf("B says it waits for %q until %v; want it to name A and %s", d.TurnAwaits, d.TurnAwaitsUntil, on) - } - g := w.h.groups[g445] - if state, why := GroupState(g, w.h.membersOf(g)); state != "delivering" { - t.Fatalf("the group reads %s (%s), want delivering", state, why) - } -} - -func stopped445(t *testing.T, w *world, says string) { - t.Helper() - d := w.delivery(bID) - if d.State != Stopped || len(d.Transitions) == 0 { - t.Fatalf("B is %s, want stopped", d.State) - } - why := d.Transitions[len(d.Transitions)-1].Why - if !strings.Contains(why, aID) || !strings.Contains(why, says) { - t.Fatalf("B was stopped saying %q, want it to name %s and say %q", why, aID, says) - } - if containsString(w.ctl.delivers, "plan-lab") || d.TurnAwaits != "" { - t.Fatalf("B's walk was let go %v, or B still says it waits: %q", w.ctl.delivers, d.TurnAwaits) - } -} - -func letGo445(t *testing.T, w *world, walk string) { - t.Helper() - if d := w.delivery(bID); d.State != Delivering || !containsString(w.ctl.delivers, walk) || d.TurnAwaits != "" { - t.Fatalf("B is %s, let go %v, waiting for %q; want its turn taken on %s", d.State, w.ctl.delivers, d.TurnAwaits, walk) - } -} - -// C's walk runs on its own: B waits for it, says so, and goes on or stops by what comes of it. -func TestAMemberAfterACarriableSupersededOneWaitsForItsCarrier(t *testing.T) { - t.Run("a later walk that can carry A is open: B waits, naming it", func(t *testing.T) { - w := newShape445().open(t) - waits445(t, w, cID) - // Past a published delivery's own 30 minutes, B is not stalled for "its turn or its word": its bound is - // C's, and what `stalled` would say names C. - w.later(Bounds[Published].For + 5*time.Minute) - w.settleAll() - waits445(t, w, cID) - for _, l := range w.h.Stalled() { - if l.ID == bID { - t.Fatalf("B is listed stalled while C's walk is within its bound: %+v", l) - } - } - if b, _ := boundOf(w.delivery(bID)); !strings.Contains(b.Says, cID) || !strings.Contains(b.Says, "until") { - t.Fatalf("B's bound says %q", b.Says) - } - }) - t.Run("that walk delivered and covers A: B takes its turn", func(t *testing.T) { - w := newShape445().open(t) - w.forge.contains[mergedA+" in "+mergedC] = true - end445(w, "plan-later", walkDone) - if s := w.state(cID); s != Delivered { - t.Fatalf("C is %s", s) - } - if d := w.delivery(aID); d.State != Superseded || d.CoveredBy == "" { - t.Fatalf("A is %s, covered by %q", d.State, d.CoveredBy) - } - letGo445(t, w, "plan-lab") - }) - t.Run("the forge says the later merge does not contain A: B is stopped", func(t *testing.T) { - w := newShape445().open(t) - w.forge.contains[mergedA+" in "+mergedC] = false - end445(w, "plan-later", walkDone) - stopped445(t, w, "can carry") - }) - t.Run("the later walk failed: B is stopped", func(t *testing.T) { - w := newShape445().open(t) - end445(w, "plan-later", walkFailed) - if s := w.state(cID); s != Failed { - t.Fatalf("C is %s", s) - } - stopped445(t, w, "can carry") - }) - t.Run("the later walk stays open past its own bound: B is stopped", func(t *testing.T) { - w := newShape445().open(t) - w.later(Bounds[Delivering].For - 10*time.Minute) - w.settleAll() - waits445(t, w, cID) - w.later(10 * time.Minute) - w.settleAll() - stopped445(t, w, "past its bound") - }) - t.Run("the forge never says whether the delivered walk contains A: B is stopped after the grace", func(t *testing.T) { - w := newShape445().open(t) - end445(w, "plan-later", walkDone) - waits445(t, w, cID) - w.later(containsGrace + time.Minute) - w.settleAll() - stopped445(t, w, "did not say") - }) -} - -// The review's shape of #203 (ADR 0276): A's waiting walk and B's were folded into one successor, plan-2, that -// answers C's merge and B's and not A's. The carrier is in B's own walk, which waits for B's turn: B takes its -// turn there, and the one walk delivers C and B; A is covered once it is done. -func TestAMemberWhoseWalkCarriesTheSupersededOneBeforeItTakesItsTurn(t *testing.T) { - s := foldShape() - s.forge = map[string]bool{mergedA + " in " + mergedC: true} - w := s.open(t) - letGo445(t, w, "plan-2") - if s := w.state(cID); s != Delivering { - t.Fatalf("C is %s", s) - } - end445(w, "plan-2", walkDone) - g := w.h.groups[g445] - if state, why := GroupState(g, w.h.membersOf(g)); state != "delivered" { - t.Fatalf("the group reads %s (%s), want delivered", state, why) - } -} - -// A carrier superseded in turn hands the wait to the later merge that took it over: C's waiting walk is folded -// into D's, which answers only D; B waits for D, and goes once D's walk covers A. -func TestTheWaitFollowsACarrierSupersededInTurn(t *testing.T) { - s := newShape445() - s.c.State, s.c.Walk = Published, &WalkSeen{ID: "plan-2", Commit: mergedC, State: walkBuilding, Waits: true, - WaitedFor: "mesh-delivery", Branch: "main", Created: at445.Add(4 * time.Minute)} - s.walks[2] = ctlWalk("plan-2", mergedC, at445.Add(4*time.Minute), walkBuilding, &WalkDelivery{Awaits: "mesh-delivery", - Merges: []WalkMerge{{Repository: ctl445, Commit: mergedC}}}) - s.extra = []*Delivery{{ID: dID445, Repository: ctl445, Commit: headD, Number: 111, Base: "main", Branch: "fix/later-still", - State: Ready, Since: at445.Add(6 * time.Minute), Created: at445.Add(-time.Hour), MergedAs: mergedD, - MergedAt: at445.Add(6 * time.Minute), Plan: planMoving(ctl445, modules445...), Check: &Verdict{Gate: "pass"}}} - w := s.open(t) - // C is a lone delivery at its turn; the fake controller lets plan-2 go. It is folded all the same, as the - // review's shape has it, before it started. - two := w.ctl.walk("plan-2") - two.Delivery.Go = nil - two.State, two.Note = walkSuperseded, "superseded at tier 0 by plan-3 ("+ctl445+" at 4d4d4d4d); planned there again" - w.h.WalkMoved(w.ctl.put(ctlWalk("plan-3", mergedD, at445.Add(7*time.Minute), walkBuilding, - &WalkDelivery{Awaits: "mesh-delivery", Merges: []WalkMerge{{Repository: ctl445, Commit: mergedD}}}))) - w.h.WalkMoved(w.ctl.put(two)) - w.settleAll() - if s := w.state(cID); s != Superseded { - t.Fatalf("C is %s", s) - } - waits445(t, w, dID445) - if strings.Contains(w.delivery(bID).TurnAwaits, cID) { - t.Fatalf("B still waits for C: %q", w.delivery(bID).TurnAwaits) - } - w.forge.contains[mergedA+" in "+mergedD] = true - end445(w, "plan-3", walkDone) - w.later(2 * time.Minute) // the forge, asked while it could not say, is asked again a minute on - w.settleAll() - if d := w.delivery(aID); d.CoveredBy == "" || !strings.Contains(d.CoveredBy, dID445) { - t.Fatalf("A is covered by %q", d.CoveredBy) - } - letGo445(t, w, "plan-lab") -} - -// A merged carrier still in an assembling batch has no state bound of its own: it is awaited for -// readyCarrierBound from its merge, and then B is stopped. -func TestACarrierInABatchIsAwaitedWithinTheWindowAndOneWalk(t *testing.T) { - s := newShape445() - s.c.State, s.c.Walk = Ready, nil - closes := at445.Add(3 * time.Minute) - s.walks[2] = Walk{ID: "plan-batch", Repository: ctl445, Branch: "main", Commit: mergedC, State: walkQueued, - Created: at445.Add(2 * time.Minute), Delivery: &WalkDelivery{Awaits: "mesh-delivery", Batch: &WalkBatch{ - ClosesAt: &closes, Behind: "plan-other"}, Merges: []WalkMerge{{Repository: ctl445, Commit: mergedC}}}} - w := s.open(t) - waits445(t, w, cID) - // The merge window at its longest, then the one open walk the batch may queue behind (ADR 0276 decisions 1, - // 4 and 9). - if readyCarrierBound != 10*time.Minute+Bounds[Delivering].For { - t.Fatalf("a ready carrier is awaited %s, want the window's 10 minutes and one walk's %s", readyCarrierBound, - Bounds[Delivering].For) - } - until := s.c.MergedAt.Add(readyCarrierBound) - if got := w.delivery(bID).TurnAwaitsUntil; got == nil || !got.Equal(until) { - t.Fatalf("B waits until %v, want %v", got, until) - } - // Within the bound, past every state bound a ready delivery could be read against, B still waits. - w.later(until.Add(-time.Minute).Sub(w.now)) - w.settleAll() - waits445(t, w, cID) - w.later(2 * time.Minute) - w.settleAll() - stopped445(t, w, "no walk carried it within "+readyCarrierBound.String()) -} - -// A member whose every module a later delivered merge carried, the forge saying it contains the member's merge, -// is reached even before CoveredBy is written on it: never a stop with an empty reason. -func TestACarriedMemberNotYetMarkedCoveredIsReached(t *testing.T) { - s := newShape445() - s.c.State, s.c.Since = Delivered, at445.Add(9*time.Minute) - s.c.Walk.State = walkDone - for _, m := range modules445 { - s.c.Steps = append(s.c.Steps, Step{Machine: "novox", Module: m, State: StepPassed}, - Step{Machine: stepTheRest, Module: m, State: StepSent}) - } - s.a.ContainedIn = map[string]bool{mergedC: true} - s.walks = s.walks[:2] - store := newMemStore() - for _, d := range []*Delivery{s.a, s.b, s.c} { - if err := store.PutDelivery(d); err != nil { - t.Fatal(err) - } - } - g := &Group{ID: g445, Members: []string{aID, bID}, Closed: true, Order: []string{aID, bID}, Said: "delivering", - Check: &GroupCheck{Verdict: "pass", Commits: commitsOf([]*Delivery{s.a, s.b})}} - if err := store.PutGroup(g); err != nil { - t.Fatal(err) - } - w := newWorldOver(t, store, newFakeController(), newFakeForge(), at445.Add(10*time.Minute)) - w.h.mu.Lock() - gg := w.h.groups[g445] - if w.h.deliveries[aID].CoveredBy != "" { - t.Fatal("A is marked covered already: the case is not the one under test") - } - w.h.turning = map[string]*turnWant{} - w.h.groupTurns(gg, w.h.membersOf(gg)) - b := w.h.deliveries[bID] - turned := w.h.turning["plan-lab"] != nil && containsString(w.h.turning["plan-lab"].ids, bID) - w.h.mu.Unlock() - if b.State != Published || !turned { - t.Fatalf("B is %s, at its turn %v; want its turn taken", b.State, turned) - } -} - -// foldShape is the review's shape: B and C on one walk, plan-2, not started, waiting for mesh-delivery's word. -func foldShape() *shape445 { - s := newShape445() - s.c.State, s.c.Walk = Published, &WalkSeen{ID: "plan-2", Commit: mergedC, State: walkBuilding, Waits: true, - WaitedFor: "mesh-delivery", Branch: "main", Created: at445.Add(4 * time.Minute)} - s.b.Walk = &WalkSeen{ID: "plan-2", Commit: mergedC, State: walkBuilding, Waits: true, WaitedFor: "mesh-delivery", - Branch: "main", Created: at445.Add(4 * time.Minute)} - two := ctlWalk("plan-2", mergedC, at445.Add(4*time.Minute), walkBuilding, &WalkDelivery{Awaits: "mesh-delivery", - Merges: []WalkMerge{{Repository: ctl445, Commit: mergedC}, {Repository: lab445, Commit: mergedB}}}) - two.Tiers = [][]string{modules445, {"mesh-lab"}} - two.Modules["mesh-lab"] = &WalkModule{} - s.walks = []Walk{s.walks[0], two} - return s -} - -// The turn on the carrier's walk is given only on the forge's word that the carrier contains A's merge: not yet -// said, B waits and the forge is asked; said "no", C is no carrier and B is stopped, never let go on plan-2. -func TestASharedTurnWaitsForTheForgesWord(t *testing.T) { - t.Run("not yet said: B waits and the forge is asked", func(t *testing.T) { - w := foldShape().open(t) - waits445(t, w, cID) - if containsString(w.ctl.delivers, "plan-2") { - t.Fatalf("plan-2 was let go %v before the forge said C contains A", w.ctl.delivers) - } - w.forge.mu.Lock() - asked := containsString(w.forge.containsAsks, mergedA+" in "+mergedC) - w.forge.mu.Unlock() - if !asked { - t.Fatal("the forge was not asked whether C's merge contains A's") - } - w.forge.contains[mergedA+" in "+mergedC] = true - w.later(2 * time.Minute) - w.settleAll() - letGo445(t, w, "plan-2") - }) - t.Run("said no: no shared turn", func(t *testing.T) { - s := foldShape() - s.forge = map[string]bool{mergedA + " in " + mergedC: false} - w := s.open(t) - if d := w.delivery(bID); d.State == Delivering || d.LetGo != (time.Time{}) { - t.Fatalf("B was given its turn on plan-2 (%s, let go %v)", d.State, d.LetGo) - } - stopped445(t, w, "does not contain its merge") - }) -} - -// Two carriers for A: C on B's own walk carries one module, D on a walk of its own the other. B is not given its -// turn on C's walk, which cannot deliver A whole: it waits for D's walk, goes once D covers its module, and is -// stopped, never delivering, when D's walk fails (re-review of #203). -func TestASharedTurnNeedsEveryModuleOnTheOneWalk(t *testing.T) { - two := func() *shape445 { - s := foldShape() - s.a.Plan = planMoving(ctl445, "mesh-controller", "route-proxy") - s.c.Plan = planMoving(ctl445, "mesh-controller") - s.extra = []*Delivery{{ID: dID445, Repository: ctl445, Commit: headD, Number: 111, Base: "main", - Branch: "fix/other", State: Delivering, Since: at445.Add(5 * time.Minute), Created: at445.Add(-time.Hour), - MergedAs: mergedD, MergedAt: at445.Add(90 * time.Second), Plan: planMoving(ctl445, "route-proxy"), - Check: &Verdict{Gate: "pass"}, Walk: &WalkSeen{ID: "plan-other", Commit: mergedD, State: walkRolling, - WaitedFor: "mesh-delivery", GoBy: byOwner, Branch: "main", Created: at445.Add(4 * time.Minute)}}} - went := at445.Add(5 * time.Minute) - other := ctlWalk("plan-other", mergedD, at445.Add(4*time.Minute), walkRolling, &WalkDelivery{ - Awaits: "mesh-delivery", Go: &went, By: byOwner, Merges: []WalkMerge{{Repository: ctl445, Commit: mergedD}}}) - other.Modules = map[string]*WalkModule{"route-proxy": {}} - s.walks = append(s.walks, other) - s.forge = map[string]bool{mergedA + " in " + mergedC: true, mergedA + " in " + mergedD: true} - return s - } - t.Run("D's walk open: B waits for both", func(t *testing.T) { - w := two().open(t) - waits445(t, w, dID445) - if !strings.Contains(w.delivery(bID).TurnAwaits, cID) || containsString(w.ctl.delivers, "plan-2") { - t.Fatalf("B is %s, let go %v, waiting for %q", w.state(bID), w.ctl.delivers, w.delivery(bID).TurnAwaits) - } - }) - t.Run("D's walk delivered: B goes on C's walk", func(t *testing.T) { - w := two().open(t) - end445(w, "plan-other", walkDone) - letGo445(t, w, "plan-2") - }) - t.Run("D's walk failed: B is stopped, never delivering", func(t *testing.T) { - w := two().open(t) - end445(w, "plan-other", walkFailed) - d := w.delivery(bID) - for _, tr := range d.Transitions { - if tr.To == Delivering { - t.Fatalf("B was delivering before it was stopped: %+v", d.Transitions) - } - } - stopped445(t, w, "route-proxy") - }) -} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/issue459_test.go b/modules/mesh-delivery/cmd/mesh-delivery/issue459_test.go index 20cdfe89..568941bb 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/issue459_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/issue459_test.go @@ -101,78 +101,6 @@ func lastWhy(d *Delivery) string { return d.Transitions[len(d.Transitions)-1].Why } -// A group member stopped on a walk it shares with a lone delivery: the walk is stopped without the member's merge, -// never whole, and the lone delivery follows the walk that walks its merge again and is delivered by it. -func TestAGroupStopWalksTheOtherDeliveriesAgainWithoutIt(t *testing.T) { - s := open459(t, []string{"novox/a", "novox/b"}, []string{"novox/z"}) - if containsString(s.ctl.delivers, "plan-2") { - t.Fatalf("plan-2 was let go before b's turn: %v", s.ctl.delivers) - } - s.failFirst() - if b := s.delivery(s.ids["novox/b"]); b.State != Stopped || !strings.Contains(lastWhy(b), s.ids["novox/a"]) { - t.Fatalf("b is %s (%q); want it stopped by its group, naming a", b.State, lastWhy(b)) - } - if containsString(s.ctl.stops, "plan-2") { - t.Fatalf("plan-2 was ended whole %v, and with it z, which is in no group and was not stopped", s.ctl.stops) - } - if want := "plan-2 without novox/b@" + s.merges["novox/b"]; !containsString(s.ctl.withouts, want) { - t.Fatalf("the controller was asked %v; want %q", s.ctl.withouts, want) - } - s.heard("plan-2", "plan-2-again") - if containsString(s.ctl.delivers, "plan-2-again") { - t.Fatal("a batch not yet cut was given its word") - } - s.cutAgain("plan-2-again") - z := s.delivery(s.ids["novox/z"]) - if z.State.Final() || z.Walk == nil || z.Walk.ID != "plan-2-again" || !containsString(s.ctl.delivers, "plan-2-again") { - t.Fatalf("z is %s on %+v, let go %v; want it following plan-2-again, let go", z.State, z.Walk, s.ctl.delivers) - } - s.done("plan-2-again") - if z, b := s.state(s.ids["novox/z"]), s.state(s.ids["novox/b"]); z != Delivered || b != Stopped { - t.Fatalf("once plan-2-again is done, z is %s and b %s; want z delivered and b stopped", z, b) - } -} - -// Two members after the failed one, both on the shared walk: one ask leaves both out, so neither is walked again -// because its own stop came second. -func TestAGroupStopLeavesOutEveryMemberItStopsOnOneWalk(t *testing.T) { - s := open459(t, []string{"novox/a", "novox/b", "novox/c"}, []string{"novox/z"}) - s.failFirst() - if len(s.ctl.withouts) != 1 || !strings.Contains(s.ctl.withouts[0], "novox/b@"+s.merges["novox/b"]) || - !strings.Contains(s.ctl.withouts[0], "novox/c@"+s.merges["novox/c"]) { - t.Fatalf("the controller was asked %v; want one stop of plan-2 leaving out b and c", s.ctl.withouts) - } - s.heard("plan-2", "plan-2-again") - again := s.ctl.walk("plan-2-again") - if len(again.Delivery.Merges) != 1 || again.Delivery.Merges[0].Repository != "novox/z" { - t.Fatalf("plan-2-again walks %+v; want z's merge alone", again.Delivery.Merges) - } - for _, r := range []string{"novox/b", "novox/c"} { - if st := s.state(s.ids[r]); st != Stopped { - t.Fatalf("%s is %s", r, st) - } - } -} - -// A lone delivery whose merge is a later merge of the stopped member's repository contains the member's merge: it -// is left out and ends stopped, naming the merge it contains; the other lone delivery is walked again. -func TestAGroupStopEndsAMergeThatContainsTheStoppedOne(t *testing.T) { - s := open459(t, []string{"novox/a", "novox/b"}, []string{"novox/z", "novox/b2"}) - // b2 is a later merge of b's repository: the walk carries it for b. - s.retarget("novox/b2", "novox/b") - s.failFirst() - s.heard("plan-2", "plan-2-again") - s.cutAgain("plan-2-again") - b2 := s.delivery(s.ids["novox/b2"]) - if b2.State != Stopped || !strings.Contains(lastWhy(b2), shortOf(s.merges["novox/b"], 8)) || - !strings.Contains(lastWhy(b2), "contains") { - t.Fatalf("b2 is %s (%q); want it stopped, naming b's merge it contains", b2.State, lastWhy(b2)) - } - if z := s.delivery(s.ids["novox/z"]); z.Walk == nil || z.Walk.ID != "plan-2-again" || z.State.Final() { - t.Fatalf("z is %s on %+v; want it following plan-2-again", z.State, z.Walk) - } -} - // retarget makes the delivery of repo `later` a later merge of repository `of`: same repository, carried by plan-2 // for the merge of `of`. func (s *shape459) retarget(later, of string) { @@ -279,39 +207,6 @@ func TestAPersonsStopOfTheWalkStopsEveryDelivery(t *testing.T) { } } -// The reviewer's case (review of #221): group [a, b]; y and z ride plan-2 with b. y is stopped `way: again`, and the -// batch that walks plan-2's others again carries b and z. Then a fails: b is stopped by its group, and its merge is -// left out of that batch — never shipped by it — and z alone is walked and delivered. -func TestAMergeWalkedAgainAndThenStoppedByItsGroupNeverShips(t *testing.T) { - s := open459(t, []string{"novox/a", "novox/b"}, []string{"novox/y", "novox/z"}) - if _, err := s.h.StopWay(s.ids["novox/y"], "not wanted after all", "jochen", "again"); err != nil { - t.Fatal(err) - } - s.heard("plan-2", "plan-2-again") - s.failFirst() - if b := s.state(s.ids["novox/b"]); b != Stopped { - t.Fatalf("b is %s", b) - } - if want := "plan-2-again without novox/b@" + s.merges["novox/b"]; !containsString(s.ctl.withouts, want) { - t.Fatalf("the controller was asked %v; want %q", s.ctl.withouts, want) - } - s.heard("plan-2-again", "plan-2-again-left") - s.cutAgain("plan-2-again") - again := s.ctl.walk("plan-2-again") - for _, m := range again.Delivery.Merges { - if m.Repository == "novox/b" { - t.Fatalf("the walk %s still carries b's stopped merge: %+v", again.ID, again.Delivery.Merges) - } - } - if !containsString(s.ctl.delivers, "plan-2-again") { - t.Fatalf("z's walk was not let go: %v", s.ctl.delivers) - } - s.done("plan-2-again") - if z := s.state(s.ids["novox/z"]); z != Delivered { - t.Fatalf("z is %s", z) - } -} - // Whatever brought it there, a walk that carries a stopped delivery's merge is asked to leave it out, and is never // given its word while it carries it. func TestAWalkCarryingAStoppedMergeIsAskedToLeaveItOutAndNeverLetGo(t *testing.T) { @@ -338,21 +233,6 @@ func TestAWalkCarryingAStoppedMergeIsAskedToLeaveItOutAndNeverLetGo(t *testing.T } } -// A group stop the controller refuses to make without the merge (an older controller) stops the whole walk: the -// stopped change is held back, and the delivery riding with it ends stopped, saying why. -func TestARefusedLeaveOutStopsTheWholeWalk(t *testing.T) { - s := open459(t, []string{"novox/a", "novox/b"}, []string{"novox/z"}) - s.ctl.refuseWithout = true - s.failFirst() - if len(s.ctl.withouts) != 1 || !containsString(s.ctl.stops, "plan-2") { - t.Fatalf("asked %v, ended whole %v; want the leave-out tried, then plan-2 ended whole", s.ctl.withouts, s.ctl.stops) - } - s.heard("plan-2") - if z := s.delivery(s.ids["novox/z"]); z.State != Stopped || !strings.Contains(lastWhy(z), "could not be left out") { - t.Fatalf("z is %s (%q); want it stopped, saying the leave-out was refused", z.State, lastWhy(z)) - } -} - // A person's stop the controller cannot make leaves the delivery as it was, and says how to go on. func TestAPersonsStopThatCannotBeMadeStopsNothing(t *testing.T) { s := shared459(t) @@ -382,23 +262,6 @@ func TestAStopSaysWhatStaysAndWhatStillCarriesIt(t *testing.T) { } } -// A merge left out for moving a module the stopped merge moves, from another repository, ends stopped saying so — -// never that it contains the stopped merge (review of #221). -func TestALeftOutMergeSharingAModuleSaysSoTruly(t *testing.T) { - s := open459(t, []string{"novox/a", "novox/b"}, []string{"novox/z"}) - s.ctl.mu.Lock() - s.ctl.moves[s.merges["novox/b"]], s.ctl.moves[s.merges["novox/z"]] = []string{"lab"}, []string{"lab"} - s.ctl.mu.Unlock() - s.failFirst() - s.heard("plan-2") - z := s.delivery(s.ids["novox/z"]) - why := lastWhy(z) - if z.State != Stopped || !strings.Contains(why, "moves lab, as the stopped merge "+shortOf(s.merges["novox/b"], 12)) || - strings.Contains(why, "contains the stopped merge") { - t.Fatalf("z is %s (%q); want it stopped, saying it moves lab as b's merge does", z.State, why) - } -} - // A person's first answer names the deliveries that would be left out for moving a module this merge moves, and says // the controller may leave out more. func TestAPersonsFirstAnswerNamesWhatSharesItsModules(t *testing.T) { diff --git a/modules/mesh-delivery/cmd/mesh-delivery/issue747_test.go b/modules/mesh-delivery/cmd/mesh-delivery/issue747_test.go index 7646ec14..4c159e07 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/issue747_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/issue747_test.go @@ -84,8 +84,8 @@ func night747Over(t *testing.T, set func(*fakeForge), built []string) *world { t.Fatal(err) } } - g := &Group{ID: g747, Members: []string{id301, id336}, Closed: true, Order: []string{id336, id301}, Said: "failed", - OrderOf: commitsOf([]*Delivery{m301, m336}), Check: &GroupCheck{Asked: "check-1", At: at.Add(-time.Minute), + g := &Group{ID: g747, Members: []string{id301, id336}, Closed: true, Said: "failed", + Check: &GroupCheck{Asked: "check-1", At: at.Add(-time.Minute), Verdict: "pass", Summary: "both compose", Commits: commitsOf([]*Delivery{m301, m336})}} if err := store.PutGroup(g); err != nil { t.Fatal(err) diff --git a/modules/mesh-delivery/cmd/mesh-delivery/issue753_test.go b/modules/mesh-delivery/cmd/mesh-delivery/issue753_test.go index 8bb50c99..235c4507 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/issue753_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/issue753_test.go @@ -80,8 +80,8 @@ func night753Over(t *testing.T, stop Transition, merges []WalkMerge, set func(*f for _, d := range inGroup { ids = append(ids, d.ID) } - g := &Group{ID: g747, Members: ids, Closed: true, Order: order, Said: "delivering", - OrderOf: commitsOf(inGroup), Check: &GroupCheck{Asked: "check-1", At: at.Add(-time.Minute), + g := &Group{ID: g747, Members: ids, Closed: true, Said: "delivering", + Check: &GroupCheck{Asked: "check-1", At: at.Add(-time.Minute), Verdict: "pass", Summary: "all compose", Commits: commitsOf(inGroup)}} if err := store.PutGroup(g); err != nil { t.Fatal(err) @@ -226,26 +226,6 @@ func m299() *Delivery { Why: "its group " + g747 + " stopped: " + id336 + " failed"}}} } -// **A stopped member gives its word only in its group's order** (review of issue 753): #299, ordered before #301 and -// stopped for #336 too, is not delivered and this walk does not carry its merge, so #301 gives no word — it would land -// before #299 — and `stalled walks` names #299. When the walk names #299's merge too, it delivers both, in order, and -// is let go. -func TestAStoppedMemberGivesItsWordOnlyInItsGroupsOrder(t *testing.T) { - w := night753With(t, groupStop753(), m299()) - if containsString(w.ctl.delivers, plan753) { - t.Fatalf("the walk was let go for #301 before #299, ordered before it: %v", w.ctl.delivers) - } - said, state := saidOf(w.h.WalksWaiting(), plan753) - if state == Unheld || !strings.Contains(said, m299().ID+" (stopped)") { - t.Fatalf("`stalled walks` reads %s %q; want #299 named", state, said) - } - w = night753Over(t, groupStop753(), []WalkMerge{{Repository: cat747, Commit: "e299aaaaaaaaaaaa", Carried: merge307}}, nil, - m299()) - if !containsString(w.ctl.delivers, plan753) { - t.Fatalf("the walk carrying both members was not let go: %v", w.ctl.delivers) - } -} - // **A word the controller refuses is never silent** (review of issue 753): the controller keeps refusing it, and past // the grace `stalled` lists the walk, with the refusal. func TestARefusedWordIsListedByStalled(t *testing.T) { diff --git a/modules/mesh-delivery/cmd/mesh-delivery/issue902_test.go b/modules/mesh-delivery/cmd/mesh-delivery/issue902_test.go index f2d8a43c..e602cc2b 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/issue902_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/issue902_test.go @@ -23,7 +23,6 @@ func group902(t *testing.T) (*world, string, string, string, string) { w := newWorld(t) const ctlHead, catHead = "c1c1c1c1c1c1c1c1", "d2d2d2d2d2d2d2d2" ctl, cat := IDOf("novox/mesh-controller", ctlHead), IDOf("novox/mesh-catalog", catHead) - w.ctl.order = func(ms []Member) Order { return Order{Order: []string{cat, ctl}} } w.h.PullUpdated(pr("novox/mesh-controller", 1, ctlHead, "feat/x", "internal/a.go")) w.h.PullUpdated(pr("novox/mesh-catalog", 2, catHead, "feat/x", "modules/app/module.json")) w.h.Checked(verdict("novox/mesh-controller", 1, ctlHead, "pass")) diff --git a/modules/mesh-delivery/cmd/mesh-delivery/main.go b/modules/mesh-delivery/cmd/mesh-delivery/main.go index 4379330c..a8952b94 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/main.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/main.go @@ -259,14 +259,14 @@ func tools(h *Holder, l *listening) []stdio.Tool { }}, {Name: seat + "show", Description: "One delivery or group whole: its delivery plan, every transition with when and why, the " + - "machine steps of its walk, its group and its order.", + "machine steps of its walk and its group.", Input: map[string]any{"type": "object", "properties": map[string]any{"id": str("a delivery's id, or a group's")}, "required": []string{"id"}}, Run: func(a map[string]any) (any, error) { return h.Show(strArg(a, "id")) }}, {Name: seat + "groups", - Description: "Every delivery group: its members in order, why each pair is ordered and the rules it won " + - "over, each contradiction no precedence resolves with how to declare the order, its composed check " + - "and its state, derived from its members.", + Description: "Every delivery group: its members, its composed check and its state, derived from its members. " + + "A group has no order: it is merged in one pass and delivered by one plan, whose tiers come from the module " + + "graph (novox/hq ADR 0418). composed_gate says its members' gate is the composed check's (ADR 0403).", Input: map[string]any{"type": "object", "properties": map[string]any{ "all": map[string]any{"type": "string", "enum": []string{"true", "false"}}}}, Run: func(a map[string]any) (any, error) { return h.Groups(boolArg(a, "all")), nil }}, diff --git a/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go b/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go index 4bbe714e..3c2a3088 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go @@ -61,9 +61,9 @@ func TestItClaimsTheDeliverySeatAndFollowsWhatItHandles(t *testing.T) { if !reflect.DeepEqual(m.State, []string{"deliveries", "groups", "asked", "pulls"}) { t.Fatalf("state %v", m.State) } - // It calls exactly what its ports ask: the controller's six verbs and the forge's four tools. + // It calls exactly what its ports ask: the controller's verbs and the forge's tools. want := []string{} - for _, v := range []string{"delivery-plan", "delivery-order", "delivery-check", "deliver", "delivery-stop", "delivery-walks", "forget-checks"} { + for _, v := range []string{"delivery-plan", "delivery-check", "deliver", "delivery-stop", "delivery-walks", "forget-checks"} { want = append(want, "seat:"+ControllerSeat+"."+v) } want = append(want, "gitea.gitea_note_append", "gitea.gitea_delivery_view", "gitea.gitea_commit_status", diff --git a/modules/mesh-delivery/cmd/mesh-delivery/model.go b/modules/mesh-delivery/cmd/mesh-delivery/model.go index e635616d..f15dc573 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/model.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/model.go @@ -33,8 +33,11 @@ type Delivery struct { Removed []string `json:"removed,omitempty"` ModuleDirs []string `json:"module_dirs,omitempty"` ModuleDirsSaid bool `json:"module_dirs_said,omitempty"` - // After are the repositories its pull request says it goes after (`after: ` lines). - After []string `json:"after,omitempty"` + // After are the repositories its pull request's description names in `after: ` lines. They are not read + // (novox/hq ADR 0419 §2): kept only to say once on the pull request that the line is not read. AfterSaid is whether + // that was said, for this pull request. + After []string `json:"after,omitempty"` + AfterSaid bool `json:"after_said,omitempty"` // Issues are the issues its title and body name ("issue 948"), shown to the operator who is asked to approve its merge. Issues []int `json:"issues,omitempty"` // Group is the delivery group it is in: its head branch name. Empty for a lone delivery. @@ -93,14 +96,9 @@ type Delivery struct { // StoppedFor is, for a member its group stopped, the member whose end stopped it (novox/hq issue 747): once that // member's change reached the machines, the stop has lapsed, and holds no walk of its repository back. StoppedFor string `json:"stopped_for,omitempty"` - // TurnAwaits says, for a group member whose turn waits for a later merge to carry a superseded member before it, - // which merges and until when; TurnAwaitsUntil is that moment, which bounds the wait (ADR 0276 decision 9, - // novox/hq issue 445). `show` shows both, and `stalled` says them past that moment. - TurnAwaits string `json:"turn_awaits,omitempty"` // HeldBack is why the walk it waits on is not let go: it carries a merge whose delivery was stopped, and is // asked to leave it out (novox/hq ADR 0299). Said by `waits`, and by `stalled` past its bound. - HeldBack string `json:"held_back,omitempty"` - TurnAwaitsUntil *time.Time `json:"turn_awaits_until,omitempty"` + HeldBack string `json:"held_back,omitempty"` // ContainedIn is the forge's answer, by a later merge commit, to whether that commit contains this one's // merge (gitea_contains): asked once per pair, and kept (ADR 0270). ContainedIn map[string]bool `json:"contained_in,omitempty"` @@ -207,13 +205,20 @@ type CheckAsk struct { Reasked int `json:"reasked,omitempty"` } +// GateComposed is the gate layer of a group member's own verdict when the group's composed check judges it instead +// (novox/hq ADR 0403 §1): no gate was run for the member alone. +const GateComposed = "composed" + // Passes is whether the verdict lets the delivery be ready: the gate passed or warned, and the // repository's own check did not fail or error. func (v *Verdict) Passes() bool { if v == nil { return false } - gate := v.Gate == "pass" || v.Gate == "warning" + // A group member's own gate is the group's composed check's (novox/hq ADR 0403 §1): the controller says the layer + // "composed", and the member counts as passed here once its repository's check did not fail, as the composed + // check, asked after every member is ready, then judges them together. + gate := v.Gate == "pass" || v.Gate == "warning" || v.Gate == GateComposed repo := v.Repo == "" || v.Repo == "pass" || v.Repo == "warning" return gate && repo } @@ -901,17 +906,12 @@ type Group struct { Members []string `json:"members"` Created time.Time `json:"created"` Updated time.Time `json:"updated"` - // Order is the members in the order they are delivered, Pairs why, Cycle the members whose order - // contradicts itself; OrderOf the member commits the order was worked out for. - Order []string `json:"order,omitempty"` - Pairs []Pair `json:"pairs,omitempty"` - Cycle []string `json:"cycle,omitempty"` - // Contradictions is each two members whose rules order them both ways at one rank, said with how a - // person declares the order (novox/hq ADR 0249). - Contradictions []Contradiction `json:"contradictions,omitempty"` - OrderOf map[string]string `json:"order_of,omitempty"` // Check is its composed check: asked of the controller for these member commits, and its verdict. Check *GroupCheck `json:"check,omitempty"` + // SaidGate is, by member id, why the member's mesh/merge-gate was last said pending: written again only when it + // changes (novox/hq ADR 0403 §1, §6, issue 978). Cleared once a composed verdict stands at the heads, which the + // forge module then sets as every member's gate. + SaidGate map[string]string `json:"said_gate,omitempty"` // Closed is set once a member reached the trunk: a group delivering takes no new member. Closed bool `json:"closed,omitempty"` // Said is the derived state last said, so a change of it is said once. @@ -922,32 +922,6 @@ type Group struct { Owed []Effect `json:"owed,omitempty"` } -// Pair is one "before" among a group's members: the rule that won, and the rules that ordered it the other -// way and lost by precedence (novox/hq ADR 0249). -type Pair struct { - Before string `json:"before"` - After string `json:"after"` - Why string `json:"why"` - Over []string `json:"over,omitempty"` -} - -// Contradiction is two members that rules of one rank order both ways, which no precedence resolves: the -// rules, and Said, which names them and says how to declare the order. -type Contradiction struct { - Members []string `json:"members"` - Rules []Pair `json:"rules"` - Said string `json:"said"` -} - -// orderSaid is a pair as a person reads it: what won, and over what. -func (p Pair) orderSaid() string { - s := p.Before + " before " + p.After + ": " + p.Why - if len(p.Over) > 0 { - s += ", over " + strings.Join(p.Over, ", ") - } - return s -} - // GroupCheck is a group's composed check. type GroupCheck struct { Asked string `json:"asked"` diff --git a/modules/mesh-delivery/cmd/mesh-delivery/ports.go b/modules/mesh-delivery/cmd/mesh-delivery/ports.go index cdad7701..d2370732 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/ports.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/ports.go @@ -31,7 +31,6 @@ type Member struct { ModuleDirs []string `json:"module_dirs,omitempty"` ModuleDirsSaid bool `json:"module_dirs_said,omitempty"` CloneURL string `json:"clone_url,omitempty"` - After []string `json:"after,omitempty"` // Ready is whether the operator's Approve of its merge is asked or given: its check is published on the ready stream (ADR 0406 §3, 0421 §10). Ready bool `json:"ready,omitempty"` // BaseSHA is the base branch's head the forge last announced the head with (novox/hq ADR 0323): the trunk commit @@ -43,24 +42,13 @@ type Member struct { func MemberOf(d *Delivery) Member { return Member{Ready: d.Ready != nil && d.Ready.Merged == nil && (d.Ready.AskID != "" || d.Ready.Approved != nil), ID: d.ID, Repository: d.Repository, Base: d.Base, Head: d.Commit, Number: d.Number, Paths: d.Paths, PathsTruncated: d.PathsTruncated, Removed: d.Removed, ModuleDirs: d.ModuleDirs, ModuleDirsSaid: d.ModuleDirsSaid, - CloneURL: d.CloneURL, After: d.After, BaseSHA: d.BaseSHA} -} - -// Order is a group's order as the controller works it out: each pair with the rules it won over by -// precedence, and each contradiction no precedence resolves (novox/hq ADR 0249). -type Order struct { - Order []string `json:"order"` - Pairs []Pair `json:"pairs,omitempty"` - Cycle []string `json:"cycle,omitempty"` - Contradictions []Contradiction `json:"contradictions,omitempty"` + CloneURL: d.CloneURL, BaseSHA: d.BaseSHA} } // Controller is the controller's verbs this owner asks with. type Controller interface { // Plan is the delivery plan of a diffset, kept nowhere. Plan(repository, base, head string, paths, moduleDirs, removed []string) (*DeliveryPlan, error) - // Order is a group's order from the graph and its members' `after:` lines. - Order(members []Member) (Order, error) // Check asks a group's composed check, or — with no group and one member — that head's own check again. Check(group string, members []Member) (string, error) // Deliver lets a waiting walk start. @@ -246,18 +234,6 @@ func (c seatController) Plan(repository, base, head string, paths, moduleDirs, r return a.Plan, nil } -func (c seatController) Order(members []Member) (Order, error) { - raw, err := c.verb("delivery-order", map[string]any{"members": string(mustJSON(members))}) - if err != nil { - return Order{}, err - } - var o Order - if err := json.Unmarshal(raw, &o); err != nil { - return Order{}, fmt.Errorf("the controller's delivery-order is not readable: %w", err) - } - return o, nil -} - func (c seatController) Check(group string, members []Member) (string, error) { args := map[string]any{"members": string(mustJSON(members))} if group != "" { diff --git a/modules/mesh-delivery/cmd/mesh-delivery/pullkey.go b/modules/mesh-delivery/cmd/mesh-delivery/pullkey.go index 9086de56..81bdaa67 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/pullkey.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/pullkey.go @@ -13,7 +13,7 @@ import ( // holding its head, the trunk commit its current check is asked against and its verdict, written by compare-and-set. // It writes a new head when the forge says one (`pull.updated`, which carries the head and the trunk commit it was // announced against), and a new trunk commit only when the check is asked again against one — the merge tool's ask is -// that same announcement. A merge in a group's order, and a trunk move that leaves an inactive pull request unasked, +// that same announcement. A merge of a group's member, and a trunk move that leaves an inactive pull request unasked, // are announced as nothing (the gitea module announces `pull.updated` only for a head and for the merge tool's ask), so // they write nothing. A verdict for that head and trunk commit writes the verdict. The build seat reads the key. diff --git a/modules/mesh-delivery/cmd/mesh-delivery/ready.go b/modules/mesh-delivery/cmd/mesh-delivery/ready.go index ad246038..19d92742 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/ready.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/ready.go @@ -130,6 +130,9 @@ func (h *Holder) readyAsks(now time.Time) []func() { h.Poke() // its check, if none is asked, is worth asking now (askCheck) h.Logf("[mesh-delivery] %s: its checks passed; the operator's Approve of its merge into main is asked once it may merge", d.ID) } + if a := h.sayAfterLine(d); a != nil { + asks = append(asks, a) + } if a := h.askFresh(d, now); a != nil { asks = append(asks, a) } @@ -203,6 +206,26 @@ func (h *Holder) readyAsks(now time.Time) []func() { return asks } +// sayAfterLine tells a pull request, once, that its description holds an `after: ` line, which is not read +// (novox/hq ADR 0419 §2): the order of a delivery comes from the module graph alone, and no refusal asks for the line. +// Once per pull request, not per head. Under the lock; the comment is said outside it. +func (h *Holder) sayAfterLine(d *Delivery) func() { + if len(d.After) == 0 || d.AfterSaid { + return nil + } + d.AfterSaid = true + for _, o := range h.deliveries { + if o != d && o.Number == d.Number && o.Repository == d.Repository && o.AfterSaid { + h.keep(d) + return nil // said on this pull request for an earlier head + } + } + h.keep(d) + h.Logf("[mesh-delivery] %s: its description holds an after: line (%s), which is not read", d.ID, strings.Join(d.After, ", ")) + return h.sayOn(d, "mesh-delivery: this description holds an `after:` line ("+strings.Join(d.After, ", ")+"). It is not read: "+ + "the order of a delivery comes from the module graph alone, and nothing asks for the line (novox/hq ADR 0419).") +} + // askFresh is the ask of a head's check against its trunk's head, when its passing verdict is out of date and either // arrived so, or has an Approve asked or given for it (novox/hq ADR 0421 §3, issue 1022; issue 1047 for a group's // member): the Approve stays through a trunk move, and the merge is asked again on the verdict this brings, with no @@ -245,7 +268,7 @@ func (h *Holder) askFresh(d *Delivery, now time.Time) func() { } // mergeAgainNow lets an approved delivery's merge be asked at once: something that decides whether it may merge happened (a -// verdict, an Approve, a group's turn), so the wait after a refusal does not hold it. Under the lock. +// verdict, an Approve, a group's composed check), so the wait after a refusal does not hold it. Under the lock. func (h *Holder) mergeAgainNow(d *Delivery) { if d != nil && d.Ready != nil && d.Ready.Merged == nil { d.Ready.LastTry = time.Time{} @@ -275,8 +298,9 @@ func (h *Holder) readyWaits(d *Delivery) string { return h.groupTurnToMerge(d) } -// groupTurnToMerge is why a group member may not merge yet; empty at its turn: the group's composed check passed for -// its members' heads as they are and is not out of date, and every member before it in the group's order merged. +// groupTurnToMerge is why a group member may not merge yet; empty when it may: the group's composed check passed for +// its members' heads as they are and is not out of date. No member waits for another's merge or delivery (novox/hq +// ADR 0418 §4): once the composed check passed and the Approve covers the heads, every member is merged in one pass. func (h *Holder) groupTurnToMerge(d *Delivery) string { g := h.groups[d.Group] if g == nil { @@ -290,17 +314,6 @@ func (h *Holder) groupTurnToMerge(d *Delivery) string { !sameMembers(c.Commits, commitsOf(members)) { return "its group " + g.ID + "'s composed check has not passed for its members' heads" } - if !containsString(g.Order, d.ID) { - return "its group " + g.ID + "'s order is not worked out for it yet" - } - for _, id := range g.Order { - if id == d.ID { - return "" - } - if o := h.deliveries[id]; o != nil && o.MergedAs == "" && !o.State.Final() { - return "its group " + g.ID + "'s order merges " + id + " first" - } - } return "" } @@ -397,7 +410,7 @@ func (h *Holder) readyLines(now time.Time) []StalledLine { case r.RefusedSince != nil && now.Sub(*r.RefusedSince) >= readyRefusedFor: out = append(out, StalledLine{ID: d.ID, Number: d.Number, State: d.State, For: now.Sub(*r.RefusedSince).Round(time.Second).String(), Bound: readyRefusedFor.String(), - H2: "none: the merge is asked again after each verdict, Approve or turn, and each minute — the operator's", + H2: "none: the merge is asked again after each verdict, Approve or composed verdict, and each minute — the operator's", Says: fmt.Sprintf("%s#%d is approved at %s, and the merge tool has refused it since %s: %s", d.Repository, d.Number, shortOf(r.Head, 12), r.RefusedSince.UTC().Format(time.RFC3339), r.Answer)}) } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/ready_test.go b/modules/mesh-delivery/cmd/mesh-delivery/ready_test.go index 2b705c56..49fd331c 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/ready_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/ready_test.go @@ -2,6 +2,7 @@ package main import ( "errors" + "sort" "strings" "testing" "time" @@ -89,34 +90,39 @@ func TestAMergeRefusedForAnHourIsSaid(t *testing.T) { } } -func TestAGroupMergesItsMarkedMembersInOrderOnlyOnceComposed(t *testing.T) { +func TestAGroupMergesEveryMemberInOnePassOnlyOnceComposedAndApproved(t *testing.T) { w := newWorld(t) + w.op.auto = false const ctlHead, catHead = "c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1", "d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2" - ctl, cat := IDOf("novox/mesh-controller", ctlHead), IDOf("novox/mesh-catalog", catHead) - w.ctl.order = func(ms []Member) Order { return Order{Order: []string{ctl, cat}} } w.h.PullUpdated(pr("novox/mesh-controller", 1, ctlHead, "feat/x", "internal/a.go")) w.h.PullUpdated(pr("novox/mesh-catalog", 2, catHead, "feat/x", "modules/app/module.json")) w.h.Checked(judgedAt("novox/mesh-controller", 1, ctlHead, "pass", "t1t1t1t1t1t1")) w.h.Checked(judgedAt("novox/mesh-catalog", 2, catHead, "pass", "t3t3t3t3t3t3")) - w.forge.open = []OpenPull{openPull("novox/mesh-controller", 1, ctlHead), - openPull("novox/mesh-catalog", 2, catHead)} - w.h.SeeOpenPulls() w.settleAll() - if len(w.forge.merges) != 0 { - t.Fatalf("a member merged before its group's composed check passed: %v", w.forge.merges) + if len(w.forge.merges) != 0 || w.op.count() != 0 { + t.Fatalf("a member was merged or asked before its group's composed check passed: %v, %d", w.forge.merges, w.op.count()) } w.h.Checked(CheckedEvent{Group: "feat/x", ID: "check-1", Verdict: "pass", Summary: "both compose"}) w.settleAll() - if len(w.forge.merges) != 1 || w.forge.merges[0] != "novox/mesh-controller#1@"+ctlHead { - t.Fatalf("composed, the first member in the group's order was not merged alone: %v", w.forge.merges) + if w.op.count() != 1 || len(w.forge.merges) != 0 { + t.Fatalf("composed, the group is asked once and nothing merges before the Approve: asks %d, merges %v", w.op.count(), w.forge.merges) } - if waits := w.delivery(cat).Ready.Waits; !strings.Contains(waits, "merges "+ctl+" first") { - t.Fatalf("the second member does not say it waits its turn: %q", waits) - } - w.h.PullMerged(merged("novox/mesh-controller", 1, ctlHead, "m1m1m1m1m1m1")) + w.op.answer(w.op.open()[0], OptionApprove) w.settleAll() - if len(w.forge.merges) != 2 || w.forge.merges[1] != "novox/mesh-catalog#2@"+catHead { - t.Fatalf("after the first member merged, the next was not: %v", w.forge.merges) + if len(w.forge.merges) != 2 { + t.Fatalf("both members are merged in the same pass on the one Approve: %v", w.forge.merges) + } + sort.Strings(w.forge.merges) + if w.forge.merges[0] != "novox/mesh-catalog#2@"+catHead || w.forge.merges[1] != "novox/mesh-controller#1@"+ctlHead { + t.Fatalf("merged %v, wanted both, each at its approved head", w.forge.merges) + } + for _, got := range w.forge.approvedHeads { + if len(got) != 2 || got[0] != "novox/mesh-catalog@"+catHead || got[1] != "novox/mesh-controller@"+ctlHead { + t.Fatalf("a member's merge was handed the approved heads %v", got) + } + } + if w.op.count() != 1 { + t.Fatalf("a second Approve was asked: %d", w.op.count()) } } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/table.go b/modules/mesh-delivery/cmd/mesh-delivery/table.go index 70fd3972..08edff7f 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/table.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/table.go @@ -622,13 +622,6 @@ func boundOf(d *Delivery) (Bound, bool) { b.For, b.Says = Bounds[Published].For, d.HeldBack // a walk not let go waits for a word, whatever its state return b, true } - if d.State == Published && d.TurnAwaits != "" && d.TurnAwaitsUntil != nil { - // Its turn waits for a later merge to carry a member before it: bounded by that carrier's own bound, and - // said with it, never the generic "waits for its turn or its word" (novox/hq issue 445). - b := Bounds[Published] - b.For, b.Says = max(b.For, d.TurnAwaitsUntil.Sub(boundSince(d))), d.TurnAwaits - return b, true - } b, bounded := Bounds[d.State] return b, bounded } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/verbs.go b/modules/mesh-delivery/cmd/mesh-delivery/verbs.go index 14f163f6..9657c2b8 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/verbs.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/verbs.go @@ -62,7 +62,7 @@ func (h *Holder) waitsFor(d *Delivery) string { " ended without building it, and it is stopped " + strandedAfter.String() + " after that walk ended otherwise" } if d.Group != "" { - return "its turn in group " + d.Group + return "its group " + d.Group + ": every member merged, and the plan's word" } return "its walk's start" case Held: @@ -144,16 +144,14 @@ func (h *Holder) Show(id string) (any, error) { // GroupLine is one group as `groups` lists it. type GroupLine struct { - ID string `json:"id"` - State string `json:"state"` - Why string `json:"why"` - Order []string `json:"order"` - Pairs []Pair `json:"pairs,omitempty"` - Cycle []string `json:"cycle,omitempty"` - // Contradictions is each contradiction no precedence resolves, said with how to declare the order. - Contradictions []Contradiction `json:"contradictions,omitempty"` - Check string `json:"composed,omitempty"` - Members []string `json:"members"` + ID string `json:"id"` + State string `json:"state"` + Why string `json:"why"` + // ComposedGate says the group's gate is the composed check's: each member's own pull request check runs the + // repository's check alone, and the composed verdict sets its mesh/merge-gate (novox/hq ADR 0403 §1, issue 978). + ComposedGate bool `json:"composed_gate"` + Check string `json:"composed,omitempty"` + Members []string `json:"members"` // Details are the members again, one object each, with what a reader needs to find the pull request's // siblings: its number, full head, base branch and whether it is open (novox/hq ADR 0382 §1). Additive: // `members` keeps its text form for older readers. @@ -185,8 +183,7 @@ func (h *Holder) Groups(all bool) []GroupLine { if !all && (state == "delivered" || state == "failed" || state == "stopped") { continue } - l := GroupLine{ID: g.ID, State: state, Why: why, Order: g.Order, Pairs: g.Pairs, Cycle: g.Cycle, - Contradictions: g.Contradictions} + l := GroupLine{ID: g.ID, State: state, Why: why, ComposedGate: true} for _, d := range members { l.Members = append(l.Members, fmt.Sprintf("%s (%s)", d.ID, d.State)) l.Details = append(l.Details, MemberLine{ID: d.ID, Repository: d.Repository, State: d.State, @@ -501,7 +498,7 @@ func (h *Holder) StopWay(id, why, by, way string) (string, error) { h.moved(d, []Transition{t}) var asks []func() if g := h.groups[d.Group]; g != nil && g.Closed { - asks = h.groupTurns(g, h.membersOf(g)) + asks = h.groupWord(g, h.membersOf(g)) } else if g != nil { h.leaveGroup(d) } diff --git a/modules/mesh-delivery/module.json b/modules/mesh-delivery/module.json index 058138ee..2cab76d1 100644 --- a/modules/mesh-delivery/module.json +++ b/modules/mesh-delivery/module.json @@ -38,7 +38,6 @@ ], "invokes": [ "seat:mesh-controller.delivery-plan", - "seat:mesh-controller.delivery-order", "seat:mesh-controller.delivery-check", "seat:mesh-controller.deliver", "seat:mesh-controller.delivery-stop",