The gate names the issues linked in the issue tracker and the forge's author, and reads no description (milestone 47/G, W11.5)
ADR 0659 §4 kept three description reads that shape words only until they move to the bus: the issue numbers the ask names, the author a stalled line names and the notice that after: lines are not read. The ask now names the issues linked to each member's pull request, read once per head through the seat issue-tracker's list (a tracker that does not answer is said, never waited on); the author is the forge's pull request record's, from the seat git's events; the after: notice is gone. The gate's forms of a pull request and a delivery hold no description field. Claude-Session: https://claude.ai/code/session_01JG73KEq4zoDfM9WtYpcQgZ (Joined from ddd93a78.) Builds: design 47 G §50, §143 (GATE15) Claude-Session: https://claude.ai/code/session_01JG73KEq4zoDfM9WtYpcQgZ
This commit is contained in:
@@ -33,8 +33,9 @@ names every refusal and rejection at once, and the pull request is told at once;
|
||||
`refusal-stall-floor-minutes`, 10). Every other refusal is stalled at once.
|
||||
- a delivery that needs its author's new head (its check failed after its Approve: a conflict, a failed check, a rejected
|
||||
group) is stalled only once no new head came for the setting `author-new-head-minutes` (120). Its line says
|
||||
`needs: new-head` and names its author: the branch, and the session a `Pushed-by: <who@node>` line in the pull
|
||||
request's description names.
|
||||
`needs: new-head` and names its author: the branch, and who opened the pull request as the forge's record says
|
||||
(the seat git's events). The ask names the issues linked to each pull request in the issue tracker (the seat
|
||||
issue-tracker's `list`); nothing is read from a description.
|
||||
|
||||
## A merge the forge account cannot carry out is not asked (issue 1945)
|
||||
|
||||
|
||||
@@ -131,15 +131,13 @@ func TestAnyOtherRefusalIsStalledAtOnce(t *testing.T) {
|
||||
}
|
||||
|
||||
// rejectedBeforeItsAnswer is a delivery approved and then rejected: its check failed after the ask (as in events_test.go),
|
||||
// its pull request's description naming who pushed it when pushedBy is given.
|
||||
func rejectedBeforeItsAnswer(t *testing.T, pushedBy string) (*world, string) {
|
||||
// its pull request opened by author as the forge's record says, when given.
|
||||
func rejectedBeforeItsAnswer(t *testing.T, author string) (*world, string) {
|
||||
t.Helper()
|
||||
w := newWorld(t)
|
||||
w.op.auto = false
|
||||
p := pr("novox/app", 7, readyHead, "feat/x", "src/a.go")
|
||||
if pushedBy != "" {
|
||||
p.Body += "\nPushed-by: " + pushedBy
|
||||
}
|
||||
p.Author = author
|
||||
w.h.PullUpdated(p)
|
||||
w.settleAll()
|
||||
id := IDOf("novox/app", readyHead)
|
||||
@@ -170,7 +168,7 @@ func rejectedBeforeItsAnswer(t *testing.T, pushedBy string) (*world, string) {
|
||||
// with no new head, in words naming its author (its branch, and the session that pushed it) and needing a new head —
|
||||
// never "stop it", never "the operator's".
|
||||
func TestARejectedDeliveryIsItsAuthorsForTwoHours(t *testing.T) {
|
||||
w, id := rejectedBeforeItsAnswer(t, "quiet-deliveries@shanks")
|
||||
w, id := rejectedBeforeItsAnswer(t, "quiet-deliveries")
|
||||
said := false
|
||||
for _, c := range w.forge.comments {
|
||||
said = said || strings.Contains(c, "not merged")
|
||||
@@ -188,7 +186,7 @@ func TestARejectedDeliveryIsItsAuthorsForTwoHours(t *testing.T) {
|
||||
t.Fatalf("not the operator's after two hours with no new head: %+v", got)
|
||||
}
|
||||
l := got[0]
|
||||
if l.Needs != NeedsNewHead || l.Author != "the author of feat/x (quiet-deliveries on shanks)" || l.Branch != "feat/x" {
|
||||
if l.Needs != NeedsNewHead || l.Author != "the author of feat/x (quiet-deliveries)" || l.Branch != "feat/x" {
|
||||
t.Fatalf("the line does not name its author and the new head it needs: %+v", l)
|
||||
}
|
||||
for _, never := range []string{"operator's", "stop it"} {
|
||||
@@ -196,13 +194,13 @@ func TestARejectedDeliveryIsItsAuthorsForTwoHours(t *testing.T) {
|
||||
t.Fatalf("the line says %q: %+v", never, l)
|
||||
}
|
||||
}
|
||||
if !strings.Contains(l.Says, "needs a new head from the author of feat/x (quiet-deliveries on shanks)") {
|
||||
if !strings.Contains(l.Says, "needs a new head from the author of feat/x (quiet-deliveries)") {
|
||||
t.Fatalf("the words do not say a new head is needed from its author: %q", l.Says)
|
||||
}
|
||||
}
|
||||
|
||||
// ADR 0505 §5: with no Pushed-by line, the branch alone names its author.
|
||||
func TestARejectedDeliveryWithNoPushedByLineIsNamedByItsBranch(t *testing.T) {
|
||||
// ADR 0505 §5: with no author in the forge's record, the branch alone names its author.
|
||||
func TestARejectedDeliveryWithNoAuthorIsNamedByItsBranch(t *testing.T) {
|
||||
w, id := rejectedBeforeItsAnswer(t, "")
|
||||
w.later(2*time.Hour + time.Minute)
|
||||
if got := linesFor(w, id); len(got) != 1 || got[0].Author != "the author of feat/x" {
|
||||
@@ -212,7 +210,7 @@ func TestARejectedDeliveryWithNoPushedByLineIsNamedByItsBranch(t *testing.T) {
|
||||
|
||||
// ADR 0505 §5: a new head within the two hours ends it: the rejected head is never the operator's.
|
||||
func TestANewHeadWithinTwoHoursKeepsTheRejectedHeadFromTheOperator(t *testing.T) {
|
||||
w, id := rejectedBeforeItsAnswer(t, "quiet-deliveries@shanks")
|
||||
w, id := rejectedBeforeItsAnswer(t, "quiet-deliveries")
|
||||
w.later(time.Hour)
|
||||
w.h.PullUpdated(pr("novox/app", 7, pushed, "feat/x", "src/a.go"))
|
||||
w.settleAll()
|
||||
|
||||
@@ -98,26 +98,14 @@ func phone(s string, n int) string {
|
||||
return s
|
||||
}
|
||||
|
||||
var issueNamed = regexp.MustCompile(`(?i)\bissues?\s*#?(\d+)`)
|
||||
|
||||
// IssuesIn reads the issues a pull request's title and body name ("issue 948"), ascending, once each.
|
||||
func IssuesIn(text string) []int {
|
||||
seen := map[int]bool{}
|
||||
var out []int
|
||||
for _, m := range issueNamed.FindAllStringSubmatch(text, -1) {
|
||||
if n, err := strconv.Atoi(m[1]); err == nil && !seen[n] {
|
||||
seen[n] = true
|
||||
out = append(out, n)
|
||||
}
|
||||
}
|
||||
sort.Ints(out)
|
||||
return out
|
||||
}
|
||||
|
||||
// issuesSaid is the issues linked to the deliveries' pull requests in the issue tracker, as the ask names them.
|
||||
func issuesSaid(ds []*Delivery) string {
|
||||
seen := map[int]bool{}
|
||||
var out []string
|
||||
for _, d := range ds {
|
||||
if d.IssuesUnread != "" {
|
||||
out = append(out, pullSaid(d)+"'s unread (the issue tracker did not answer)")
|
||||
}
|
||||
for _, n := range d.Issues {
|
||||
if !seen[n] {
|
||||
seen[n] = true
|
||||
@@ -126,7 +114,7 @@ func issuesSaid(ds []*Delivery) string {
|
||||
}
|
||||
}
|
||||
if len(out) == 0 {
|
||||
return "none named"
|
||||
return "none linked"
|
||||
}
|
||||
return strings.Join(out, ", ")
|
||||
}
|
||||
@@ -440,6 +428,9 @@ func (h *Holder) approval(d *Delivery, now time.Time) (string, func()) {
|
||||
if waits := h.writeWaits(d, members, now); waits != "" {
|
||||
return waits, nil // asked only once the forge account can carry the merge out (writes.go, issue 1945)
|
||||
}
|
||||
if read := h.issuesToRead(members); read != nil {
|
||||
return "the issues linked to its pull request are read from the issue tracker", read
|
||||
}
|
||||
group := ""
|
||||
if len(members) > 1 {
|
||||
group = d.Group
|
||||
|
||||
@@ -562,7 +562,6 @@ func TestAPassingVerdictAsksTheApproveAndItsApproveMergesWithinSecondsOverARealB
|
||||
announced := pr("novox/app", 7, head, "fix/x", "src/a.go")
|
||||
announced.BaseSHA = "a1a1a1a1a1a1"
|
||||
announced.Title = "Fix the apostrophe (issue 975)"
|
||||
announced.Body = "The description, with no review line yet."
|
||||
b.publish(t, "mesh.seat.git.event.pull.updated", seatUpdated(announced))
|
||||
rt.until("the pull request's key to hold its head", eventWithin, func() bool {
|
||||
e, err := b.kvs["pulls"].Get(ctx, "novox.app.7")
|
||||
@@ -754,7 +753,6 @@ func TestAGroupOfTwoWithGreenChecksGetsOneAskAndOnItsApproveBothMergeOneAfterThe
|
||||
for _, p := range []PullEvent{pr("novox/mesh-controller", 1, ctlHead, "feat/x", "internal/a.go"),
|
||||
pr("novox/mesh-catalog", 2, catHead, "feat/x", "modules/app/module.json")} {
|
||||
p.BaseSHA = "a1a1a1a1a1a1"
|
||||
p.Body = "Reviewed-by: lead@shanks at " + p.HeadSHA
|
||||
b.publish(t, "mesh.seat.git.event.pull.updated", seatUpdated(p))
|
||||
}
|
||||
// Both members pass at the checker, whose one verdict covers both heads: the group is asked once, no composed check is asked
|
||||
|
||||
@@ -2,6 +2,7 @@ package main
|
||||
|
||||
import (
|
||||
gitseat "git.novox.be/novox/mesh-delivery/contracts/go/seats/git"
|
||||
"reflect"
|
||||
|
||||
"encoding/json"
|
||||
"strings"
|
||||
@@ -120,29 +121,31 @@ func (*askedAgainError) Error() string {
|
||||
return "the forge refused gitea_merge_pull_request: novox/app#7 is not merged: its verdict was judged against trunk aaaaaaaaaaaa, and main is now bbbbbbbbbbbb. The check of head cccccccccccc merged into trunk bbbbbbbbbbbb was asked; took 3s. Ask again at cccccccccccc once both statuses pass. Nothing was merged"
|
||||
}
|
||||
|
||||
// The gate reads nothing from a pull request's description (novox/hq ADR 0493 §6, ADR 0659, which ends ADR 0496): a pull request whose description
|
||||
// holds no review line, or one naming another head, is asked once its other waits are met, and the ask names no review.
|
||||
// The gate reads nothing from a pull request's description (novox/hq ADR 0493 §6, ADR 0659, which ends ADR 0496): its form
|
||||
// of a pull request and its delivery hold no description, no review line, no pushed-by and no after: line, so no wait can
|
||||
// read one; a pull request is asked once its other waits are met, and the ask names no review.
|
||||
func TestAPullRequestIsAskedWhateverItsDescriptionSays(t *testing.T) {
|
||||
for _, c := range []struct{ name, body string }{
|
||||
{"no line", "Fixes issue 7."},
|
||||
{"another head", "Reviewed-by: lead@shanks at " + pushed},
|
||||
{"empty", ""},
|
||||
} {
|
||||
t.Run(c.name, func(t *testing.T) {
|
||||
w := newWorld(t)
|
||||
w.op.auto = false
|
||||
p := pr("novox/app", 7, readyHead, "feat/x", "src/a.go")
|
||||
p.Body = c.body
|
||||
w.h.PullUpdated(p)
|
||||
hear(t, w, shimCheckedKey, judgedAt("novox/app", 7, readyHead, "pass", "t1t1t1t1t1t1"))
|
||||
open := w.op.open()
|
||||
if len(open) != 1 {
|
||||
t.Fatalf("an unreviewed pull request with its other waits met was not asked: %d", len(open))
|
||||
for _, form := range []any{PullEvent{}, Delivery{}} {
|
||||
ty := reflect.TypeOf(form)
|
||||
for i := 0; i < ty.NumField(); i++ {
|
||||
name := strings.ToLower(ty.Field(i).Name)
|
||||
for _, never := range []string{"body", "description", "pushed", "after", "review"} {
|
||||
if strings.Contains(name, never) {
|
||||
t.Errorf("%s holds the field %s, a description's", ty.Name(), ty.Field(i).Name)
|
||||
}
|
||||
}
|
||||
if e := w.op.ask(open[0]).Explanation; strings.Contains(strings.ToLower(e), "review") {
|
||||
t.Fatalf("the ask names a review:\n%s", e)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
w := newWorld(t)
|
||||
w.op.auto = false
|
||||
w.h.PullUpdated(pr("novox/app", 7, readyHead, "feat/x", "src/a.go"))
|
||||
hear(t, w, shimCheckedKey, judgedAt("novox/app", 7, readyHead, "pass", "t1t1t1t1t1t1"))
|
||||
open := w.op.open()
|
||||
if len(open) != 1 {
|
||||
t.Fatalf("an unreviewed pull request with its other waits met was not asked: %d", len(open))
|
||||
}
|
||||
if e := w.op.ask(open[0]).Explanation; strings.Contains(strings.ToLower(e), "review") {
|
||||
t.Fatalf("the ask names a review:\n%s", e)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -170,41 +173,3 @@ func TestAPushAfterTheAskEndsItAndSaysSoOnceOnThePullRequest(t *testing.T) {
|
||||
t.Fatalf("said more than once: %v", w.forge.comments)
|
||||
}
|
||||
}
|
||||
|
||||
// ADR 0659 clause 6: no wait of ready.go reads a description field. A delivery and a group whose descriptions differ only
|
||||
// (no line, a Reviewed-by line naming the head, another head, garbage, an after: and a Pushed-by line) answer the same
|
||||
// readyWaits, ungatedWaits and group state.
|
||||
func TestNoWaitReadsADescription(t *testing.T) {
|
||||
heads := map[string]string{"novox/a": "a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1", "novox/b": "b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2"}
|
||||
bodies := []string{"", "Fixes issue 7.", "Reviewed-by: lead@shanks at %s", "Reviewed-by: lead@shanks at " + pushed,
|
||||
"\x00garbage: Reviewed-by pending\nafter: novox/zzz\nPushed-by: x@y"}
|
||||
var first []string
|
||||
for i, body := range bodies {
|
||||
w := newWorld(t)
|
||||
w.op.auto = false
|
||||
for n, repo := range []string{"novox/a", "novox/b"} {
|
||||
p := pr(repo, n+1, heads[repo], "feat/y", "x")
|
||||
p.Body = strings.ReplaceAll(body, "%s", heads[repo])
|
||||
w.h.PullUpdated(p)
|
||||
w.h.Checked(verdict(repo, n+1, heads[repo], "pass"))
|
||||
}
|
||||
w.h.Checked(CheckedEvent{Group: "feat/y", ID: "check-1", Verdict: "pass", Summary: "compose"})
|
||||
w.settleAll()
|
||||
g := w.h.groups["feat/y"]
|
||||
state, why := GroupState(g, w.h.membersOf(g))
|
||||
ds := []*Delivery{w.delivery(IDOf("novox/a", heads["novox/a"])), w.delivery(IDOf("novox/b", heads["novox/b"]))}
|
||||
w.h.mu.Lock()
|
||||
got := []string{state, why}
|
||||
for _, d := range ds {
|
||||
got = append(got, w.h.askWaits(d), w.h.approvedWaits(d))
|
||||
}
|
||||
w.h.mu.Unlock()
|
||||
if i == 0 {
|
||||
first = got
|
||||
continue
|
||||
}
|
||||
if strings.Join(got, "|") != strings.Join(first, "|") {
|
||||
t.Fatalf("description %q changes the waits:\n got %q\nwant %q", body, got, first)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5,7 +5,6 @@ import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"slices"
|
||||
"sort"
|
||||
"strings"
|
||||
"sync"
|
||||
@@ -45,6 +44,8 @@ type Holder struct {
|
||||
Consumer *StepConsumer
|
||||
// SaySeat says an event of the seat mesh-delivery (recordevents.go): heads-replaced here; nil says none.
|
||||
SaySeat SeatSay
|
||||
// Links reads the issues linked to a pull request from the issue tracker (issuelinks.go); nil reads none.
|
||||
Links IssueLinks
|
||||
// Pulls keeps the pull requests' keys on the bus (pullkey.go); nil keeps none.
|
||||
Pulls PullKeys
|
||||
// Say says a condition to the operator, and Conditions keeps which the gate raised (conditions.go); nil says none.
|
||||
@@ -297,7 +298,6 @@ type PullEvent struct {
|
||||
Repo string `json:"repo"`
|
||||
Number int `json:"number"`
|
||||
Title string `json:"title,omitempty"`
|
||||
Body string `json:"body,omitempty"`
|
||||
Base string `json:"base"`
|
||||
Head string `json:"head"`
|
||||
HeadSHA string `json:"head_sha"`
|
||||
@@ -357,14 +357,9 @@ func (h *Holder) pullUpdated(p PullEvent) (isNew bool) {
|
||||
h.fillFromAnnouncement(d, p)
|
||||
// A pull request retargeted to another base (review of issue 1422): what it merges into is the forge's word now.
|
||||
h.retarget(d, p)
|
||||
// So is the line naming who pushed it (ADR 0505 §5).
|
||||
if by := PushedIn(p.Body); p.Body != "" && by != d.PushedBy {
|
||||
d.PushedBy = by
|
||||
h.keep(d)
|
||||
}
|
||||
// An `after:` line written into the description later is told about too (novox/hq ADR 0419 §2).
|
||||
if after := AfterIn(p.Body); p.Body != "" && !slices.Equal(after, d.After) {
|
||||
d.After = after
|
||||
// So is who opened it, from the forge's pull request record (ADR 0505 §5, ADR 0659 §4).
|
||||
if p.Author != "" && p.Author != d.Author {
|
||||
d.Author = p.Author
|
||||
h.keep(d)
|
||||
}
|
||||
if d.State == Proposed && d.outOfDate() != "" && d.AskedAgain == nil {
|
||||
@@ -449,8 +444,9 @@ func (h *Holder) fillFromAnnouncement(d *Delivery, p PullEvent) {
|
||||
d.Title, d.Base, d.Branch, d.HTMLURL, d.CloneURL = p.Title, p.Base, p.Head, p.HTMLURL, p.CloneURL
|
||||
d.Paths, d.PathsTruncated, d.Removed = p.Paths, p.PathsTruncated, p.Removed
|
||||
d.ModuleDirs, d.ModuleDirsSaid = p.ModuleDirs, p.ModuleDirsSaid
|
||||
d.Issues, d.After = IssuesIn(p.Title+"\n"+p.Body), AfterIn(p.Body)
|
||||
d.PushedBy = PushedIn(p.Body)
|
||||
if p.Author != "" {
|
||||
d.Author = p.Author
|
||||
}
|
||||
if p.BaseSHA != "" {
|
||||
d.BaseSHA, d.BaseSHAAt = p.BaseSHA, h.Now()
|
||||
}
|
||||
@@ -485,9 +481,9 @@ func (h *Holder) retarget(d *Delivery, p PullEvent) {
|
||||
h.keep(d)
|
||||
}
|
||||
|
||||
// PullEdited takes a pull request's edit (novox/hq issue 1079): its `after:` lines, the `Pushed-by` line and the issues it names
|
||||
// are read again, exactly as an announcement reads them; nothing here holds or withdraws an ask. An edit with no description,
|
||||
// or of a pull request with no open delivery at that head, changes nothing.
|
||||
// PullEdited takes a pull request's edit (novox/hq issue 1079): a delivery made from its verdict is filled from it, and a
|
||||
// record stopped under another, closed pull request at this head is taken over; nothing of its description is read
|
||||
// (novox/hq ADR 0659), and nothing here holds or withdraws an ask.
|
||||
func (h *Holder) PullEdited(p PullEvent) {
|
||||
if p.HeadSHA == "" || p.Number == 0 {
|
||||
return
|
||||
@@ -515,20 +511,6 @@ func (h *Holder) PullEdited(p PullEvent) {
|
||||
if d != nil && d.Number == p.Number {
|
||||
h.fillFromAnnouncement(d, p) // a delivery made from its verdict, its head never heard (issue 1726)
|
||||
}
|
||||
if p.Body == "" {
|
||||
return
|
||||
}
|
||||
if d == nil || d.Number != p.Number || d.State.Final() || d.MergedAs != "" {
|
||||
return
|
||||
}
|
||||
d.PushedBy = PushedIn(p.Body)
|
||||
d.After = AfterIn(p.Body)
|
||||
title := p.Title
|
||||
if title == "" {
|
||||
title = d.Title
|
||||
}
|
||||
d.Issues = IssuesIn(title + "\n" + p.Body)
|
||||
h.keep(d)
|
||||
}
|
||||
|
||||
// withOther is others with p put in, replacing an earlier announcement of the same pull request.
|
||||
@@ -581,11 +563,10 @@ func (h *Holder) freshAnnouncement(p PullEvent, now time.Time, startedOver strin
|
||||
d := &Delivery{ID: IDOf(p.repository(), p.HeadSHA), Repository: p.repository(), Commit: p.HeadSHA, Number: p.Number,
|
||||
Title: p.Title, Base: p.Base, Branch: p.Head, HTMLURL: p.HTMLURL, CloneURL: p.CloneURL, Paths: p.Paths,
|
||||
PathsTruncated: p.PathsTruncated, Removed: p.Removed, ModuleDirs: p.ModuleDirs, ModuleDirsSaid: p.ModuleDirsSaid,
|
||||
After: AfterIn(p.Body), Issues: IssuesIn(p.Title + "\n" + p.Body), Created: now, BaseSHA: p.BaseSHA}
|
||||
Author: p.Author, Created: now, BaseSHA: p.BaseSHA}
|
||||
if p.BaseSHA != "" {
|
||||
d.BaseSHAAt = now
|
||||
}
|
||||
d.PushedBy = PushedIn(p.Body)
|
||||
why := fmt.Sprintf("%s#%d's head announced", d.Repository, d.Number)
|
||||
if startedOver != "" {
|
||||
why = fmt.Sprintf("%s; %s", why, startedOver)
|
||||
@@ -756,7 +737,7 @@ func (h *Holder) pullMerged(p PullEvent, late bool) {
|
||||
d = &Delivery{ID: IDOf(p.repository(), head), Repository: p.repository(), Commit: head, Number: p.Number,
|
||||
Title: p.Title, Base: p.Base, Branch: p.Head, HTMLURL: p.HTMLURL, CloneURL: p.CloneURL, Paths: p.Paths,
|
||||
PathsTruncated: p.PathsTruncated, Removed: p.Removed, ModuleDirs: p.ModuleDirs, ModuleDirsSaid: p.ModuleDirsSaid,
|
||||
After: AfterIn(p.Body), Issues: IssuesIn(p.Title + "\n" + p.Body), Created: now, FromForge: true}
|
||||
Author: p.Author, Created: now, FromForge: true}
|
||||
// Its check, as the forge holds it on the head: the verdict the pull request merged with.
|
||||
if s := p.HeadChecks[ctxCheck]; s != "" {
|
||||
d.Check = verdictOfCheck(s, "", now)
|
||||
|
||||
@@ -2,8 +2,12 @@ package main
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
seat "git.novox.be/novox/mesh-delivery/contracts/go/seats/meshdelivery"
|
||||
"sort"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -263,13 +267,16 @@ func TestAMergedGroupEndsMergedWithSourceMainMoved(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// ADR 0419 §2, issue 978: a description's `after:` line is not read, and says so once on its pull request. The
|
||||
// composed check is asked as for any other group: no order is asked of the controller first, and none gates it.
|
||||
func TestAnAfterLineIsNotReadAndSaidOnceAndNothingWaitsForAnOrder(t *testing.T) {
|
||||
// W11.5 (novox/hq ADR 0659 §4, ADR 0493 §4): the issues an ask names are the issue tracker's links of each member's pull
|
||||
// request, read through the seat issue-tracker, never the title; a tracker that does not answer is said, and holds nothing;
|
||||
// the author is the forge's pull request record's. ADR 0419 §2's after: notice is gone with the description.
|
||||
func TestTheAskNamesTheIssuesLinkedInTheIssueTracker(t *testing.T) {
|
||||
w := newWorld(t)
|
||||
w.op.auto = false
|
||||
links := &fakeLinks{linked: map[string][]int{"novox/a#1": {948, 12}}, failing: map[string]bool{"novox/b#2": true}}
|
||||
w.h.Links = links
|
||||
pa := pr("novox/a", 1, "a1a1a1a1a1a1a1", "feat/z")
|
||||
pa.Body = "Goes after the other.\n\nafter: novox/b\n"
|
||||
pa.Title, pa.Author = "Fix issue 77", "quiet-deliveries"
|
||||
w.h.PullUpdated(pa)
|
||||
w.h.PullUpdated(pr("novox/b", 2, "b2b2b2b2b2b2b2", "feat/z"))
|
||||
w.h.Checked(verdict("novox/a", 1, "a1a1a1a1a1a1a1", "pass"))
|
||||
@@ -277,29 +284,49 @@ func TestAnAfterLineIsNotReadAndSaidOnceAndNothingWaitsForAnOrder(t *testing.T)
|
||||
w.settleAll()
|
||||
w.later(time.Minute)
|
||||
w.settleAll()
|
||||
said := 0
|
||||
open := w.op.open()
|
||||
if len(open) != 1 {
|
||||
t.Fatalf("the group with an unread tracker is not asked: %v", open)
|
||||
}
|
||||
e := w.op.ask(open[0]).Explanation
|
||||
if !strings.Contains(e, "Issues: issue 12, issue 948, b#2's unread (the issue tracker did not answer)") || strings.Contains(e, "77") {
|
||||
t.Fatalf("the ask's issues are not the links:\n%s", e)
|
||||
}
|
||||
if d := w.delivery(IDOf("novox/a", "a1a1a1a1a1a1a1")); d.Author != "quiet-deliveries" || authorOf(d) != "the author of feat/z (quiet-deliveries)" {
|
||||
t.Fatalf("the author is %q, said %q", d.Author, authorOf(d))
|
||||
}
|
||||
if links.asked["novox/a#1"] != 1 {
|
||||
t.Fatalf("the links of novox/a#1 were read %d times at one head, want once", links.asked["novox/a#1"])
|
||||
}
|
||||
for _, c := range w.forge.comments {
|
||||
if strings.Contains(c, "not read") && strings.Contains(c, "novox/a#1") {
|
||||
said++
|
||||
if strings.Contains(c, "after:") {
|
||||
t.Fatalf("an after: notice is said: %q", c)
|
||||
}
|
||||
}
|
||||
if said != 1 {
|
||||
t.Fatalf("the after: line is not said once on its pull request: %v", w.forge.comments)
|
||||
}
|
||||
|
||||
// fakeLinks is the issue tracker's links of pull requests, by `<repository>#<number>`.
|
||||
type fakeLinks struct {
|
||||
mu sync.Mutex
|
||||
linked map[string][]int
|
||||
failing map[string]bool
|
||||
asked map[string]int
|
||||
}
|
||||
|
||||
func (f *fakeLinks) Linked(repository string, number int) ([]int, error) {
|
||||
f.mu.Lock()
|
||||
defer f.mu.Unlock()
|
||||
key := repository + "#" + strconv.Itoa(number)
|
||||
if f.asked == nil {
|
||||
f.asked = map[string]int{}
|
||||
}
|
||||
// A new head of the same pull request, with the line still there, is not told again.
|
||||
pa2 := pr("novox/a", 1, "a9a9a9a9a9a9a9", "feat/z")
|
||||
pa2.Body = pa.Body
|
||||
w.h.PullUpdated(pa2)
|
||||
w.settleAll()
|
||||
said = 0
|
||||
for _, c := range w.forge.comments {
|
||||
if strings.Contains(c, "not read") {
|
||||
said++
|
||||
}
|
||||
}
|
||||
if said != 1 {
|
||||
t.Fatalf("said again for a new head: %v", w.forge.comments)
|
||||
f.asked[key]++
|
||||
if f.failing[key] {
|
||||
return nil, errors.New("no responders")
|
||||
}
|
||||
out := append([]int(nil), f.linked[key]...)
|
||||
sort.Ints(out)
|
||||
return out, nil
|
||||
}
|
||||
|
||||
// Stalled: a delivery past its state's bound is listed with what H2 may do; close takes only that. Only a proposed
|
||||
|
||||
@@ -18,7 +18,6 @@ func TestClosingThePullRequestOfASharedHeadStartsTheOtherOnesDeliveryOver(t *tes
|
||||
id := IDOf(gCtl, head1114)
|
||||
w.h.PullUpdated(pr(gCtl, 378, head1114, "feat/chain", "internal/a.go"))
|
||||
b := pr(gCtl, 379, head1114, "feat/chain-2", "internal/a.go")
|
||||
b.Body = "Fixes issue 379."
|
||||
w.h.PullUpdated(b) // announced while #378 is live: the same delivery heard again
|
||||
if d := w.delivery(id); d.Number != 378 {
|
||||
t.Fatalf("setup: the record is #%d, wanted #378 still", d.Number)
|
||||
@@ -56,7 +55,6 @@ func TestAnEditOfAPullRequestWhoseSharedHeadsRecordIsStoppedUnderAnotherTakesItO
|
||||
t.Fatalf("setup: %s", d.State)
|
||||
}
|
||||
e := pr(gCtl, 379, head1114, "feat/chain-2")
|
||||
e.Body = "Fixes issue 379."
|
||||
w.h.PullEdited(e)
|
||||
d := w.delivery(id)
|
||||
if d.Number != 379 || d.State != Proposed || len(d.Paths) == 0 {
|
||||
@@ -77,7 +75,6 @@ func TestAMergedRecordIsNeverTakenOverByTheOtherPullRequestOfItsHead(t *testing.
|
||||
w.h.PullMerged(merged(gCtl, 378, head1114, "f378f378f378f378"))
|
||||
w.h.PullClosed(pr(gCtl, 378, head1114, "feat/chain"))
|
||||
e := pr(gCtl, 379, head1114, "feat/chain-2")
|
||||
e.Body = "text"
|
||||
w.h.PullEdited(e)
|
||||
if d := w.delivery(id); d.Number != 378 || d.MergedAs == "" {
|
||||
t.Fatalf("a merged record was taken over: #%d merged as %q", d.Number, d.MergedAs)
|
||||
|
||||
@@ -11,8 +11,6 @@ import (
|
||||
// branch, title, URLs or review record. The ready pass skipped it in silence, so no Approve was ever asked, and the
|
||||
// announcement of the same head, heard later, filled none of it.
|
||||
|
||||
const verdictFirstBody = "Fixes a bug.\n\nReviewed-by: reviewer@shanks at " + readyHead + "\n"
|
||||
|
||||
// verdictFirst is the live shape: the verdict heard, the head not, and the delivery ready.
|
||||
func verdictFirst(t *testing.T) (*world, string) {
|
||||
w := newWorld(t)
|
||||
@@ -48,7 +46,7 @@ func TestADeliveryWithoutItsHeadSaysWhatItWaitsForAndAsksNoOne(t *testing.T) {
|
||||
func TestAnAnnouncementFillsADeliveryMadeFromItsVerdictAndTheOperatorIsAsked(t *testing.T) {
|
||||
w, id := verdictFirst(t)
|
||||
p := pr("novox/app", 419, readyHead, "fix/x", "src/a.go")
|
||||
p.Title, p.Body, p.CloneURL = "Fix x", verdictFirstBody, "https://forge.invalid/novox/app.git"
|
||||
p.Title, p.CloneURL = "Fix x", "https://forge.invalid/novox/app.git"
|
||||
w.h.PullUpdated(p)
|
||||
w.settleAll()
|
||||
d := w.delivery(id)
|
||||
@@ -112,7 +110,7 @@ func TestAnAnnouncementFillingAVerdictFirstHeadSupersedesTheOlderHeadAndItsAsk(t
|
||||
func TestAnEditOfThePullRequestFillsADeliveryMadeFromItsVerdict(t *testing.T) {
|
||||
w, id := verdictFirst(t)
|
||||
p := pr("novox/app", 419, readyHead, "fix/x")
|
||||
p.Title, p.Body = "Fix x", verdictFirstBody
|
||||
p.Title = "Fix x"
|
||||
w.h.PullEdited(p)
|
||||
w.settleAll()
|
||||
d := w.delivery(id)
|
||||
|
||||
@@ -0,0 +1,106 @@
|
||||
package main
|
||||
|
||||
// The issues an ask names, from the issue tracker's links (novox/hq ADR 0493 §4, ADR 0659 §4): a pull request is linked to
|
||||
// the issue it fixes by a tool call of the seat issue-tracker, never matched from its title or description, and the gate
|
||||
// reads those links through the seat's verb `list`. Words only: an ask is never held for them; a read that fails is said in
|
||||
// the ask as the tracker unread, and read again at the pull request's next head.
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"sort"
|
||||
"strconv"
|
||||
)
|
||||
|
||||
// TrackerListVerb is the seat issue-tracker's verb the gate reads a pull request's issues with (the manifest invokes it).
|
||||
const TrackerListVerb = "seat:issue-tracker.list"
|
||||
|
||||
// IssueLinks reads the issues linked to a pull request in the issue tracker.
|
||||
type IssueLinks interface {
|
||||
// Linked is the issues linked to the pull request number of repository (owner/name), ascending.
|
||||
Linked(repository string, number int) ([]int, error)
|
||||
}
|
||||
|
||||
// seatIssues is the issue tracker through its seat.
|
||||
type seatIssues struct{ ask asking }
|
||||
|
||||
func (s seatIssues) Linked(repository string, number int) ([]int, error) {
|
||||
// A pull request is linked as `<repository's name>#<number>`, lower-case, as the tracker keeps it.
|
||||
target := "pull-request:" + repoName(repository) + "#" + strconv.Itoa(number)
|
||||
raw, err := s.ask(TrackerListVerb, map[string]any{"linked": target, "limit": 200})
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
answer, output, ok := answerOf(raw)
|
||||
if !ok {
|
||||
return nil, fmt.Errorf("the seat issue-tracker refused list: %s", lastLine(output))
|
||||
}
|
||||
var listed struct {
|
||||
Rows []struct {
|
||||
Number int `json:"number"`
|
||||
} `json:"rows"`
|
||||
Error string `json:"error"`
|
||||
}
|
||||
if err := json.Unmarshal(answer, &listed); err != nil {
|
||||
return nil, fmt.Errorf("the seat issue-tracker's list is not readable: %s", clip(string(answer), 200))
|
||||
}
|
||||
if listed.Error != "" {
|
||||
return nil, fmt.Errorf("the seat issue-tracker could not list %s: %s", target, listed.Error)
|
||||
}
|
||||
out := make([]int, 0, len(listed.Rows))
|
||||
for _, r := range listed.Rows {
|
||||
out = append(out, r.Number)
|
||||
}
|
||||
sort.Ints(out)
|
||||
return out, nil
|
||||
}
|
||||
|
||||
// issuesToRead is the read of the linked issues of every member not read at its head yet, done outside the lock; nil when
|
||||
// all are read or no issue tracker is reached. Under the lock.
|
||||
func (h *Holder) issuesToRead(members []*Delivery) func() {
|
||||
if h.Links == nil {
|
||||
return nil
|
||||
}
|
||||
type read struct {
|
||||
id, repository, head, why string
|
||||
number int
|
||||
issues []int
|
||||
}
|
||||
var reads []read
|
||||
for _, m := range members {
|
||||
if m.Number != 0 && m.IssuesRead != m.Commit {
|
||||
reads = append(reads, read{id: m.ID, repository: m.Repository, head: m.Commit, number: m.Number})
|
||||
}
|
||||
}
|
||||
if len(reads) == 0 {
|
||||
return nil
|
||||
}
|
||||
for _, m := range members {
|
||||
if m.issuesReading == m.Commit && m.IssuesRead != m.Commit {
|
||||
return func() {} // read already: its end looks again
|
||||
}
|
||||
}
|
||||
for _, m := range members {
|
||||
m.issuesReading = m.Commit
|
||||
}
|
||||
return func() {
|
||||
for i := range reads {
|
||||
r := &reads[i]
|
||||
issues, err := h.Links.Linked(r.repository, r.number)
|
||||
r.issues = issues
|
||||
if err != nil {
|
||||
r.why = err.Error()
|
||||
h.Logf("[mesh-delivery] %s: the issues linked to its pull request cannot be read: %v; its ask says so", r.id, err)
|
||||
}
|
||||
}
|
||||
h.mu.Lock()
|
||||
for _, r := range reads {
|
||||
if d := h.deliveries[r.id]; d != nil && d.Commit == r.head {
|
||||
d.Issues, d.IssuesRead, d.IssuesUnread = r.issues, r.head, r.why
|
||||
h.keep(d)
|
||||
}
|
||||
}
|
||||
h.mu.Unlock()
|
||||
h.lookAt(h.Now(), "an ask whose issues were read from the issue tracker")
|
||||
}
|
||||
}
|
||||
@@ -61,6 +61,7 @@ func main() {
|
||||
Settings: settings,
|
||||
Store: kvStore{},
|
||||
Forge: seatForge{ask: stdio.Ask},
|
||||
Links: seatIssues{ask: stdio.Ask},
|
||||
Emit: func(event string, body any) error { return stdio.Emit(event, body) },
|
||||
Logf: logf,
|
||||
Operator: kvOperator{},
|
||||
|
||||
@@ -144,7 +144,8 @@ func TestItClaimsTheDeliverySeatAndFollowsWhatItHandles(t *testing.T) {
|
||||
// merge state through the one statuses tool the seat does not carry yet (seatForge.mergeState); the forge's writes are the
|
||||
// bridge's (ADR 0437 §8). It uses operator-channel only to say its conditions (ADR 0437 §9, issue 1275): the teller asks
|
||||
// (ADR 0437 §6).
|
||||
want := []string{"seat:git.pull", "seat:git.statuses", "seat:git.open-pulls", "seat:git.contains", "seat:mesh-hand-acts.hand-act"}
|
||||
want := []string{"seat:git.pull", "seat:git.statuses", "seat:git.open-pulls", "seat:git.contains", "seat:mesh-hand-acts.hand-act",
|
||||
TrackerListVerb}
|
||||
if !reflect.DeepEqual(m.Uses, []string{"operator-channel"}) {
|
||||
t.Fatalf("uses %v: operator-channel, for its conditions alone", m.Uses)
|
||||
}
|
||||
|
||||
@@ -3,7 +3,6 @@ package main
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"regexp"
|
||||
"sort"
|
||||
"strings"
|
||||
"time"
|
||||
@@ -32,15 +31,16 @@ type Delivery struct {
|
||||
Removed []string `json:"removed,omitempty"`
|
||||
ModuleDirs []string `json:"module_dirs,omitempty"`
|
||||
ModuleDirsSaid bool `json:"module_dirs_said,omitempty"`
|
||||
// After are the repositories its pull request's description names in `after: <repository>` lines. They are not read
|
||||
// (novox/hq ADR 0419 §2): kept only to say once on the pull request that the line is not read. AfterSaid is whether
|
||||
// that was said, for this pull request.
|
||||
After []string `json:"after,omitempty"`
|
||||
AfterSaid bool `json:"after_said,omitempty"`
|
||||
// Comments is how many comments this owner owed on the pull request at this head, which numbers the next.
|
||||
Comments int `json:"comments,omitempty"`
|
||||
// Issues are the issues its title and body name ("issue 948"), shown to the operator who is asked to approve its merge.
|
||||
Issues []int `json:"issues,omitempty"`
|
||||
// Issues are the issues linked to its pull request in the issue tracker (novox/hq ADR 0493 §4, ADR 0659 §4), shown to the
|
||||
// operator who is asked to approve its merge; IssuesRead is the head they were read at, IssuesUnread why the issue
|
||||
// tracker could not say them. Words only: nothing waits on them.
|
||||
Issues []int `json:"issues,omitempty"`
|
||||
IssuesRead string `json:"issues_read,omitempty"`
|
||||
IssuesUnread string `json:"issues_unread,omitempty"`
|
||||
// issuesReading is a read of them in flight, so a look meanwhile does not ask it again; not kept.
|
||||
issuesReading string
|
||||
// Group is the delivery group it is in: its head branch name. Empty for a lone delivery.
|
||||
Group string `json:"group,omitempty"`
|
||||
|
||||
@@ -99,9 +99,9 @@ type Delivery struct {
|
||||
// Ready is what this owner did with the operator's Approve of its merge into main, and with the merge (novox/hq ADR
|
||||
// 0421, ready.go). The json key is the one the ready mark's record had, so rows written before still read.
|
||||
Ready *ReadyRun `json:"ready,omitempty"`
|
||||
// PushedBy is the session that pushed its head, as its pull request's description names it in a line
|
||||
// `Pushed-by: <who@node>` (playbook 07): named with its branch when a new head is needed (novox/hq ADR 0505 §5).
|
||||
PushedBy string `json:"pushed_by,omitempty"`
|
||||
// Author is who opened its pull request, as the forge's pull request record says (the seat git's events): named with
|
||||
// its branch when a new head is needed (novox/hq ADR 0505 §5, ADR 0659 §4).
|
||||
Author string `json:"author,omitempty"`
|
||||
// FreshWanted is whether a passing verdict that arrived out of date is to have its check asked against the
|
||||
// trunk's head, before the operator is asked (novox/hq ADR 0421 §3, issue 1022). Cleared when it is asked.
|
||||
FreshWanted bool `json:"fresh_wanted,omitempty"`
|
||||
@@ -284,44 +284,19 @@ func sameCommit(a, b string) bool {
|
||||
return len(a) >= 7 && strings.HasPrefix(strings.ToLower(b), strings.ToLower(a))
|
||||
}
|
||||
|
||||
// pushedLine is the line naming the session that pushed a pull request's head: `Pushed-by: <who@node>` (ADR 0505 §5).
|
||||
var pushedLine = regexp.MustCompile(`(?im)^\s*Pushed-by:\s*([A-Za-z0-9_.\-]+(?:@[A-Za-z0-9_.\-]+)?)\s*$`)
|
||||
|
||||
// PushedIn reads it: the last such line, or "" when there is none.
|
||||
func PushedIn(body string) string {
|
||||
who := ""
|
||||
for _, m := range pushedLine.FindAllStringSubmatch(body, -1) {
|
||||
who = m[1]
|
||||
}
|
||||
return who
|
||||
}
|
||||
|
||||
// authorOf is a delivery's author as the operator reads it: "the author of <branch>", and the session that pushed it
|
||||
// when its description names one ("who on node": the router refuses an @ in its words).
|
||||
// authorOf is a delivery's author as the operator reads it: "the author of <branch>", and who opened its pull request as
|
||||
// the forge's record says (the router refuses an @ in its words).
|
||||
func authorOf(d *Delivery) string {
|
||||
who := "the author of " + d.Branch
|
||||
if d.Branch == "" {
|
||||
who = "its author"
|
||||
}
|
||||
if d.PushedBy != "" {
|
||||
who += " (" + strings.Replace(d.PushedBy, "@", " on ", 1) + ")"
|
||||
if d.Author != "" {
|
||||
who += " (" + strings.Replace(d.Author, "@", " on ", 1) + ")"
|
||||
}
|
||||
return who
|
||||
}
|
||||
|
||||
// afterLines are the `after: <repository>` lines of a pull request's description.
|
||||
var afterLines = regexp.MustCompile(`(?im)^\s*after:\s*([A-Za-z0-9_.\-/]+)\s*$`)
|
||||
|
||||
// AfterIn reads them.
|
||||
func AfterIn(body string) []string {
|
||||
var out []string
|
||||
for _, m := range afterLines.FindAllStringSubmatch(body, -1) {
|
||||
out = append(out, strings.TrimSuffix(m[1], ".git"))
|
||||
}
|
||||
sort.Strings(out)
|
||||
return out
|
||||
}
|
||||
|
||||
// Group is a delivery group: two or more deliveries sharing a head branch name, one level only.
|
||||
type Group struct {
|
||||
ID string `json:"id"`
|
||||
|
||||
@@ -39,7 +39,6 @@ func TestADeliveryOnMeshCheckGoesReadyAskedAndMergedOverARealBus(t *testing.T) {
|
||||
Statuses: []string{}, Checks: map[string]HeadCheck{}}}
|
||||
rt.mu.Unlock()
|
||||
announced := pr("novox/app", 8, head, "fix/y", "src/b.go")
|
||||
announced.Body = "Reviewed-by: lead@shanks at " + head
|
||||
b.publish(t, "mesh.seat.git.event.pull.updated", seatUpdated(announced))
|
||||
|
||||
// The checker's verdict at the head, then the forge's word that its bridge set mesh/check.
|
||||
|
||||
@@ -212,9 +212,6 @@ func (h *Holder) readyAsks(now time.Time) []func() {
|
||||
h.Logf("[mesh-delivery] %s is ready (%s); the operator's Approve of its merge into main is asked once it may merge",
|
||||
d.ID, h.checkAt([]*Delivery{d}).Words)
|
||||
}
|
||||
if a := h.sayAfterLine(d); a != nil {
|
||||
asks = append(asks, a)
|
||||
}
|
||||
r := d.Ready
|
||||
if r == nil || r.Merged != nil || r.declined() {
|
||||
continue
|
||||
@@ -315,26 +312,6 @@ func (h *Holder) readyAsks(now time.Time) []func() {
|
||||
return asks
|
||||
}
|
||||
|
||||
// sayAfterLine tells a pull request, once, that its description holds an `after: <repository>` line, which is not read
|
||||
// (novox/hq ADR 0419 §2): the order of a delivery comes from the module graph alone, and no refusal asks for the line.
|
||||
// Once per pull request, not per head. Under the lock; the comment is said outside it.
|
||||
func (h *Holder) sayAfterLine(d *Delivery) func() {
|
||||
if len(d.After) == 0 || d.AfterSaid {
|
||||
return nil
|
||||
}
|
||||
d.AfterSaid = true
|
||||
for _, o := range h.deliveries {
|
||||
if o != d && o.Number == d.Number && o.Repository == d.Repository && o.AfterSaid {
|
||||
h.keep(d)
|
||||
return nil // said on this pull request for an earlier head
|
||||
}
|
||||
}
|
||||
h.keep(d)
|
||||
h.Logf("[mesh-delivery] %s: its description holds an after: line (%s), which is not read", d.ID, strings.Join(d.After, ", "))
|
||||
return h.sayOn(d, "mesh-delivery: this description holds an `after:` line ("+strings.Join(d.After, ", ")+"). It is not read: "+
|
||||
"the order of a delivery comes from the module graph alone, and nothing asks for the line (novox/hq ADR 0419).")
|
||||
}
|
||||
|
||||
// mergeAgainNow lets an approved delivery's merge be asked at once: something that decides whether it may merge happened (a
|
||||
// verdict, an Approve, a group's composed check), so the wait after a refusal does not hold it. Under the lock.
|
||||
func (h *Holder) mergeAgainNow(d *Delivery) {
|
||||
|
||||
@@ -294,7 +294,8 @@
|
||||
"seat:git.statuses",
|
||||
"seat:git.open-pulls",
|
||||
"seat:git.contains",
|
||||
"seat:mesh-hand-acts.hand-act"
|
||||
"seat:mesh-hand-acts.hand-act",
|
||||
"seat:issue-tracker.list"
|
||||
],
|
||||
"state": [
|
||||
"deliveries",
|
||||
|
||||
Reference in New Issue
Block a user