diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/bus_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/bus_test.go index b34c470c..cdeebbcb 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/bus_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/bus_test.go @@ -514,6 +514,11 @@ func TestAPassingVerdictAsksTheApproveAndItsApproveMergesWithinSecondsOverARealB announced.Title = "Fix the apostrophe (issue 975)" announced.Body = "The description, with no review line yet." b.publish(t, "mesh.mod.gitea.event.pull.updated", announced) + rt.until("the pull request's key to hold its head", eventWithin, func() bool { + e, err := b.kvs["pulls"].Get(ctx, "novox.app.7") + var k PullKey + return err == nil && json.Unmarshal(e.Value(), &k) == nil && k.Head == head && k.Trunk == "a1a1a1a1a1a1" + }) b.checkPasses(t, rt, "fix/x", "a1a1a1a1a1a1", judgedPull{"novox/app", 7, head}) // No review names the head, so nothing is asked (ADR 0496, issue 1803): the delivery waits for a review. diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/pullkey.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/pullkey.go index 68b98350..4b2b7701 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/pullkey.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/pullkey.go @@ -51,7 +51,38 @@ func (h *Holder) KeepPull(p PullEvent) { if err != nil { h.Logf("[mesh-delivery] the key of %s/%s#%d could not be written: %v", p.Owner, p.Repo, p.Number, err) h.again("a pull request's key that could not be written") + return } + // The head's mesh/check may have been heard before this key held the head: the two events are taken on goroutines of + // their own, and KeepCheckVerdict then found no head to write a verdict to. The verdict the delivery holds for this head + // and trunk is written now; read after the write above landed, it cannot miss one taken before that write. + verdict := h.checkVerdictOf(p.repository(), p.HeadSHA, p.BaseSHA) + if verdict == "" { + return + } + err = h.Pulls.Change(pullKeyName(p.Owner, p.Repo, p.Number), func(k PullKey) (PullKey, bool) { + if !sameCommit(k.Head, p.HeadSHA) || !sameCommit(k.Trunk, p.BaseSHA) || k.Verdict != "" { + return k, false + } + k.Verdict = verdict + return k, true + }) + if err != nil { + h.Logf("[mesh-delivery] the verdict in the key of %s/%s#%d could not be written: %v", p.Owner, p.Repo, p.Number, err) + h.again("a pull request's key that could not be written") + } +} + +// checkVerdictOf is the verdict of the mesh/check the delivery of a head holds, when it judged this head on this trunk +// commit; empty otherwise. Outside the lock. +func (h *Holder) checkVerdictOf(repository, head, trunk string) string { + h.mu.Lock() + defer h.mu.Unlock() + d := h.deliveries[IDOf(strings.ToLower(repository), head)] + if d == nil || d.Check == nil || d.Check.Status != ctxCheck || !sameCommit(d.Check.Head, head) || !sameCommit(d.Check.Trunk, trunk) { + return "" + } + return d.Check.Gate } // KeepCheckVerdict writes the verdict of a head's mesh/check into the key of its pull request, when the key holds that head diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/pullkey_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/pullkey_test.go index 4d250f66..a24a2882 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/pullkey_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/pullkey_test.go @@ -145,3 +145,21 @@ func TestTheSameMergeHeardAgainAfterALaterMergeMovesNoTrunkBack(t *testing.T) { t.Fatalf("the trunk moved from %q to %q on a merge heard again; wanted it to stay m9m9m9m9m9m9", before.TrunkNow, after.TrunkNow) } } + +// The forge's mesh/check event handled wholly between the announcement's PullUpdated and its KeepPull (the two events are +// taken on goroutines of their own): the verdict must still reach the pull request's key. +func TestAVerdictHeardBeforeTheKeyIsWrittenReachesTheKey(t *testing.T) { + w := newWorld(t) + pulls := newFakePulls() + w.h.Pulls = pulls + p := announcedAt(readyHead, "a1a1a1a1a1a1") + w.h.PullUpdated(p) + s := ForgeStatusEvent{Owner: "novox", Repo: "app", SHA: readyHead, Context: ctxCheck, State: "success", + Description: "pass " + readyHead[:12] + " on a1a1a1a1a1a1 | unit 3 run"} + w.h.CommitStatus(s) + w.h.KeepCheckVerdict(s) + w.h.KeepPull(p) + if got := pulls.get(appKey); got.Head != readyHead || got.Verdict != "pass" { + t.Fatalf("the key holds %+v, want the head with the verdict pass", got) + } +}