diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index d1dd7461..8cc6e8f7 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -524,15 +524,23 @@ func (h *Holder) PullMerged(p PullEvent) { if over := startedOverFor(d, p.Number); over != "" { // A delivery stopped only by an earlier, closed duplicate pull request with the same head is not lost when // its merge is heard without the open announcement either: it is started over for the merging pull request - // the same way an announcement would, before the merge is taken as usual (novox/hq issue 783). + // the same way an announcement would, before the merge is taken as usual (novox/hq issue 783). Heard this + // way — its own merge, with no open announcement of this pull request — it is made exactly as a first-heard + // merge in every field but its id: FromForge, and its check the forge's word on the head, so a long-past + // merge replayed from the bus's history still falls under the history rule (issue 313) and is not held for + // a person (review round 1). fresh, t, err := h.freshAnnouncement(p, now, over) if err != nil { h.refuse(err) return } + fresh.FromForge = true + if s := p.HeadChecks["mesh/merge-gate"]; s != "" { + fresh.Check = &Verdict{At: now, Gate: verdictOfState(s), Summary: "the head's mesh/merge-gate, as the forge holds it", + Repo: verdictOfState(p.HeadChecks["mesh/repo-check"])} + } d, ts = fresh, []Transition{t} - if v := decidedOnHead(p.HeadChecks, now); v != nil { - d.Check = v + if d.Check != nil { ts = append(ts, settle(d, Facts{Now: now})...) } } else if d == nil { diff --git a/modules/mesh-delivery/cmd/mesh-delivery/issue783_test.go b/modules/mesh-delivery/cmd/mesh-delivery/issue783_test.go index c8b11518..c0dd6062 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/issue783_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/issue783_test.go @@ -3,6 +3,7 @@ package main import ( "strings" "testing" + "time" ) // novox/hq issue 783: mesh-catalog commit b5417c28 was pushed while #305 (from branch adr-359-mesh-mail-733) was @@ -20,6 +21,7 @@ const ( head783, merge783 = "d305d305d305d305", "d315d315d315d315" head783b, merge783b = "d306d306d306d306", "d316d316d316d316" head783c = "d307d307d307d307" + head783d, merge783d = "d308d308d308d308", "d318d318d318d318" catalog783 = "novox/mesh-catalog" branch783 = "adr-359-mesh-mail-733" ) @@ -113,3 +115,37 @@ func TestAStoppedDeliveryAnnouncedAgainByItsOwnPullRequestStaysStopped(t *testin t.Fatalf("#307 announced again by itself left it %s, Number %d; want it to stay stopped", d.State, d.Number) } } + +// TestAMergeHeardLongAfterStartingOverAStoppedDuplicateIsHistoryLikeAnyOtherReplayedMerge is review round 1's +// decision: a delivery started over from a merge heard without its open announcement (as in decision 4, above) +// takes FromForge exactly as a first-heard merge does, so one replayed from the bus's history long after it +// happened still falls under the history rule (novox/hq issue 313) and is never held for a person. +func TestAMergeHeardLongAfterStartingOverAStoppedDuplicateIsHistoryLikeAnyOtherReplayedMerge(t *testing.T) { + w := newWorld(t) + id := IDOf(catalog783, head783d) + + w.h.PullUpdated(pr(catalog783, 308, head783d, branch783+"-d", "src/d.go")) + w.h.PullClosed(pr(catalog783, 308, head783d, branch783+"-d")) + if w.state(id) != Stopped { + t.Fatalf("#308 closed unmerged left it %s; want stopped", w.state(id)) + } + + m := merged(catalog783, 318, head783d, merge783d) + m.MergedAt = w.now.Add(-historyLate - time.Minute).UTC().Format(time.RFC3339) + w.h.PullMerged(m) + + d := w.delivery(id) + if d.State == Stopped || d.Number != 318 { + t.Fatalf("#318's merge, heard long after it happened with no announcement of its own, left it %s, Number %d; "+ + "want started over for #318", d.State, d.Number) + } + if !d.FromForge { + t.Fatal("a merge heard without its own announcement is made from the forge's word: FromForge is false") + } + if d.History == "" { + t.Fatalf("a merge heard %s after it happened is history, never held for a person; History is empty", historyLate+time.Minute) + } + if d.State == Held { + t.Fatalf("history is never held for a person; state is %s, HeldWhy %q", d.State, d.HeldWhy) + } +}