mesh-issues: test that a delivered fix and a stale claim are said (hq issue 483)

Each test fails on main: a linked pull request is never measured, nothing warns of a delivered fix left in fixing, and list has no flag to find either.
2026-10-11 12:14:42 +02:00
parent 300406e0dd
commit c4ea58aeb0
1 changed files with 307 additions and 0 deletions
@@ -0,0 +1,307 @@
+package main
+
+// Tests of novox/hq issue 483: an issue whose fix is delivered stays in fixing, and a claim nobody works on is
+// never noticed. The issues the operator found (231, 402, 457, …) named their fix the way the live tracker keeps
+// it: a link of kind pull-request, by the worker, while the issue stayed in fixing, with fixed-by given only at
+// the resolve (issue-tracker.get 457, 2026-10-11).
+
+import (
+	"encoding/json"
+	"strings"
+	"testing"
+	"time"
+
+	stdio "git.novox.be/novox/mesh-sdk/go"
+)
+
+// asJSON is a verb's answer as the tool runner hands it on: what a caller reads, whatever the Go types are.
+func asJSON(t *testing.T, v any) map[string]any {
+	t.Helper()
+	raw, err := json.Marshal(v)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var m map[string]any
+	if err := json.Unmarshal(raw, &m); err != nil {
+		t.Fatal(err)
+	}
+	return m
+}
+
+// linkPR links a pull request the way the live issues do: kind pull-request, target <repository>#<number>.
+func linkPR(t *testing.T, h *harness, n uint64, target string) {
+	t.Helper()
+	if _, err := h.call(t, "link", map[string]any{"number": n, "kind": "pull-request", "target": target}); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func rowsOf(t *testing.T, h *harness, args map[string]any) []map[string]any {
+	t.Helper()
+	v, err := h.call(t, "list", args)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var out []map[string]any
+	for _, r := range asJSON(t, v)["rows"].([]any) {
+		out = append(out, r.(map[string]any))
+	}
+	return out
+}
+
+// Part 1: when every pull request linked to an issue in fixing is delivered, the issue gains one note naming
+// them, when each was delivered, and its worker, and the event ready-to-verify names the worker.
+func TestALinkedFixDeliveredIsNotedOnceNamingItsWorker(t *testing.T) {
+	h := newHarness(t)
+	now := clocked(h)
+	n := h.open(t, "a build fails during the registry's clean-up", "mesh-controller")
+	if _, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "fix-457@shanks"}); err != nil {
+		t.Fatal(err)
+	}
+	linkPR(t, h, n, "mesh-catalog#225")
+	linkPR(t, h, n, "mesh-catalog#226")
+	active := get(t, h, n).Worker.Active
+	merged := now.Add(-20 * time.Minute)
+	transition(t, h, 225, "published", "", "abc", merged)
+	if v, _ := h.View(n); v.Delivery != "fix merged, not delivered: mesh-catalog#225" {
+		t.Fatalf("a linked pull request's delivery is not measured: %q", v.Delivery)
+	}
+	delivered := now.Add(-10 * time.Minute)
+	transition(t, h, 225, "delivered", "", "abc", delivered)
+	if countEvents(h, EventReadyToVerify) != 0 {
+		t.Fatal("said delivered while mesh-catalog#226 is not")
+	}
+	transition(t, h, 226, "delivered", "", "def", delivered.Add(time.Minute))
+	transition(t, h, 226, "delivered", "", "def", delivered.Add(time.Minute))
+	is := get(t, h, n)
+	var said []string
+	for _, nt := range is.Notes {
+		if strings.HasPrefix(nt.Text, "Ready to verify:") {
+			said = append(said, nt.Text)
+		}
+	}
+	if len(said) != 1 {
+		t.Fatalf("want one note, got %q", said)
+	}
+	for _, want := range []string{"mesh-catalog#225", "mesh-catalog#226", delivered.Format(time.RFC3339),
+		delivered.Add(time.Minute).Format(time.RFC3339), "fix-457@shanks"} {
+		if !strings.Contains(said[0], want) {
+			t.Fatalf("the note does not say %q: %s", want, said[0])
+		}
+	}
+	if is.Status != StatusFixing || is.Worker == nil || is.Worker.By != "fix-457@shanks" {
+		t.Fatalf("the tracker's own note changed the issue's work: %+v", is)
+	}
+	var ev map[string]any
+	for _, e := range h.said() {
+		if e.Event == EventReadyToVerify {
+			ev = e.Body
+		}
+	}
+	if countEvents(h, EventReadyToVerify) != 1 || ev["worker"] != "fix-457@shanks" {
+		t.Fatalf("%d ready-to-verify, last %v", countEvents(h, EventReadyToVerify), ev)
+	}
+	// The tracker's own note keeps no worker current: the worker's activity is still its own last act.
+	if !is.Worker.Active.Equal(active) {
+		t.Fatalf("the worker was made active by the tracker's note: %v", is.Worker.Active)
+	}
+}
+
+// Part 2: past the bound, an issue in fixing whose every linked fix is delivered is warned on the operator's
+// channel, once, naming the issue and its worker; the warning clears when the issue leaves fixing.
+func TestDeliveredNotResolvedIsWarnedAfterTheBoundAndClearedWhenItLeavesFixing(t *testing.T) {
+	h := newHarness(t)
+	now := clocked(h)
+	note := &notified{}
+	h.Notify = note.notify
+	n := h.open(t, "x", "mesh-catalog modules/records")
+	if _, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "fix-x@g14"}); err != nil {
+		t.Fatal(err)
+	}
+	linkPR(t, h, n, "mesh-catalog#9")
+	transition(t, h, 9, "delivered", "", "abc", *now)
+	*now = now.Add(59 * time.Minute)
+	if _, err := h.Sweep(); err != nil || len(note.said) != 0 {
+		t.Fatalf("warned under the bound: %q %v", note.said, err)
+	}
+	if rows := rowsOf(t, h, map[string]any{"flag": "delivered-not-resolved"}); len(rows) != 1 ||
+		rows[0]["delivered-not-resolved"] != true || rows[0]["worker"] != "fix-x@g14" {
+		t.Fatalf("list flag delivered-not-resolved: %v", rows)
+	}
+	*now = now.Add(2 * time.Minute)
+	for i := 0; i < 2; i++ {
+		if _, err := h.Sweep(); err != nil {
+			t.Fatal(err)
+		}
+	}
+	key := "issue-tracker.delivered-not-resolved.365"
+	if len(note.said) != 1 || !strings.HasPrefix(note.said[0], key+" warning ") ||
+		!strings.Contains(note.said[0], "fixed and delivered, not resolved") || !strings.Contains(note.said[0], "fix-x@g14") ||
+		!strings.Contains(note.said[0], "365") {
+		t.Fatalf("said %q", note.said)
+	}
+	if v := asJSON(t, mustView(t, h, n)); v["delivered-not-resolved"] != true {
+		t.Fatalf("get does not say it: %v", v["delivered-not-resolved"])
+	}
+	if _, err := h.Move(MoveAsk{Number: n, Status: StatusResolved, FixedBy: []string{"mesh-catalog#9"}}); err != nil {
+		t.Fatal(err)
+	}
+	if err := h.OnMoved(envelopeOf(n)); err != nil {
+		t.Fatal(err)
+	}
+	if len(note.clear) != 1 || note.clear[0] != key {
+		t.Fatalf("not cleared when it left fixing: %q", note.clear)
+	}
+	if rows := rowsOf(t, h, map[string]any{"flag": "delivered-not-resolved"}); len(rows) != 0 {
+		t.Fatalf("a resolved issue still listed: %v", rows)
+	}
+	// The setting's bound is read.
+	h2 := newHarness(t)
+	now2 := clocked(h2)
+	n2 := &notified{}
+	h2.Notify = n2.notify
+	// Set as the operator's settings file gives it.
+	if err := json.Unmarshal([]byte(`{"delivered-not-resolved-after": "10m"}`), &h2.settings); err != nil {
+		t.Fatal(err)
+	}
+	m := h2.open(t, "y", "mesh-catalog modules/records")
+	if _, err := h2.Move(MoveAsk{Number: m, Status: StatusFixing, By: "fix-y@g14"}); err != nil {
+		t.Fatal(err)
+	}
+	linkPR(t, h2, m, "mesh-catalog#9")
+	transition(t, h2, 9, "delivered", "", "abc", *now2)
+	*now2 = now2.Add(11 * time.Minute)
+	if _, err := h2.Sweep(); err != nil || len(n2.said) != 1 {
+		t.Fatalf("the setting's bound was not read: %q %v", n2.said, err)
+	}
+}
+
+// A warning the channel did not take is said at the next sweep, and an issue's own status is what clears it at
+// the sweep too, when the moved event was not heard.
+func TestDeliveredNotResolvedIsRetriedAndClearedAtTheSweep(t *testing.T) {
+	h := newHarness(t)
+	now := clocked(h)
+	note := &notified{err: errStuck}
+	h.Notify = note.notify
+	n := h.open(t, "x", "mesh-catalog modules/records")
+	if _, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "fix-x@g14"}); err != nil {
+		t.Fatal(err)
+	}
+	linkPR(t, h, n, "mesh-catalog#9")
+	transition(t, h, 9, "delivered", "", "abc", *now)
+	*now = now.Add(2 * time.Hour)
+	_, _ = h.Sweep()
+	note.err = nil
+	_, _ = h.Sweep()
+	if len(note.said) != 1 {
+		t.Fatalf("not said at the next sweep: %q", note.said)
+	}
+	if _, err := h.Move(MoveAsk{Number: n, Status: StatusLocated}); err != nil {
+		t.Fatal(err)
+	}
+	_, _ = h.Sweep()
+	if len(note.clear) != 1 {
+		t.Fatalf("not cleared at the sweep: %q", note.clear)
+	}
+}
+
+// Part 3: an issue in fixing with no open linked pull request and nothing done on it for a day is a stale claim,
+// naming its worker; a new worker takes it over without a reason, and the tracker says the claim was stale.
+func TestAStaleClaimIsFlaggedAndTakenOverWithoutAReason(t *testing.T) {
+	h := newHarness(t)
+	now := clocked(h)
+	n := h.open(t, "claimed and left", "hq")
+	if _, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "build-424@shanks"}); err != nil {
+		t.Fatal(err)
+	}
+	busy := h.open(t, "claimed, with a pull request in review", "hq")
+	if _, err := h.Move(MoveAsk{Number: busy, Status: StatusFixing, By: "fix-b@g14"}); err != nil {
+		t.Fatal(err)
+	}
+	linkPR(t, h, busy, "mesh-catalog#9")
+	transition(t, h, 9, "ready", "its merge", "", *now)
+	*now = now.Add(23 * time.Hour)
+	if rows := rowsOf(t, h, map[string]any{"flag": "stale-claim"}); len(rows) != 0 {
+		t.Fatalf("stale before the day: %v", rows)
+	}
+	*now = now.Add(2 * time.Hour)
+	rows := rowsOf(t, h, map[string]any{"flag": "stale-claim"})
+	if len(rows) != 1 || rows[0]["number"] != float64(n) || rows[0]["worker"] != "build-424@shanks" || rows[0]["stale-claim"] != true {
+		t.Fatalf("want only %d flagged naming its worker, got %v", n, rows)
+	}
+	if v := asJSON(t, mustView(t, h, n)); v["stale-claim"] != true {
+		t.Fatalf("get does not say it: %v", v)
+	}
+	if v := asJSON(t, mustView(t, h, busy)); v["stale-claim"] == true {
+		t.Fatal("an issue with an open pull request is flagged")
+	}
+	m, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "fix-483@shanks"})
+	if err != nil {
+		t.Fatalf("a stale claim was not taken over without a reason: %v", err)
+	}
+	if a := asJSON(t, m); a["stale-claim"] != true || !strings.Contains(m.Said, "stale claim") {
+		t.Fatalf("the answer does not say the claim was stale: %v", a)
+	}
+	is := get(t, h, n)
+	last := is.Notes[len(is.Notes)-1].Text
+	if !strings.Contains(last, "stale claim") || !strings.Contains(last, "build-424@shanks") {
+		t.Fatalf("the note does not say the claim was stale: %s", last)
+	}
+	for _, e := range h.said() {
+		if e.Event == EventMoved && e.Body["took-over-from"] == "build-424@shanks" && e.Body["stale-claim"] != true {
+			t.Fatalf("the moved event does not say the claim was stale: %v", e.Body)
+		}
+	}
+}
+
+// Part 4: list refuses a flag it does not know, naming the two it does.
+func TestListRefusesAnUnknownFlag(t *testing.T) {
+	h := newHarness(t)
+	_, err := h.call(t, "list", map[string]any{"flag": "stuck"})
+	if err == nil || !strings.Contains(err.Error(), "delivered-not-resolved") || !strings.Contains(err.Error(), "stale-claim") {
+		t.Fatalf("want refused naming the flags, got %v", err)
+	}
+}
+
+// The sweep asks the delivery of a linked pull request it never heard of, a few per sweep, so an issue whose
+// pull request merged before it was linked (as the operator's thirteen did) is measured at all.
+func TestTheSweepAsksALinkedPullRequestNeverHeardOf(t *testing.T) {
+	h := newHarness(t)
+	now := clocked(h)
+	n := h.open(t, "x", "mesh-catalog modules/records")
+	if _, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "fix-x@g14"}); err != nil {
+		t.Fatal(err)
+	}
+	linkPR(t, h, n, "mesh-catalog#209")
+	var asked []string
+	h.Deliveries = func(repo, number string) (Delivery, error) {
+		asked = append(asked, repo+"#"+number)
+		return Delivery{State: "delivered", Since: now.Add(-3 * time.Hour)}, nil
+	}
+	if _, err := h.Sweep(); err != nil {
+		t.Fatal(err)
+	}
+	if len(asked) != 1 || asked[0] != "mesh-catalog#209" {
+		t.Fatalf("asked %q", asked)
+	}
+	if v, _ := h.View(n); v.Delivery != "delivered to every machine it should reach" || countEvents(h, EventReadyToVerify) != 1 {
+		t.Fatalf("%q, %d ready-to-verify", v.Delivery, countEvents(h, EventReadyToVerify))
+	}
+	if _, err := h.Sweep(); err != nil || len(asked) != 1 {
+		t.Fatalf("asked again once heard: %q %v", asked, err)
+	}
+}
+
+func envelopeOf(n uint64) stdio.Envelope {
+	raw, _ := json.Marshal(map[string]any{"number": n})
+	return stdio.Envelope{Key: "mesh-issues.moved", Body: raw}
+}
+
+func mustView(t *testing.T, h *harness, n uint64) *View {
+	t.Helper()
+	v, err := h.View(n)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return v
+}