Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
0cf7b2344f |
@@ -12,7 +12,7 @@ import { mergeableUnder, newestByContext } from "./checks.js";
|
||||
import { MERGE_METHODS, Refused, checkName } from "./guard.js";
|
||||
import { CHECK_CONTEXT, REPO_CHECK_CONTEXT } from "./pulls.js";
|
||||
import { approvedHeadsOf, groupRefusal, memberKey, namesSiblings, notApproved, readSiblings, siblingOutOfDate, siblingsFile } from "./siblings.js";
|
||||
import { announcementOf, askedWords, outOfDateWords, sameCommit, tookFile, tookWords, twelve, verdictAtTrunk, waitsForComposed } from "./trunk.js";
|
||||
import { announcementOf, askedWords, outOfDateWords, sameCommit, tookFile, tookWords, twelve, verdictAtTrunk } from "./trunk.js";
|
||||
|
||||
/** A head named for a merge: at least 12 hex characters of its sha (novox/hq issue 381), up to a sha-256 commit's 64. */
|
||||
const REVIEWED_HEAD = /^[0-9a-f]{12,64}$/;
|
||||
@@ -117,14 +117,6 @@ export function mergeTool(gitea: GiteaClient, given: MergeDeps): ToolDefinition
|
||||
: at.judged ? `was judged against trunk ${twelve(at.judged)}` : "names no trunk commit it was judged against";
|
||||
const took = tookWords(tookFile(deps.stateDir), `${owner}/${repo}`);
|
||||
const head = twelve(pull.head_sha), now = twelve(trunkHead);
|
||||
// **A member that waits for its group's composed check is not announced again** (novox/hq issue 1106): its own
|
||||
// check can only say "composed" once more, so an ask here brought a verdict naming no trunk and the next merge was
|
||||
// refused the same way for ever. The composed check is mesh-delivery's to ask; this says whose verdict is awaited.
|
||||
if (waitsForComposed(checks[CHECK_CONTEXT])) {
|
||||
throw new Refused(`${owner}/${repo}#${number} is not merged: its merge gate ${checks[CHECK_CONTEXT]?.description}; the ` +
|
||||
`verdict that names its trunk commit is its group's composed check's, and mesh-delivery asks it, this tool asks no check ` +
|
||||
`(main is now ${now}). Ask again once the group's composed check has passed. Nothing was merged`);
|
||||
}
|
||||
if (at.onItsWay) {
|
||||
throw new Refused(`${owner}/${repo}#${number} is not merged: its verdict ${judged}, and ${pull.base} is now ` +
|
||||
`${now}. Its check is already asked and not yet answered (${checks[CHECK_CONTEXT]?.description}), so it is not ` +
|
||||
|
||||
@@ -226,10 +226,6 @@ function repoWrong(repo: Layer | undefined, facts: RepoCheckFacts): boolean {
|
||||
return !!repo && repoStatus(repo, facts).state !== "success";
|
||||
}
|
||||
|
||||
/** What a group member's gate says while the group's composed check is the one that judges it: the member's own check can say
|
||||
* nothing more of it, so asking that check again brings no verdict that names a trunk (novox/hq issue 1106). */
|
||||
export const WAITS_FOR_COMPOSED = "waits for its group's composed check";
|
||||
|
||||
/** The forge's status for the gate. */
|
||||
export function statusFor(c: Checked): CommitStatus {
|
||||
const gate = c.gate ?? { verdict: c.verdict, summary: c.summary };
|
||||
@@ -264,7 +260,7 @@ export function statusesFor(c: Checked, facts: RepoCheckFacts = {}, gateNow?: st
|
||||
// set. The gate stays pending, saying it waits for the group, unless a composed verdict already stands on it.
|
||||
const out: CommitStatus[] = [];
|
||||
if (!(gateNow ?? "").includes(" composed: ")) {
|
||||
out.push({ state: "pending", context: CHECK_CONTEXT, description: WAITS_FOR_COMPOSED });
|
||||
out.push({ state: "pending", context: CHECK_CONTEXT, description: "waits for its group's composed check" });
|
||||
}
|
||||
const repo = c["repo-check"];
|
||||
if (repo) out.push(judgedFirst(c, repoStatus(repo, facts)));
|
||||
|
||||
@@ -339,30 +339,6 @@ test("gitea_merge_pull_request refuses a verdict judged against an earlier trunk
|
||||
assert.equal(state.emitted.length, 4, "a merge asks no check");
|
||||
});
|
||||
|
||||
// **A group member whose gate waits for its group's composed check is not announced again by the merge tool** (novox/hq issue
|
||||
// 1106): the member's own check can only say "composed" again, so each refusal's ask brought a verdict that names no trunk and
|
||||
// the next merge was refused the same way, for ever. The tool says whose verdict is awaited and asks nothing.
|
||||
test("gitea_merge_pull_request asks no check of a member that waits for its group's composed check", async () => {
|
||||
const merge = () => tool("gitea_merge_pull_request").run({ owner: "novox", repo: "mesh-catalog", number: 7, head: REVIEWED });
|
||||
state.trunk = TRUNK;
|
||||
const before = state.emitted.length;
|
||||
state.statuses = [{ context: "mesh/merge-gate", state: "pending", description: "waits for its group's composed check" },
|
||||
{ context: "mesh/repo-check", state: "success", description: "pass: y" }];
|
||||
for (let i = 0; i < 3; i++) {
|
||||
await assert.rejects(merge(), (err: Error) => {
|
||||
assert.match(err.message, /waits for its group's composed check/);
|
||||
assert.match(err.message, /mesh-delivery asks it, this tool asks no check/);
|
||||
assert.match(err.message, /Nothing was merged/);
|
||||
return true;
|
||||
});
|
||||
}
|
||||
assert.equal(state.emitted.length, before, "no check is announced again and again");
|
||||
assert.equal(state.merged, false);
|
||||
// Once the composed verdict names the member's unmoved trunk, it merges.
|
||||
state.statuses = judgedAt(TRUNK);
|
||||
assert.equal(((await merge()) as Record<string, unknown>).merged, true);
|
||||
});
|
||||
|
||||
// A merge lands only the head that was reviewed (novox/hq issue 381): mesh-controller#197 was reviewed at one head,
|
||||
// force-pushed, checked green at the new one, and merged there, because the tool merged whatever the head was.
|
||||
test("gitea_merge_pull_request merges only the head it is named, and refuses one that is not the head", async () => {
|
||||
|
||||
@@ -17,7 +17,7 @@
|
||||
import { existsSync, mkdirSync, readFileSync, renameSync, writeFileSync } from "node:fs";
|
||||
import { dirname } from "node:path";
|
||||
import type { CommitStatus, GiteaClient, GiteaPull, HeadCheck } from "./client.js";
|
||||
import { CHECK_CONTEXT, REPO_CHECK_CONTEXT, WAITS_FOR_COMPOSED } from "./pulls.js";
|
||||
import { CHECK_CONTEXT, REPO_CHECK_CONTEXT } from "./pulls.js";
|
||||
|
||||
/** A commit as a verdict names it: its first twelve characters. */
|
||||
export function twelve(sha: string | undefined): string {
|
||||
@@ -58,11 +58,6 @@ export function checkOnItsWay(gate: HeadCheck | undefined): boolean {
|
||||
return d.startsWith("waiting in the build queue") || d.startsWith("building on ") || d.startsWith("checking again");
|
||||
}
|
||||
|
||||
/** Whether a gate says it waits for its group's composed check. */
|
||||
export function waitsForComposed(gate: HeadCheck | undefined): boolean {
|
||||
return !!gate && gate.state === "pending" && (gate.description ?? "").startsWith(WAITS_FOR_COMPOSED);
|
||||
}
|
||||
|
||||
/**
|
||||
* The statuses a trunk's move puts back to pending on one open pull request's head, from what the forge holds on it
|
||||
* now: none when it has no merge check yet, when a check is pending (on its way, or already out of date — its
|
||||
|
||||
@@ -634,7 +634,6 @@ func (h *Holder) answered(key string, ans sdk.Answer) []func() {
|
||||
for _, d := range covered {
|
||||
r := d.Ready
|
||||
r.Warrant = ans.Words
|
||||
r.Repeats = 0 // the operator chose: whatever the tool refused before is asked afresh (issue 1106)
|
||||
h.mergeAgainNow(d) // the operator chose: the merge is asked at once
|
||||
switch ans.Option {
|
||||
case OptionApprove:
|
||||
|
||||
@@ -173,43 +173,3 @@ func TestAGroupMergedWholeMovesSourceMainOnceAndAnnouncesWithTheTeller(t *testin
|
||||
t.Fatalf("source.main moved again: %d", len(w.source.moves))
|
||||
}
|
||||
}
|
||||
|
||||
// 1106: the same refusal again and again with the trunk not moved is not asked on without end. Three refusals in a row with
|
||||
// the same cause end the asking by a verdict, a status or a composed check, and say it urgent; a trunk's move, or the
|
||||
// operator's Approve, starts it again.
|
||||
func TestARefusalThatRepeatsWithNoTrunkMoveIsNotAskedAgainWithoutEnd(t *testing.T) {
|
||||
w := answeredFirst(t)
|
||||
d := w.h.deliveries[IDOf(gCat, gHk)]
|
||||
refuse := func(took string) {
|
||||
w.h.ForgeRefused(sdk.Forge{Kind: sdk.ForgeRefused, Repository: gCat, Number: 2, Head: gHk, Act: "merge",
|
||||
Words: "novox/mesh-catalog#2 is not merged: its verdict names no trunk commit it was judged against, and main is now f70a99dbf485. " +
|
||||
"The check of head d2d2d2d2d2d2 merged into trunk f70a99dbf485 was asked; the last check took " + took + ". Ask again once both statuses pass"})
|
||||
}
|
||||
tried := w.now
|
||||
for i, took := range []string{"4m1s", "4m9s"} {
|
||||
refuse(took)
|
||||
d.Ready.LastTry = tried
|
||||
w.h.mergeAgainNow(d)
|
||||
if !d.Ready.LastTry.IsZero() {
|
||||
t.Fatalf("refusal %d: the merge was not asked again at once on a verdict, as after a first refusal", i+1)
|
||||
}
|
||||
}
|
||||
refuse("4m5s") // the third in a row, with only the check's duration changed
|
||||
if d.Ready.Repeats != 3 {
|
||||
t.Fatalf("%d refusals counted for the same cause, wanted 3", d.Ready.Repeats)
|
||||
}
|
||||
d.Ready.LastTry = tried
|
||||
w.h.mergeAgainNow(d) // the composed check or a verdict answers: nothing is asked
|
||||
if d.Ready.LastTry.IsZero() {
|
||||
t.Fatal("the merge was asked again after the same refusal three times with the trunk not moved")
|
||||
}
|
||||
if lines := urgentLines(w); len(lines) != 1 || !strings.Contains(lines[0].Says, "names no trunk commit") {
|
||||
t.Fatalf("the repeated refusal is not said urgent: %+v", lines)
|
||||
}
|
||||
// The trunk moves: the merge is asked again.
|
||||
d.TrunkNow = "a1a1a1a1a1a1a1a1"
|
||||
w.h.mergeAgainNow(d)
|
||||
if !d.Ready.LastTry.IsZero() {
|
||||
t.Fatal("a trunk's move did not let the merge be asked again")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -287,20 +287,9 @@ func (h *Holder) ForgeRefused(f sdk.Forge) {
|
||||
if r.RefusedSince == nil {
|
||||
r.RefusedSince = &now
|
||||
}
|
||||
r.noteRefusal(d, answer)
|
||||
if !outOfDateRefusal(answer) {
|
||||
r.Urgent = true // never a warning again for this head
|
||||
}
|
||||
if r.Repeats >= maxRepeatedRefusals {
|
||||
// Loud, not endless (novox/hq issue 1106): the same refusal again with the trunk not moved is no trunk's move, so
|
||||
// the merge is not asked again by a verdict or a status; the operator is told, urgent, and decides.
|
||||
r.Urgent = true
|
||||
if r.Repeats == maxRepeatedRefusals {
|
||||
h.Logf("[mesh-delivery] %s: the merge tool refused it %d times in a row with the same words and the trunk not moved, so "+
|
||||
"the merge is asked no more until the trunk moves, a new head is announced or the operator approves again: %s",
|
||||
d.ID, r.Repeats, answer)
|
||||
}
|
||||
}
|
||||
if answer != r.Answer {
|
||||
h.Logf("[mesh-delivery] %s is approved, and the merge tool did not merge it (asked again at the next verdict, Approve or composed check): %s", d.ID,
|
||||
answer)
|
||||
|
||||
@@ -85,11 +85,6 @@ type ReadyRun struct {
|
||||
// Urgent is set once a refusal of this head was one that is urgent at once: its severity never steps down to a
|
||||
// warning after (issue 1275, AC6).
|
||||
Urgent bool `json:"urgent,omitempty"`
|
||||
// Repeats is how many times in a row the merge tool refused with the same words while the trunk stayed where RepeatTrunk
|
||||
// says: past maxRepeatedRefusals the merge is asked no more by a verdict, a status or a composed check, and the refusal
|
||||
// is urgent (novox/hq issue 1106). A trunk's move, a new head or the operator's Approve starts the count again.
|
||||
Repeats int `json:"repeats,omitempty"`
|
||||
RepeatTrunk string `json:"repeat_trunk,omitempty"`
|
||||
// RefusedSince is when the merge tool began refusing it without a break.
|
||||
RefusedSince *time.Time `json:"refused_since,omitempty"`
|
||||
// Merged is when the merge tool merged it.
|
||||
@@ -327,43 +322,10 @@ func (h *Holder) askFresh(d *Delivery, now time.Time) func() {
|
||||
// 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) {
|
||||
if d != nil && d.Ready != nil && d.Ready.Merged == nil {
|
||||
if d.Ready.repeatedRefusal(d) {
|
||||
return // the same refusal again and again with the trunk where it was: asking more changes nothing (issue 1106)
|
||||
}
|
||||
d.Ready.LastTry = time.Time{}
|
||||
}
|
||||
}
|
||||
|
||||
// maxRepeatedRefusals is how many refusals in a row, with the same words and the trunk not moved, end the asking again: a
|
||||
// refusal that repeats with no trunk move is not a trunk's move, and the check asked after each only brings the same words.
|
||||
const maxRepeatedRefusals = 3
|
||||
|
||||
// repeatedRefusal is whether the merge was refused maxRepeatedRefusals times with these words and the trunk where it is
|
||||
// now: it is then asked no more until something changes (a trunk's move, a new head, the operator's Approve).
|
||||
func (r *ReadyRun) repeatedRefusal(d *Delivery) bool {
|
||||
return r.Repeats >= maxRepeatedRefusals && r.RepeatTrunk == d.TrunkNow
|
||||
}
|
||||
|
||||
// refusalKey is a refusal's first sentence, up to the trunk it names ("... and main is now <t>"): what follows says how long the
|
||||
// last check took and the like, which differs each time and is no change of cause.
|
||||
func refusalKey(answer string) string {
|
||||
if i := strings.Index(answer, " is now "); i >= 0 {
|
||||
if j := strings.Index(answer[i:], ". "); j >= 0 {
|
||||
return answer[:i+j]
|
||||
}
|
||||
}
|
||||
return answer
|
||||
}
|
||||
|
||||
// noteRefusal counts a refusal: the same words with the trunk where it was count on, anything else starts the count at one.
|
||||
func (r *ReadyRun) noteRefusal(d *Delivery, answer string) {
|
||||
if refusalKey(answer) == refusalKey(r.Answer) && r.RepeatTrunk == d.TrunkNow && r.Repeats > 0 {
|
||||
r.Repeats++
|
||||
return
|
||||
}
|
||||
r.Repeats, r.RepeatTrunk = 1, d.TrunkNow
|
||||
}
|
||||
|
||||
// readyWaits is what a delivery's merge waits on; empty when the Approve may be asked and the merge asked. A ready
|
||||
// delivery is asked. So is a proposed one whose verdict went out of date, when an Approve was asked for it already (it
|
||||
// stays through a trunk move, ADR 0421 §3) and its check against the trunk's head is not on its way; one without an
|
||||
|
||||
@@ -703,3 +703,23 @@ func TestNatsResticsFilesFillFromTheBackupPlacement(t *testing.T) {
|
||||
b.assign(t, "restic", plan("# a\n/srv/a\n", "# a\nstate\n"))
|
||||
eventually(t, "a module taken away refills backups.conf", func() bool { return e.read(t, backups) == head+"# a\n/srv/a\n" })
|
||||
}
|
||||
|
||||
// defaults writes the defaults a module's manifest declares as mesh-config resolves them (source default), for a
|
||||
// moved module whose templates read config values (issue 1780).
|
||||
func (b *aBus) defaults(t *testing.T, module string, manifest []byte) {
|
||||
t.Helper()
|
||||
var m struct {
|
||||
Config map[string]struct {
|
||||
Default json.RawMessage `json:"default"`
|
||||
} `json:"config"`
|
||||
}
|
||||
if err := json.Unmarshal(manifest, &m); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for key, d := range m.Config {
|
||||
if len(d.Default) > 0 && string(d.Default) != "null" {
|
||||
b.put(t, ConfigBucket, node+"."+module+"."+key, fmt.Sprintf(`{"node":%q,"module":%q,"key":%q,"value":%s,`+
|
||||
`"source":"default"}`, node, module, key, d.Default))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -158,6 +158,7 @@ func TestNatsTheEnvironmentOfTheCataloguesModulesIsWhatTheOldFormComposed(t *tes
|
||||
if !manifest.Moved || len(manifest.Items) == 0 {
|
||||
t.Fatalf("%s is not a moved contributor of items", m)
|
||||
}
|
||||
b.defaults(t, m, read(m+"/module.json"))
|
||||
texts := map[string]string{}
|
||||
var planned []PlannedItem
|
||||
for _, it := range manifest.Items {
|
||||
|
||||
@@ -63,6 +63,7 @@ func TestNatsTheSessionFilesFilledFromTheirContributorsItemsAreWhatTheOldFormCom
|
||||
if !c.Moved || len(c.Shell) != 0 {
|
||||
t.Fatalf("%s is not moved or still holds shell entries", m)
|
||||
}
|
||||
b.defaults(t, m, []byte(read(m+"/module.json")))
|
||||
texts := map[string]string{}
|
||||
var planned []PlannedItem
|
||||
for _, it := range c.Items {
|
||||
|
||||
Reference in New Issue
Block a user