Read a stop a later merge covered as covered, and keep it in deliveries while stalled lists it
A stop for a walk that never built its modules (issue 474) kept saying "error: stopped" on the forge and in show after a later delivered merge carried its change to every machine. The stop stays final, as a covered superseded member stays superseded (issue 442); its status, view, note and show now say covered and by which merge, told once and from what is kept, so a restart or a stop covered before this tells it too. deliveries dropped such a stop after a day while stalled still listed it; it now lists it as long as stalled does. novox/hq issue 480, points 1 and 3.
This commit is contained in:
@@ -104,6 +104,9 @@ func statusOf(d *Delivery) (string, string) {
|
||||
case Superseded:
|
||||
return "warning", clip("superseded: "+why, 140)
|
||||
case Stopped:
|
||||
if d.coveredStop() {
|
||||
return "success", clip(d.coveredStopSaid(), 140) // its change reached the machines (novox/hq issue 480)
|
||||
}
|
||||
return "error", clip("stopped: "+why, 140)
|
||||
case Retired:
|
||||
return "success", "history: merged before this owner delivered anything, never walked"
|
||||
@@ -124,6 +127,9 @@ const viewMarker = "<!-- mesh-delivery:view -->"
|
||||
func ViewBody(d *Delivery, g *Group) string {
|
||||
var b strings.Builder
|
||||
fmt.Fprintf(&b, "%s\n**Delivery** `%s` — **%s** since %s\n\n", viewMarker, d.ID, d.State, d.Since.UTC().Format(time.RFC3339))
|
||||
if d.coveredStop() {
|
||||
fmt.Fprintf(&b, "**Covered** after its stop: a later merge that contains it was delivered: %s\n\n", d.CoveredBy)
|
||||
}
|
||||
if d.HeldWhy != "" && d.State == Held {
|
||||
fmt.Fprintf(&b, "Held for a person: %s — `mesh-delivery.release` with why lets it go on.\n\n", d.HeldWhy)
|
||||
}
|
||||
|
||||
@@ -953,12 +953,16 @@ func (h *Holder) plan() []func() {
|
||||
// grace, it is stopped saying so — never left waiting for a start that cannot come.
|
||||
for _, d := range h.deliveries {
|
||||
ended := d.endedUnstarted()
|
||||
if d.coveredStop() && !d.CoverTold {
|
||||
h.tellCovered(d, now) // covered before this holder told it, or before a restart (novox/hq issue 480)
|
||||
continue
|
||||
}
|
||||
if strandedUncovered(d) {
|
||||
// Stopped and final, but still read for a later merge that delivered its modules: until one did,
|
||||
// `stalled` keeps listing it (review of novox/hq issue 474).
|
||||
var ask []string
|
||||
if d.CoveredBy, ask = h.coverOf(d); d.CoveredBy != "" {
|
||||
h.keep(d)
|
||||
h.tellCovered(d, now)
|
||||
}
|
||||
for _, in := range ask {
|
||||
if a := h.askContains(d, in, now); a != nil {
|
||||
@@ -1103,6 +1107,25 @@ func (h *Holder) laterOnItsWay(d *Delivery) string {
|
||||
return ""
|
||||
}
|
||||
|
||||
// tellCovered tells the forge that a stop is covered since, once: its status says covered and by which merge, its view
|
||||
// says so, and its note keeps the line on its head and its merge commit (novox/hq issue 480). No transition: the stop
|
||||
// stays final, and the bus's transition event is not said again.
|
||||
func (h *Holder) tellCovered(d *Delivery, now time.Time) {
|
||||
d.CoverTold = true
|
||||
line := oneLine(fmt.Sprintf("%s delivery %s: stopped, %s", now.UTC().Format(time.RFC3339), d.ID, d.coveredStopSaid()))
|
||||
d.Owed = append(d.Owed, Effect{Kind: EffectNote, Commit: d.Commit, Line: line, Since: now})
|
||||
if d.MergedAs != "" && !sameCommit(d.MergedAs, d.Commit) {
|
||||
d.Owed = append(d.Owed, Effect{Kind: EffectNote, Commit: d.MergedAs, Line: line, Since: now})
|
||||
}
|
||||
if d.shownOnItsPullRequest() {
|
||||
d.owe(Effect{Kind: EffectView, Since: now})
|
||||
state, desc := statusOf(d)
|
||||
d.owe(Effect{Kind: EffectStatus, Commit: d.Commit, Context: "mesh/delivery", State: state, Line: desc, Since: now})
|
||||
}
|
||||
h.Logf("[mesh-delivery] %s: stopped, %s", d.ID, d.coveredStopSaid())
|
||||
h.keep(d)
|
||||
}
|
||||
|
||||
// strandedUncovered is whether a delivery stopped because its ended walk delivered nothing is still uncovered: a
|
||||
// merged change that never reached the machines, which `stalled` keeps listing until a later delivered merge
|
||||
// carries its modules (review of novox/hq issue 474).
|
||||
|
||||
@@ -0,0 +1,134 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// novox/hq issue 480: a delivery stopped because its walk ended done before its word without building its modules
|
||||
// (issue 474) is still read for a later merge that delivers them, and kept CoveredBy once one did. Its state stays
|
||||
// stopped — a final state never moves, as for a covered superseded member (issue 442) — but the forge's status, its
|
||||
// view, its note and `show` went on saying "stopped" for a change that reached every machine. Once covered they say
|
||||
// covered, and by which merge. And while `stalled` lists such a stop, `deliveries` lists it too, past a day.
|
||||
|
||||
// strandedNight is theNight with #215 stopped, its walk past the grace with nothing later carrying build-agent.
|
||||
func strandedNight(t *testing.T) (*world, string) {
|
||||
w := theNight(t)
|
||||
id := IDOf(ctl474, head215)
|
||||
w.later(strandedAfter + 5*time.Minute)
|
||||
w.settleAll()
|
||||
if d := w.delivery(id); d.State != Stopped || d.Stranded == "" {
|
||||
t.Fatalf("#215 is %s, stranded %q", d.State, d.Stranded)
|
||||
}
|
||||
return w, id
|
||||
}
|
||||
|
||||
func TestAStopALaterMergeCoversReadsCoveredByThatMerge(t *testing.T) {
|
||||
w, id := strandedNight(t)
|
||||
if got := statusOn(w, ctl474, head215); !strings.HasPrefix(got, "error stopped: ") {
|
||||
t.Fatalf("stopped, its status says %q", got)
|
||||
}
|
||||
w.forge.contains[merge215+" in "+merge219] = true
|
||||
by := deliverLater(w, ctl474, 219, "main", head219, merge219, w.now, walkDone, "build-agent")
|
||||
w.later(2 * time.Minute)
|
||||
w.settleAll()
|
||||
d := w.delivery(id)
|
||||
if d.State != Stopped || !strings.Contains(d.CoveredBy, by) {
|
||||
t.Fatalf("after #219 it is %s, covered by %q", d.State, d.CoveredBy)
|
||||
}
|
||||
got := statusOn(w, ctl474, head215)
|
||||
if !strings.HasPrefix(got, "success covered") || !strings.Contains(got, by) {
|
||||
t.Fatalf("covered by %s, its status still says %q", by, got)
|
||||
}
|
||||
shown, err := w.h.Show(id)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if reads, _ := shown.(map[string]any)["reads"].(string); !strings.HasPrefix(reads, "covered") || !strings.Contains(reads, by) {
|
||||
t.Fatalf("show reads %q", reads)
|
||||
}
|
||||
view := w.forge.views[ctl474+"#215"]
|
||||
if !strings.Contains(view, "Covered") || !strings.Contains(view, by) {
|
||||
t.Fatalf("its view does not say it was covered by %s:\n%s", by, view)
|
||||
}
|
||||
noted := false
|
||||
for _, l := range w.forge.notesOn(merge215) {
|
||||
noted = noted || strings.Contains(l, "covered") && strings.Contains(l, by)
|
||||
}
|
||||
if !noted {
|
||||
t.Fatalf("its merge's note does not say it was covered: %v", w.forge.notesOn(merge215))
|
||||
}
|
||||
// Told once: further ticks owe nothing more.
|
||||
w.later(time.Hour)
|
||||
w.settleAll()
|
||||
if d := w.delivery(id); len(d.Owed) != 0 || !d.CoverTold {
|
||||
t.Fatalf("still owes %+v, told %v", d.Owed, d.CoverTold)
|
||||
}
|
||||
}
|
||||
|
||||
// A stop covered before this was built (CoveredBy kept, nothing told) is told after a restart, from what was kept.
|
||||
func TestACoveredStopKeptBeforeIsToldAfterARestart(t *testing.T) {
|
||||
w, id := strandedNight(t)
|
||||
w.h.mu.Lock()
|
||||
d := w.h.deliveries[id]
|
||||
d.CoveredBy = IDOf(ctl474, head219) + " (merged as " + merge219[:8] + ")"
|
||||
if err := w.store.PutDelivery(d); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
w.h.mu.Unlock()
|
||||
again := newWorldOver(t, w.store, w.ctl, w.forge, w.now.Add(time.Hour))
|
||||
again.settleAll()
|
||||
if got := statusOn(again, ctl474, head215); !strings.HasPrefix(got, "success covered") ||
|
||||
!strings.Contains(got, IDOf(ctl474, head219)) {
|
||||
t.Fatalf("after a restart its status says %q", got)
|
||||
}
|
||||
if d := again.delivery(id); d.State != Stopped || len(d.Transitions) != len(w.delivery(id).Transitions) {
|
||||
t.Fatalf("a restart moved it: %s", d.State)
|
||||
}
|
||||
}
|
||||
|
||||
// Only a stop for a walk that never built its modules reads covered: a person's stop, or one the cover test has
|
||||
// not answered, keeps its error.
|
||||
func TestOnlyACoveredStrandedStopReadsCovered(t *testing.T) {
|
||||
for _, d := range []*Delivery{
|
||||
{State: Stopped, CoveredBy: "x"}, // a person's stop, with a cover found while it was held
|
||||
{State: Stopped, Stranded: "never reached"}, // stranded, not covered
|
||||
} {
|
||||
if state, _ := statusOf(d); state != "error" {
|
||||
t.Fatalf("%+v reads %s", d, state)
|
||||
}
|
||||
if d.coveredStop() {
|
||||
t.Fatalf("%+v reads as a covered stop", d)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestAStopStalledListsStaysInDeliveries(t *testing.T) {
|
||||
w, id := strandedNight(t)
|
||||
w.later(30 * time.Hour)
|
||||
w.settleAll()
|
||||
listed := func() *Line {
|
||||
for _, l := range w.h.Deliveries("", "", "", false) {
|
||||
if l.ID == id {
|
||||
return &l
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
if len(w.h.Stalled()) != 1 {
|
||||
t.Fatalf("stalled lists %+v", w.h.Stalled())
|
||||
}
|
||||
l := listed()
|
||||
if l == nil || !l.Stalled || !strings.Contains(l.Waits, "build-agent") {
|
||||
t.Fatalf("listed by stalled, deliveries shows %+v", l)
|
||||
}
|
||||
// Covered, it ends like any final delivery: a day after its stop.
|
||||
w.forge.contains[merge215+" in "+merge219] = true
|
||||
deliverLater(w, ctl474, 219, "main", head219, merge219, w.now, walkDone, "build-agent")
|
||||
w.later(2 * time.Minute)
|
||||
w.settleAll()
|
||||
if len(w.h.Stalled()) != 0 || listed() != nil {
|
||||
t.Fatalf("covered, stalled lists %+v and deliveries %+v", w.h.Stalled(), listed())
|
||||
}
|
||||
}
|
||||
@@ -68,6 +68,9 @@ type Delivery struct {
|
||||
// Stranded says why a merge whose walk ended done before its word, without moving every module it moves, is
|
||||
// stopped: nothing later delivered them either (novox/hq issue 474). Set once the cover test says no.
|
||||
Stranded string `json:"stranded,omitempty"`
|
||||
// CoverTold is whether the forge was told that such a stop was covered since: its status, its view and its note
|
||||
// (novox/hq issue 480). Kept, so a restart tells it once, and a stop covered before this was built is told too.
|
||||
CoverTold bool `json:"cover_told,omitempty"`
|
||||
// TurnAwaits says, for a group member whose turn waits for a later merge to carry a superseded member before it,
|
||||
// which merges and until when; TurnAwaitsUntil is that moment, which bounds the wait (ADR 0276 decision 9,
|
||||
// novox/hq issue 445). `show` shows both, and `stalled` says them past that moment.
|
||||
@@ -248,6 +251,20 @@ func (d *Delivery) FromForgesWord() bool {
|
||||
|
||||
// shownOnItsPullRequest is whether its view and status are kept on its pull request: not for history, whose
|
||||
// pull request the forge may no longer hold — or holds under its number another one.
|
||||
// coveredStop is whether a delivery stopped because its walk ended done before its word without building its modules
|
||||
// (novox/hq issue 474) was covered since: a later merge into its trunk that contains it was delivered, carrying every
|
||||
// module it moves to every machine running it (ADR 0270's cover test, kept as CoveredBy). Its state stays stopped —
|
||||
// final, as a covered superseded member stays superseded (issue 442) — and it reads covered, by that merge (novox/hq
|
||||
// issue 480). A stop for any other reason never reads covered.
|
||||
func (d *Delivery) coveredStop() bool {
|
||||
return d.State == Stopped && d.Stranded != "" && d.CoveredBy != ""
|
||||
}
|
||||
|
||||
// coveredStopSaid is how a covered stop reads: on the forge's status, in its view and note, and in `show`.
|
||||
func (d *Delivery) coveredStopSaid() string {
|
||||
return "covered after its stop: a later merge that contains it was delivered: " + d.CoveredBy
|
||||
}
|
||||
|
||||
func (d *Delivery) shownOnItsPullRequest() bool {
|
||||
return d.Number > 0 && d.History == "" && d.State != Retired
|
||||
}
|
||||
|
||||
@@ -71,6 +71,11 @@ func (h *Holder) waitsFor(d *Delivery) string {
|
||||
if d.Walk != nil {
|
||||
return "its walk: " + clip(d.Walk.Note, 120)
|
||||
}
|
||||
case Stopped:
|
||||
if strandedUncovered(d) {
|
||||
return "a later delivered merge to carry " + strings.Join(d.notMovedBy(), ", ") + ": it is stopped, and " +
|
||||
"listed by stalled until one does (novox/hq issue 480)"
|
||||
}
|
||||
}
|
||||
return ""
|
||||
}
|
||||
@@ -86,7 +91,8 @@ func (h *Holder) Deliveries(state, repository, group string, all bool) []Line {
|
||||
group != "" && d.Group != group {
|
||||
continue
|
||||
}
|
||||
if d.State.Final() && !all && now.Sub(d.Since) > 24*time.Hour {
|
||||
// A final delivery `stalled` still lists stays, however old: the two never disagree (novox/hq issue 480).
|
||||
if d.State.Final() && !all && now.Sub(d.Since) > 24*time.Hour && !strandedUncovered(d) {
|
||||
continue
|
||||
}
|
||||
l := Line{ID: d.ID, State: d.State, Since: d.Since.UTC().Format(time.RFC3339), For: now.Sub(d.Since).Round(time.Second).String(),
|
||||
@@ -97,7 +103,7 @@ func (h *Holder) Deliveries(state, repository, group string, all bool) []Line {
|
||||
if d.Walk != nil {
|
||||
l.Walk = d.Walk.ID
|
||||
}
|
||||
l.Stalled = pastBound(d, now)
|
||||
l.Stalled = pastBound(d, now) || strandedUncovered(d)
|
||||
out = append(out, l)
|
||||
}
|
||||
sort.Slice(out, func(i, j int) bool {
|
||||
@@ -115,6 +121,9 @@ func (h *Holder) Show(id string) (any, error) {
|
||||
defer h.mu.Unlock()
|
||||
if d := h.deliveries[id]; d != nil {
|
||||
out := map[string]any{"delivery": d, "waits": h.waitsFor(d)}
|
||||
if d.coveredStop() {
|
||||
out["reads"] = d.coveredStopSaid() // stopped, and its change reached the machines (novox/hq issue 480)
|
||||
}
|
||||
if g := h.groups[d.Group]; g != nil {
|
||||
state, why := GroupState(g, h.membersOf(g))
|
||||
out["group"] = map[string]any{"group": g, "state": state, "why": why}
|
||||
|
||||
Reference in New Issue
Block a user