Merge pull request 'Put a group's composed check out of date when a member is rechecked (issue 867, ADR 0323 §6)' (#327) from fix/867-recheck-asks-the-group-again into main

modules/mesh-delivery/cmd/mesh-delivery/recheck_test.go

@@ -3,6 +3,7 @@ package main
 import (
 	"strings"
 	"testing"
+	"time"
 )
 
 // A recheck puts the merge check's statuses back to pending before it asks the check: the forge keeps the
@@ -95,3 +96,80 @@ func TestARecheckTheForgeCannotTakeIsRefused(t *testing.T) {
 		t.Fatalf("refused, it moved to %s", s)
 	}
 }
+
+// novox/hq issue 867, ADR 0323 §6: "the composed check runs again when a member is rechecked". A group whose composed
+// check failed stayed rejected after a member was rechecked and passed, because the kept verdict was for the same heads
+// and nothing marked it out of date.
+func TestAMembersRecheckAsksTheGroupsComposedCheckAgain(t *testing.T) {
+	w := newWorld(t)
+	w.h.PullUpdated(pr("novox/a", 1, "a1a1a1a1a1a1a1", "feat/z"))
+	w.h.PullUpdated(pr("novox/b", 2, "b2b2b2b2b2b2b2", "feat/z"))
+	w.h.Checked(verdict("novox/a", 1, "a1a1a1a1a1a1a1", "pass"))
+	w.h.Checked(verdict("novox/b", 2, "b2b2b2b2b2b2b2", "pass"))
+	w.settleAll()
+	groupAsks := func() (n int) {
+		for _, c := range w.ctl.checks {
+			if strings.HasPrefix(c, "feat/z:") {
+				n++
+			}
+		}
+		return
+	}
+	if groupAsks() != 1 {
+		t.Fatalf("the group was asked %v", w.ctl.checks)
+	}
+	w.h.Checked(CheckedEvent{Group: "feat/z", ID: "check-1", Verdict: "fail", Summary: "they do not compose"})
+	w.settleAll()
+	if state, _ := GroupState(w.h.groups["feat/z"], w.h.membersOf(w.h.groups["feat/z"])); state != "rejected" {
+		t.Fatalf("the group is %s", state)
+	}
+	if _, err := w.h.Recheck(IDOf("novox/b", "b2b2b2b2b2b2b2"), "the judge was fixed", "a person"); err != nil {
+		t.Fatal(err)
+	}
+	// The member's own check passes again, at the same head, a while later.
+	w.later(5 * time.Minute)
+	w.h.Checked(verdict("novox/b", 2, "b2b2b2b2b2b2b2", "pass"))
+	w.settleAll()
+	if groupAsks() != 2 {
+		t.Fatalf("a member's recheck did not ask the group again: %v", w.ctl.checks)
+	}
+	if g := w.h.groups["feat/z"]; g.Check == nil || g.Check.Verdict != "" || g.Check.Asked == "check-1" {
+		t.Fatalf("the old verdict still stands: %+v", g.Check)
+	}
+}
+
+// A recheck made while the group's composed check is still running is not lost: the old run's answer lands, and the
+// group is still asked again once the member is ready.
+func TestARecheckWhileTheComposedCheckRunsAsksTheGroupAgain(t *testing.T) {
+	w := newWorld(t)
+	w.h.PullUpdated(pr("novox/a", 1, "a1a1a1a1a1a1a1", "feat/z"))
+	w.h.PullUpdated(pr("novox/b", 2, "b2b2b2b2b2b2b2", "feat/z"))
+	w.h.Checked(verdict("novox/a", 1, "a1a1a1a1a1a1a1", "pass"))
+	w.h.Checked(verdict("novox/b", 2, "b2b2b2b2b2b2b2", "pass"))
+	w.settleAll() // the composed check check-1 is asked and running: no verdict yet
+	if _, err := w.h.Recheck(IDOf("novox/b", "b2b2b2b2b2b2b2"), "the judge was fixed", "a person"); err != nil {
+		t.Fatal(err)
+	}
+	// The old run answers.
+	w.h.Checked(CheckedEvent{Group: "feat/z", ID: "check-1", Verdict: "fail", Summary: "they do not compose"})
+	w.later(5 * time.Minute)
+	w.h.Checked(verdict("novox/b", 2, "b2b2b2b2b2b2b2", "pass"))
+	w.settleAll()
+	n := 0
+	for _, c := range w.ctl.checks {
+		if strings.HasPrefix(c, "feat/z:") {
+			n++
+		}
+	}
+	if n != 2 {
+		t.Fatalf("the group was not asked again after a recheck made while its check ran: %v", w.ctl.checks)
+	}
+	if g := w.h.groups["feat/z"]; g.Check == nil || g.Check.Verdict != "" || g.Check.OutOfDate != "" {
+		t.Fatalf("the old run's answer stands: %+v", g.Check)
+	}
+	// The old run answering again is history.
+	w.h.Checked(CheckedEvent{Group: "feat/z", ID: "check-1", Verdict: "fail", Summary: "late"})
+	if g := w.h.groups["feat/z"]; g.Check.Verdict != "" {
+		t.Fatalf("a late answer of the old run was taken: %+v", g.Check)
+	}
+}

modules/mesh-delivery/cmd/mesh-delivery/verbs.go

@@ -310,6 +310,16 @@ func (h *Holder) Recheck(id, why, by string) (string, error) {
 		return "", fmt.Errorf("%v (its merge check says pending; the forge's next announcement asks it)", err)
 	}
 	d.Check = nil
+	// The composed check of its group goes out of date (novox/hq ADR 0323 §6, issue 867): it was judged for these very
+	// heads (running or answered: a run's answer is taken by its id and must not stand for the recheck), so without this the kept verdict (a failure above all) stands and the group is never asked again. It is
+	// asked again once the member is judged again, when every member is ready.
+	if g := h.groups[d.Group]; g != nil && g.Check != nil {
+		now := h.Now()
+		g.Check.OutOfDate = fmt.Sprintf("composed check out of date: %s was rechecked (%s)", d.ID, why)
+		g.Check.OutOfDateAt = now
+		g.Updated = now
+		h.keepGroup(g)
+	}
 	h.moved(d, []Transition{t})
 	m := MemberOf(d)
 	h.mu.Unlock()