Ask a refused merge again when its checks turn green on the forge (issue 1144) #368
@@ -2,6 +2,7 @@ package main
|
||||
|
||||
import (
|
||||
"slices"
|
||||
"strings"
|
||||
|
||||
"git.novox.be/novox/mesh-sdk/go/asks"
|
||||
)
|
||||
@@ -51,8 +52,34 @@ func (h *Holder) AskDecided(id, state string, w *asks.Warrant) {
|
||||
// CommitStatus takes a commit status set on the forge: one of the merge checks (mesh/merge-gate, mesh/repo-check) says a
|
||||
// check started, answered, or was put back, so the forge's open pull requests are read to see which heads still wait on
|
||||
// one (SeeOpenPulls). A status of any other context shows nothing the merge needs.
|
||||
//
|
||||
// A merge check turning success for the head an approved, ready, not-yet-merged delivery's merge was refused at is
|
||||
// itself an event that decides whether it may merge (issue 1144): the merge tool may have refused it on a check still
|
||||
// pending, which this status answers. The merge is asked again at once (mergeAgainNow), under the lock, so the next
|
||||
// look asks it; no timer does this (issue 1115). A status of another context, another commit or a state other than
|
||||
// success changes nothing.
|
||||
func (h *Holder) CommitStatus(e ForgeStatusEvent) {
|
||||
if slices.Contains(mergeChecks, e.Context) {
|
||||
h.wantForge()
|
||||
if !slices.Contains(mergeChecks, e.Context) {
|
||||
return
|
||||
}
|
||||
h.wantForge()
|
||||
if e.State != "success" {
|
||||
return
|
||||
}
|
||||
h.mu.Lock()
|
||||
defer h.mu.Unlock()
|
||||
repository := strings.ToLower(e.Owner + "/" + e.Repo)
|
||||
for _, d := range h.deliveries {
|
||||
if !strings.EqualFold(d.Repository, repository) || d.Ready == nil || d.Ready.Merged != nil || d.MergedAs != "" ||
|
||||
d.State.Final() {
|
||||
continue
|
||||
}
|
||||
head := d.Commit
|
||||
if sha := d.Ready.Heads[d.ID]; sha != "" {
|
||||
head = sha
|
||||
}
|
||||
if sameCommit(head, e.SHA) {
|
||||
h.mergeAgainNow(d) // its approved head's check turned success on the forge: the merge is asked again at once
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -427,7 +427,8 @@ 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 composed verdict, and each minute — the operator's",
|
||||
H2: "none: the merge is asked again on a verdict, an Approve, a group's composed check or a merge check's " +
|
||||
"status turning success — 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)})
|
||||
}
|
||||
|
||||
@@ -67,6 +67,8 @@ func TestAnApprovedPullRequestIsMergedThroughTheMergeTool(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// 1144/AC3: a refused merge is not asked again by any timer; only an event (here, mergeAgainNow called directly, as a
|
||||
// verdict, an Approve or a composed check does) asks it again.
|
||||
func TestAMergeRefusedIsSaidAtOnceAndAskedAgainOnlyOnAnEvent(t *testing.T) {
|
||||
w := newWorld(t)
|
||||
w.h.PullUpdated(pr("novox/app", 7, readyHead, "feat/x", "src/a.go"))
|
||||
@@ -93,6 +95,88 @@ func TestAMergeRefusedIsSaidAtOnceAndAskedAgainOnlyOnAnEvent(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// 1144/AC1: a refused merge, approved and ready, is asked again at once when the forge's forge.status says one of
|
||||
// the merge checks (mesh/merge-gate, mesh/repo-check) is success for the commit the merge was refused at — the
|
||||
// delivery's approved head — and merges once the tool accepts (novox/hq mesh-catalog#366: the verdict reached this
|
||||
// owner before the forge wrote the statuses, the merge asked then was refused on a pending check, and nothing asked
|
||||
// it again when the statuses turned success).
|
||||
func TestAMergeRefusedIsAskedAgainWhenItsCheckTurnsSuccessOnTheForge(t *testing.T) {
|
||||
w := newWorld(t)
|
||||
w.h.PullUpdated(pr("novox/app", 7, readyHead, "feat/x", "src/a.go"))
|
||||
w.h.Checked(judgedAt("novox/app", 7, readyHead, "pass", "t1t1t1t1t1t1"))
|
||||
w.forge.refuse = errors.New("the forge refused gitea_merge_pull_request: mesh/repo-check (pending)")
|
||||
w.forge.open = []OpenPull{openPull("novox/app", 7, readyHead)}
|
||||
w.h.SeeOpenPulls()
|
||||
w.settleAll()
|
||||
if len(w.forge.merges) != 1 {
|
||||
t.Fatalf("not asked once, refused: %v", w.forge.merges)
|
||||
}
|
||||
w.forge.refuse = nil
|
||||
w.h.CommitStatus(ForgeStatusEvent{Owner: "novox", Repo: "app", SHA: readyHead, Context: "mesh/repo-check", State: "success"})
|
||||
w.settleAll()
|
||||
if len(w.forge.merges) != 2 {
|
||||
t.Fatalf("not asked again when its check turned success: %v", w.forge.merges)
|
||||
}
|
||||
if w.delivery(IDOf("novox/app", readyHead)).Ready.Merged == nil {
|
||||
t.Fatalf("it did not merge once the tool accepted")
|
||||
}
|
||||
}
|
||||
|
||||
// 1144/AC2: a status of another context, of another commit, or a state other than success changes nothing for a
|
||||
// refused merge: it is not asked again.
|
||||
func TestAMergeRefusedIsNotAskedAgainOnAStatusThatDoesNotMatch(t *testing.T) {
|
||||
w := newWorld(t)
|
||||
w.h.PullUpdated(pr("novox/app", 7, readyHead, "feat/x", "src/a.go"))
|
||||
w.h.Checked(judgedAt("novox/app", 7, readyHead, "pass", "t1t1t1t1t1t1"))
|
||||
w.forge.refuse = errors.New("the forge refused gitea_merge_pull_request: mesh/repo-check (pending)")
|
||||
w.forge.open = []OpenPull{openPull("novox/app", 7, readyHead)}
|
||||
w.h.SeeOpenPulls()
|
||||
w.settleAll()
|
||||
if len(w.forge.merges) != 1 {
|
||||
t.Fatalf("not asked once, refused: %v", w.forge.merges)
|
||||
}
|
||||
w.forge.refuse = nil
|
||||
for _, e := range []ForgeStatusEvent{
|
||||
{Owner: "novox", Repo: "app", SHA: readyHead, Context: "some/other-check", State: "success"}, // another context
|
||||
{Owner: "novox", Repo: "app", SHA: pushed, Context: "mesh/repo-check", State: "success"}, // another commit
|
||||
{Owner: "novox", Repo: "app", SHA: readyHead, Context: "mesh/repo-check", State: "pending"}, // another state
|
||||
} {
|
||||
w.h.CommitStatus(e)
|
||||
w.settleAll()
|
||||
}
|
||||
if len(w.forge.merges) != 1 {
|
||||
t.Fatalf("asked again on a status that should change nothing: %v", w.forge.merges)
|
||||
}
|
||||
if w.delivery(IDOf("novox/app", readyHead)).Ready.Merged != nil {
|
||||
t.Fatalf("merged on a status that should change nothing")
|
||||
}
|
||||
}
|
||||
|
||||
// 1144/AC4: the stalled line for a refused approved head names the events that ask the merge again, and no longer
|
||||
// says "each minute" (removed by issue 1115; false since then — there is no timer).
|
||||
func TestTheStalledLineForARefusedApprovedHeadNamesTheEventsThatAskTheMergeAgain(t *testing.T) {
|
||||
w := newWorld(t)
|
||||
w.h.PullUpdated(pr("novox/app", 7, readyHead, "feat/x", "src/a.go"))
|
||||
w.h.Checked(judgedAt("novox/app", 7, readyHead, "pass", "t1t1t1t1t1t1"))
|
||||
w.forge.refuse = errors.New("the forge refused gitea_merge_pull_request: mesh/repo-check (pending)")
|
||||
w.forge.open = []OpenPull{openPull("novox/app", 7, readyHead)}
|
||||
w.h.SeeOpenPulls()
|
||||
w.settleAll()
|
||||
lines := w.h.Stalled()
|
||||
if len(lines) != 1 {
|
||||
t.Fatalf("wanted one stalled line, got %+v", lines)
|
||||
}
|
||||
h2 := lines[0].H2
|
||||
if strings.Contains(h2, "each minute") {
|
||||
t.Fatalf("the stalled line still says 'each minute', false since issue 1115: %q", h2)
|
||||
}
|
||||
for _, want := range []string{"verdict", "Approve", "composed check", "status"} {
|
||||
if !strings.Contains(h2, want) {
|
||||
t.Fatalf("the stalled line does not name %q as asking the merge again: %q", want, h2)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestAGroupMergesEveryMemberInOnePassOnlyOnceComposedAndApproved(t *testing.T) {
|
||||
w := newWorld(t)
|
||||
w.op.auto = false
|
||||
|
||||
@@ -55,6 +55,7 @@ func TestAProposedDeliveryWhoseHeadHoldsAPassingVerdictAtTheForgeAdoptsItAndAsks
|
||||
}
|
||||
}
|
||||
|
||||
// 1144/AC3: no timer asks a refused merge again; only an event does (here, mergeAgainNow called directly).
|
||||
func TestAMergeTheForgeRefusedIsAskedAgainOnlyOnAnEventWithNoTimer(t *testing.T) {
|
||||
w := newWorld(t)
|
||||
w.h.PullUpdated(pr("novox/app", 7, readyHead, "feat/x", "src/a.go"))
|
||||
|
||||
Reference in New Issue
Block a user