diff --git a/cmd/mesh-controller/check_siblings.go b/cmd/mesh-controller/check_siblings.go index 45f1d63b..b2e5626e 100644 --- a/cmd/mesh-controller/check_siblings.go +++ b/cmd/mesh-controller/check_siblings.go @@ -57,19 +57,39 @@ func nameOf(repository string) string { // learned, empty when they could: the group's members carry no head or base — a mesh-delivery from before the rule. func siblingsFor(p link.PullUpdated, entries []inventory.Entry, beside map[string]link.CheckedOut, lines []groupsLine) ([]link.CheckSibling, string) { - if p.Head == "" { - return nil, "" - } + own := strings.ToLower(p.Owner + "/" + p.Repo) var group *groupsLine - for i := range lines { - if lines[i].ID == p.Head { - group = &lines[i] + if p.Head == "" { + // A recheck (mesh-delivery's `recheck` through `delivery check`) carries no head branch name: the group is the one + // whose members hold this very pull request, by repository and number. + learnable := false + for i := range lines { + for _, m := range lines[i].Details { + learnable = true + if p.Number != 0 && m.Number == p.Number && strings.ToLower(m.Repository) == own { + group = &lines[i] + } + } + } + if group == nil { + for _, l := range lines { + if len(l.Details) == 0 && len(l.Members) > 0 && !learnable { + return nil, "mesh-delivery named the groups' members without their pull requests (a mesh-delivery from before novox/hq ADR 0382): " + + "the group of a pull request announced without its head branch cannot be found" + } + } + return nil, "" // a lone pull request + } + } else { + for i := range lines { + if lines[i].ID == p.Head { + group = &lines[i] + } + } + if group == nil { + return nil, "" } } - if group == nil { - return nil, "" - } - own := strings.ToLower(p.Owner + "/" + p.Repo) if len(group.Details) == 0 && len(group.Members) > 0 { return nil, "mesh-delivery named the group's members without their heads and bases (a mesh-delivery from before novox/hq ADR 0382)" } diff --git a/cmd/mesh-controller/check_siblings_test.go b/cmd/mesh-controller/check_siblings_test.go index 1322da7e..b35e8e60 100644 --- a/cmd/mesh-controller/check_siblings_test.go +++ b/cmd/mesh-controller/check_siblings_test.go @@ -117,10 +117,25 @@ func TestASiblingIsTheGroupsOtherOpenPullRequestIntoABaseOfTheSameName(t *testin if got, _ := siblingsFor(release, entries, besideTheCore(), lines); len(got) != 1 || got[0].Dir != "mesh-controller" { t.Errorf("into a release branch: %+v", got) } - // A pull request on no head branch is in no group. + // A recheck announces the pull request without its head branch: its group is the one holding it (repository, number). pull.Head = "" + got, unknown := siblingsFor(pull, entries, besideTheCore(), lines) + if len(got) != 1 || got[0].Dir != "mesh-tools" || unknown != "" { + t.Errorf("a recheck of a group member: %+v %q", got, unknown) + } + // A pull request in no group, rechecked, is alone: no sibling, and nothing said. + pull.Number = 999 if got, unknown := siblingsFor(pull, entries, besideTheCore(), lines); got != nil || unknown != "" { - t.Errorf("no head branch: %+v %q", got, unknown) + t.Errorf("a lone pull request: %+v %q", got, unknown) + } + if got, unknown := siblingsFor(pull, entries, besideTheCore(), nil); got != nil || unknown != "" { + t.Errorf("no groups at all: %+v %q", got, unknown) + } + // A mesh-delivery that names members without their pull requests cannot find the group of a recheck: it says so. + older := []groupsLine{{ID: "feat/x", Members: []string{"novox/mesh-host@111111111111 (checked)"}}} + pull.Number = 114 + if got, unknown := siblingsFor(pull, entries, besideTheCore(), older); got != nil || unknown == "" { + t.Errorf("an older mesh-delivery: %+v %q", got, unknown) } } diff --git a/cmd/mesh-controller/replay867_test.go b/cmd/mesh-controller/replay867_test.go index 4194cde6..9db90bd9 100644 --- a/cmd/mesh-controller/replay867_test.go +++ b/cmd/mesh-controller/replay867_test.go @@ -34,7 +34,13 @@ import ( // It runs the whole path: the forge announces each pull request (following.PullUpdated), the controller asks // mesh-delivery which pull requests the group holds, the ask is queued for the build seat, and the build seat's check // (builder.Check, with a container runtime that runs each container's command on this machine) judges it. -func TestReplay867(t *testing.T) { +func TestReplay867(t *testing.T) { replay867(t, false) } + +// TestReplay867Recheck is the same pair re-checked through mesh-delivery (`delivery check` with one member): the forge's +// announcement is not repeated, so the pull request reaches the controller without its head branch name. +func TestReplay867Recheck(t *testing.T) { replay867(t, true) } + +func replay867(t *testing.T, recheck bool) { if _, err := exec.LookPath("go"); err != nil { t.Skip("no Go toolchain on this machine to build a judge with") } @@ -46,7 +52,7 @@ func TestReplay867(t *testing.T) { module string }{{"the node-engine change", "mesh-host", 114, r.hostHead, "mesh-host"}, {"the controller change", "mesh-controller", 346, r.controllerHead, "mesh-controller"}} { - v := r.checked(t, c.repo, c.number, c.head) + v := r.checked(t, c.repo, c.number, c.head, recheck) if v.Gate == nil || v.Gate.Verdict != "pass" || v.Repo == nil || v.Repo.Verdict != "pass" { t.Errorf("%s (%s#%d) did not pass its own merge check beside its group: gate %+v, repository %+v\n%s", c.what, c.repo, c.number, v.Gate, v.Repo, v.Report) @@ -179,7 +185,7 @@ func theGroupOfIssue867(t *testing.T) *theIssue867 { } // checked is a pull request announced by the forge, asked of the build seat by the controller, and checked there. -func (r *theIssue867) checked(t *testing.T, repo string, number int, head string) builderVerdict { +func (r *theIssue867) checked(t *testing.T, repo string, number int, head string, recheck bool) builderVerdict { t.Helper() js, ask := aBuildQueue(t) _ = ask @@ -188,6 +194,12 @@ func (r *theIssue867) checked(t *testing.T, repo string, number int, head string p := link.PullUpdated{Owner: "novox", Repo: repo, Number: number, Base: "main", Head: "feat/867", Commit: head, Paths: []string{"lock.txt", "validate/more.go", "cmd/mesh-controller/main.go"}, ModuleDirsSaid: true, MergeCheck: true, MergeCheckSaid: true} + if recheck { + // As `delivery check --members ` builds it from mesh-delivery's member: no head branch name. + p = orderMember{ID: "novox/" + repo + "@" + head[:12], Repository: "novox/" + repo, Base: "main", Head: head, + Number: number, Paths: p.Paths, ModuleDirsSaid: true}.pull() + p.MergeCheck, p.MergeCheckSaid = true, true + } if err := (following{open: r.open, oneShot: true}).PullUpdated(t.Context(), p); err != nil { t.Fatal(err) } diff --git a/testdata/store-tests b/testdata/store-tests index 83f5dc5f..055495ac 100644 --- a/testdata/store-tests +++ b/testdata/store-tests @@ -80,3 +80,4 @@ ./cmd/mesh-controller TestAPersonsRecordAndTheBusHoldEveryMove ./cmd/mesh-controller TestARecordedMoveThatCannotBeJudgedWaits ./cmd/mesh-controller TestReplay867 +./cmd/mesh-controller TestReplay867Recheck