diff --git a/cmd/mesh-controller/catch_up_integration_test.go b/cmd/mesh-controller/catch_up_integration_test.go index 701dbb53..ac9cf58e 100644 --- a/cmd/mesh-controller/catch_up_integration_test.go +++ b/cmd/mesh-controller/catch_up_integration_test.go @@ -61,7 +61,7 @@ func aTrunkOf(t *testing.T, repo string, modules ...string) *aTrunk { _, m.Number, _ = pullOfSubject(tm.Subject) return m, nil } - findMergeWarrant = func(_ context.Context, _ catalogueReader, number int, head string) (string, bool, string, error) { + findMergeWarrant = func(_ context.Context, _ catalogueReader, _, _ string, number int, head string) (string, bool, string, error) { for _, m := range tr.merges { if strings.Contains(m.Subject, fmt.Sprintf("(#%d)", number)) && m.Head == head && tr.approved[m.Commit] { return "the operator chose Approve (ask merge-" + fmt.Sprint(number) + ")", true, "", nil @@ -175,17 +175,14 @@ func TestAMergeOlderThanTheLastFiftyIsNotRead(t *testing.T) { c := fmt.Sprintf("mesh-catalog-c%02d", i) hear(t, open, catalogueMerge(c, "app", t0.Add(time.Duration(i)*time.Minute)), t0.Add(time.Duration(i)*time.Minute+time.Second)) } - // Paged as the forge pages: fifty a page. + // Paged as the forge pages: fifty a page, and a trunk of another repository is empty (a short page, or none). pages := trunkCommitsPage trunkCommitsPage = func(ctx context.Context, owner, name, branch string, page int) ([]trunkCommit, error) { all, err := pages(ctx, owner, name, branch, 1) - if err != nil || page > 2 { + if err != nil || page < 1 || (page-1)*50 >= len(all) { return nil, err } - if page == 1 { - return all[:50], nil - } - return all[50:], nil + return all[(page-1)*50 : min(page*50, len(all))], nil } t.Cleanup(func() { trunkCommitsPage = pages }) (following{open: open}).catchUp(t.Context(), nil) @@ -258,3 +255,37 @@ func TestAForgeThatDoesNotAnswerLeavesTheCatchUpForTheNextStart(t *testing.T) { t.Fatalf("the next start did not catch up (left for later: %v)", later) } } + +// Finding 2 of the review: a gap that adds a module is a gap, judged as hearMerge judges it. With the operator's Approve on record it +// is heard (hearMerge asks the new module in its walk); without, it raises the condition and is not lost in silence. +func TestAGapThatAddsAModuleIsHeardOnItsWarrantAndRaisedWithout(t *testing.T) { + open := windowed(t) + everyHeadWaits(t) + asksRecorded(t) + keeper, _ := withConditionsInMemory(t) + tr := aTrunkOf(t, "mesh-catalog", "newmodule") + tr.approved["mesh-catalog-c01"] = false + catch := func() { (following{open: open}).catchUp(t.Context(), nil) } + catch() + if mergeKnown(t, open, "mesh-catalog-c01") { + t.Fatal("a merge adding a module was heard with no Approve on record") + } + all, err := keeper.Open(t.Context()) + if err != nil { + t.Fatal(err) + } + var raised int + for _, c := range all { + if c.Kind == kindMergeNotHeard { + raised++ + } + } + if raised != 1 { + t.Fatalf("a merge adding a module with no Approve raised %d condition(s), not one: it was lost in silence", raised) + } + tr.approved["mesh-catalog-c01"] = true + catch() + if !mergeKnown(t, open, "mesh-catalog-c01") { + t.Fatal("a merge adding a module was not heard once its Approve was on record") + } +} diff --git a/cmd/mesh-controller/catch_up_test.go b/cmd/mesh-controller/catch_up_test.go index 9b5b1c1b..317d2afd 100644 --- a/cmd/mesh-controller/catch_up_test.go +++ b/cmd/mesh-controller/catch_up_test.go @@ -6,6 +6,7 @@ import ( "fmt" "strings" "testing" + "time" "git.novox.be/novox/mesh-sdk/go/asks" sdkdelivery "git.novox.be/novox/mesh-sdk/go/delivery" @@ -91,8 +92,141 @@ func TestACaughtUpMergeIsAcceptedOnTheSameWarrantAsAnAnnouncedOne(t *testing.T) } } -// Keycloak's merge was asked as mesh-delivery.merge-378-b9104454f9a5-1: the warrant is found under mesh-delivery as under the -// teller, by the pull request's number and the first twelve digits of its head. +// aTellersAsk is the ask the delivery gate composes for a pull request, as the router holds it once the operator chose Approve +// (the gate's composeMergeAsk: its id from sdkdelivery.AskingID of the group and the heads, its Approve bound to mergeAct over the +// heads, the pull request and its head's first twelve digits in the details), with the warrant the router wrote. +func aTellersAsk(t *testing.T, group string, members ...sdkdelivery.Head) (asks.Ask, asks.Warrant) { + t.Helper() + key, err := sdkdelivery.MergeAskKey(group, members) + if err != nil { + t.Fatal(err) + } + id := sdkdelivery.AskingID(key, 1) + heads := map[string]string{} + var details []string + for _, m := range members { + owner, repo, _ := strings.Cut(m.Repository, "/") + heads[deliveryIDOf(owner, repo, m.Head)] = m.Head + details = append(details, fmt.Sprintf("%s#%d %q at %s: issues: none named; review: approved; reaches: no delivery plan yet", repo, m.Number, "title", m.Head[:12])) + } + bind := func(option string) string { + d, err := asks.ActDigest(mergeAct(id, option, heads)) + if err != nil { + t.Fatal(err) + } + return d + } + at := time.Date(2026, 10, 20, 12, 2, 1, 0, time.UTC) + ask := asks.Ask{ID: id, Headline: "Merge into main", Explanation: "passed its checks.", Who: asks.Operator, Expires: at.Add(24 * time.Hour), + OnExpiry: "nothing is merged; the ask is made again", Details: strings.Join(details, "\n"), + Options: []asks.Option{ + {ID: optionApprove, Label: "Approve", Does: "merges", Level: asks.Approve, Binds: bind(optionApprove)}, + {ID: "decline", Label: "Decline", Does: "nothing is merged", Level: asks.Approve, Binds: bind("decline")}}} + w := asks.Warrant{Ask: id, Asker: sdkdelivery.TellerModule, Outcome: asks.OutcomeChosen, Option: optionApprove, Label: "Approve", + Level: asks.Approve, By: &asks.Person{Who: asks.Operator, Kind: "telegram", Identity: "1", Verified: "user id"}, + At: at.Add(time.Minute), AskDigest: ask.Digest()} + return ask, w +} + +// withRouter hands findMergeWarrant these asks as the router's records, by asker. +func withRouter(t *testing.T, records map[string]map[string]struct { + ask asks.Ask + w asks.Warrant +}) { + t.Helper() + wasIDs, wasAsk := routerAskIDs, routerAskOf + closedRecords.Range(func(k, _ any) bool { closedRecords.Delete(k); return true }) + routerAskIDs = func(_ context.Context, _ catalogueReader, asker string) ([]string, error) { + var ids []string + for id := range records[asker] { + ids = append(ids, id) + } + return ids, nil + } + routerAskOf = func(_ context.Context, _ catalogueReader, asker, id string) (*asks.Ask, *asks.Warrant, error) { + r, ok := records[asker][id] + if !ok { + return nil, nil, nil + } + return &r.ask, &r.w, nil + } + t.Cleanup(func() { + routerAskIDs, routerAskOf = wasIDs, wasAsk + closedRecords.Range(func(k, _ any) bool { closedRecords.Delete(k); return true }) + }) +} + +type aRouterRecord = struct { + ask asks.Ask + w asks.Warrant +} + +// The teller's asks, as the router held them on 2026-10-20: the id is the router's own key, rebuilt here from the group and the +// head the gate composed it for (mesh-controller#405 at 8a330afd, mesh-catalog#381 at 1c702422, mesh-host#124 at 52600ec7). It names +// neither the pull request nor the head, so the warrant is found by the heads the Approve is bound to. +func TestATellersWarrantIsFoundByTheHeadItsApproveIsBoundTo(t *testing.T) { + real := []struct { + group, key, owner, repo string + number int + head string + }{ + {"feat/r4b-sdk-users", "merge-feat_r4b_sdk_users-ba59b03220f8-989afe0a3bcf", "novox", "mesh-controller", 405, "8a330afd44ba852873339cf1210e90989dca46e1"}, + {"fix/1351-remove-todays-delivery-module", "merge-fix_1351_remove_todays_delivery_module-64a353-d93b97b9697b", "novox", "mesh-catalog", 381, "1c7024224dd77c5b603299378883e0993b2e2fdd"}, + {"feat/r4b-sdk-users", "merge-feat_r4b_sdk_users-6a59ba48da14-424e053ebc9c", "novox", "mesh-host", 124, "52600ec7bd61ee271a5cc7f4136bcf0a2a8f00fc"}, + } + teller := map[string]aRouterRecord{} + for _, r := range real { + ask, w := aTellersAsk(t, r.group, sdkdelivery.Head{Repository: r.owner + "/" + r.repo, Number: r.number, Head: r.head}) + if ask.ID != r.key { + t.Fatalf("the id of %s#%d is %q, and the router held it as %q: the fixture is not the router's", r.repo, r.number, ask.ID, r.key) + } + teller[ask.ID] = aRouterRecord{ask, w} + } + withRouter(t, map[string]map[string]aRouterRecord{sdkdelivery.TellerModule: teller}) + for _, r := range real { + why, found, reason, err := findMergeWarrant(t.Context(), nil, r.owner, r.repo, r.number, r.head) + if err != nil || !found || !strings.Contains(why, r.key) || !strings.Contains(why, "bound to the head of this pull request") { + t.Errorf("%s#%d: the warrant was not found: %q %v %q %v", r.repo, r.number, why, found, reason, err) + } + // Another head of the same pull request is another merge: the Approve was not given for it. + if _, found, _, _ := findMergeWarrant(t.Context(), nil, r.owner, r.repo, r.number, "ffffffffffffffffffffffffffffffffffffffff"); found { + t.Errorf("%s#%d: another head found the warrant of this one", r.repo, r.number) + } + } + // One pull request's Approve is none for another repository's merge at the same head digits. + if _, found, _, _ := findMergeWarrant(t.Context(), nil, "novox", "mesh-tools", 405, real[0].head); found { + t.Error("a merge of another repository found mesh-controller#405's warrant") + } +} + +// A group's ask binds several heads, and the router's record carries only twelve digits of the other members': the pull request is +// found in the ask as it was published, which the warrant's digest covers. +func TestAGroupsWarrantIsFoundByTheAsksWordsForThePullRequestAndItsHead(t *testing.T) { + a := sdkdelivery.Head{Repository: "novox/mesh-host", Number: 124, Head: "52600ec7bd61ee271a5cc7f4136bcf0a2a8f00fc"} + b := sdkdelivery.Head{Repository: "novox/mesh-controller", Number: 405, Head: "8a330afd44ba852873339cf1210e90989dca46e1"} + ask, w := aTellersAsk(t, "feat/r4b-sdk-users", a, b) + withRouter(t, map[string]map[string]aRouterRecord{sdkdelivery.TellerModule: {ask.ID: {ask, w}}}) + for _, h := range []sdkdelivery.Head{a, b} { + owner, repo, _ := strings.Cut(h.Repository, "/") + why, found, reason, err := findMergeWarrant(t.Context(), nil, owner, repo, h.Number, h.Head) + if err != nil || !found || !strings.Contains(why, "a group's ask naming this pull request at this head") { + t.Errorf("%s#%d: not found in the group's ask: %q %v %q %v", repo, h.Number, why, found, reason, err) + } + } + if _, found, _, _ := findMergeWarrant(t.Context(), nil, "novox", "mesh-catalog", 381, a.Head); found { + t.Error("a pull request the group's ask does not name found its warrant") + } + // A Decline is no warrant. + declined := w + declined.Option, declined.Label = "decline", "Decline" + withRouter(t, map[string]map[string]aRouterRecord{sdkdelivery.TellerModule: {ask.ID: {ask, declined}}}) + if _, found, _, _ := findMergeWarrant(t.Context(), nil, "novox", "mesh-host", 124, a.Head); found { + t.Error("a Decline found a warrant") + } +} + +// Keycloak's merge was asked as mesh-delivery.merge-378-b9104454f9a5-1, before release 3e: the id names the pull request and +// the first twelve digits of its head, and the warrant is found by them under mesh-delivery. func TestAWarrantAskedUnderMeshDeliveryIsFoundByThePullRequestAndItsHead(t *testing.T) { ok := approvedMerge(t) head := "b9104454f9a5e0c4d3b2a1f0e9d8c7b6a5948372" @@ -100,34 +234,41 @@ func TestAWarrantAskedUnderMeshDeliveryIsFoundByThePullRequestAndItsHead(t *test ask, w := ok.ask, ok.warrant ask.ID, w.Ask, w.Asker = askID, askID, theDelivery w.AskDigest = ask.Digest() - wasIDs, wasAsk := routerAskIDs, routerAskOf - routerAskIDs = func(_ context.Context, _ catalogueReader, asker string) ([]string, error) { - if asker == theDelivery { - return []string{"merge-377-aaaaaaaaaaaa-1", askID, "merge-378-b9104454f9a5-0"}, nil - } - return nil, nil - } - routerAskOf = func(_ context.Context, _ catalogueReader, asker, id string) (*asks.Ask, *asks.Warrant, error) { - if asker == theDelivery && id == askID { - return &ask, &w, nil - } - return nil, nil, nil // the router holds no closed record of the other tries - } - t.Cleanup(func() { routerAskIDs, routerAskOf = wasIDs, wasAsk }) - why, found, reason, err := findMergeWarrant(t.Context(), nil, 378, head) + withRouter(t, map[string]map[string]aRouterRecord{theDelivery: { + "merge-377-aaaaaaaaaaaa-1": {}, + askID: {ask, w}, + }}) + why, found, reason, err := findMergeWarrant(t.Context(), nil, "novox", "mesh-catalog", 378, head) if err != nil || !found || !strings.Contains(why, askID) { t.Fatalf("the warrant was not found: %q %v %q %v", why, found, reason, err) } - // Another pull request's head, or none, finds none and says why. - if _, found, reason, _ := findMergeWarrant(t.Context(), nil, 378, "ffffffffffff0000"); found || !strings.Contains(reason, "merge-378-ffffffffffff-") { - t.Fatalf("another head found a warrant, or said %q", reason) - } - if _, found, reason, _ := findMergeWarrant(t.Context(), nil, 0, head); found || reason == "" { - t.Fatalf("a merge that names no pull request found a warrant: %v %q", found, reason) + // Another head names another ask, and none is found. + if _, found, _, _ := findMergeWarrant(t.Context(), nil, "novox", "mesh-catalog", 378, "ffffffffffff0000"); found { + t.Fatal("another head found a warrant") } // A router that cannot be read is an error, never "no warrant". routerAskIDs = func(context.Context, catalogueReader, string) ([]string, error) { return nil, errors.New("no answer") } - if _, found, _, err := findMergeWarrant(t.Context(), nil, 378, head); found || err == nil { + if _, found, _, err := findMergeWarrant(t.Context(), nil, "novox", "mesh-catalog", 378, head); found || err == nil { t.Fatalf("an unreadable router read as no warrant: %v %v", found, err) } } + +// A trunk shorter than a page, or with none, is read without a fault: the pages end when the forge's do. +func TestAShortTrunkIsReadWholeAndAnEmptyOneIsNoMerges(t *testing.T) { + was := trunkCommitsPage + t.Cleanup(func() { trunkCommitsPage = was }) + trunkCommitsPage = func(_ context.Context, _, _, _ string, page int) ([]trunkCommit, error) { + if page > 1 { + return nil, nil + } + return []trunkCommit{{Commit: "m2", SecondParent: "h2"}, {Commit: "p"}, {Commit: "m1", SecondParent: "h1"}}, nil + } + got, err := readTrunkMerges(t.Context(), "novox", "x", "main") + if err != nil || len(got) != 2 || got[0].Commit != "m1" || got[1].Commit != "m2" { + t.Fatalf("a short trunk read as %+v %v", got, err) + } + trunkCommitsPage = func(context.Context, string, string, string, int) ([]trunkCommit, error) { return nil, nil } + if got, err := readTrunkMerges(t.Context(), "novox", "x", "main"); err != nil || len(got) != 0 { + t.Fatalf("an empty trunk read as %+v %v", got, err) + } +}