chadtmiller.com/knotless
git that lives in your Atmosphere account
1
10
The queue says when a pull's pre-flight failed, rather than that it runs
.changeset/queue-failed-preflight.mdnew+10−0
| @@ -0,0 +1,10 @@ | ||
--- | ||
default: patch | ||
--- | ||
| ||
# The queue says when a pull's pre-flight failed | ||
| ||
`knotless queue` lists a queued pull whose pre-flight failed as `failed | ||
<workflows>`, and `show` says which failed, rather than `queued` and | ||
"running its checks again" while nothing runs, until the runner ejects it | ||
or runs it again. | ||
crates/knotless/src/cli.rs+1−5
| @@ -2311,11 +2311,7 @@ async fn command( | ||
p.position.to_string(), | ||
format!("{} at {}", b.state, abbrev(&b.commit)), | ||
), | ||
(Some(p), None) if !w.preflight.is_empty() => ( | ||
p.position.to_string(), | ||
format!("pre-flight {}", w.preflight.join(",")), | ||
), | ||
(Some(p), None) => (p.position.to_string(), "queued".to_string()), | ||
(Some(p), None) => (p.position.to_string(), w.waits_for()), | ||
(None, _) => ("-".to_string(), "ejected".to_string()), | ||
}; | ||
writeln!(out, "{at}\t{base}\t{state}\t{}\t{}", i.uri, i.title)?; | ||
crates/knotless/src/queue.rs+51−6
| @@ -427,6 +427,10 @@ pub mod stage { | ||
/// One of its pre-flights failed on an older tip of its branch, and | ||
/// runs again at the same head on the tip as it is now. | ||
pub const AGAIN: &str = "again"; | ||
/// One of its pre-flights failed and none runs: the runner ejects it | ||
/// once it sees the failure, or, if that was on an older tip, runs it | ||
/// again. Which, the runs alone do not say. | ||
pub const FAILED: &str = "failed"; | ||
/// Its pre-flights passed: it goes into the next batch. | ||
pub const READY: &str = "ready"; | ||
/// Its pre-flights passed, and it waits for a review of its head from | ||
| @@ -439,6 +443,28 @@ pub mod stage { | ||
} | ||
| ||
impl Waiting { | ||
/// What it waits for, queued in no batch, as `knotless queue` lists | ||
/// it: `failed test` for a pre-flight that failed, `pre-flight | ||
/// check,test` for those running, or else `queued`. | ||
pub fn waits_for(&self) -> String { | ||
match self.stage == stage::FAILED { | ||
true => format!("failed {}", self.failed().join(",")), | ||
false if !self.preflight.is_empty() => { | ||
format!("pre-flight {}", self.preflight.join(",")) | ||
} | ||
false => "queued".to_string(), | ||
} | ||
} | ||
| ||
/// The pre-flights whose newest run failed, by workflow. | ||
pub fn failed(&self) -> Vec<&str> { | ||
let failed = [status::FAILURE, status::ERROR]; | ||
(self.preflights.iter()) | ||
.filter(|c| failed.contains(&c.status.as_str())) | ||
.map(|c| c.workflow.as_str()) | ||
.collect() | ||
} | ||
| ||
/// Where it stands, in a few words: `Queued, 3rd of 5`, `Queued, 1st | ||
/// of 2, running its checks`, `Queued, 2nd of 2, ready for the next | ||
/// batch`, `In a batch of 4 being checked`, `Landing`, or why it was | ||
| @@ -461,6 +487,11 @@ impl Waiting { | ||
"{place}, running its checks again on {} as it is now", | ||
p.base | ||
), | ||
stage::FAILED => format!( | ||
"{place}, but its {} failed. The runner ejects it, or runs it again if that run was on an older {}", | ||
self.failed().join(" and "), | ||
p.base | ||
), | ||
stage::READY => format!("{place}, ready for the next batch"), | ||
stage::REVIEW => format!("{place}, waiting for its review"), | ||
_ => place, | ||
| @@ -559,9 +590,11 @@ fn failed(c: &Check) -> bool { | ||
} | ||
| ||
/// Where a pull queued in no batch stands, one of [`stage`], by `newest`, | ||
/// its pre-flights' newest runs, among `all` its pre-flight runs. A newest | ||
/// that failed while it is still queued failed on an older tip, as one on | ||
/// the tip ejects it: the runner runs it again. | ||
/// its pre-flights' newest runs, among `all` its pre-flight runs. One | ||
/// running at a head and base that failed before is running again; a | ||
/// newest that failed with none running is [`stage::FAILED`], never | ||
/// said to be running, as nothing is until the runner ejects it or runs | ||
/// it again. | ||
fn stage_of(newest: &[&Check], all: &[&Check]) -> &'static str { | ||
let going: Vec<&&Check> = (newest.iter()) | ||
.filter(|c| status::going(&c.record.status)) | ||
| @@ -574,8 +607,10 @@ fn stage_of(newest: &[&Check], all: &[&Check]) -> &'static str { | ||
}; | ||
if newest.is_empty() { | ||
stage::QUEUED | ||
} else if going.iter().any(again) || (going.is_empty() && newest.iter().any(|c| failed(c))) { | ||
} else if going.iter().any(again) { | ||
stage::AGAIN | ||
} else if going.is_empty() && newest.iter().any(|c| failed(c)) { | ||
stage::FAILED | ||
} else if !going.is_empty() { | ||
stage::PREFLIGHT | ||
} else if newest.iter().all(|c| c.record.status == status::SUCCESS) { | ||
| @@ -1321,7 +1356,7 @@ mod tests { | ||
[ | ||
("a", "checking", None), | ||
("b", "preflight", ahead), | ||
("c", "again", ahead), | ||
("c", "failed", ahead), | ||
("d", "again", ahead), | ||
("e", "queued", ahead), | ||
("f", "ready", ahead), | ||
| @@ -1330,13 +1365,23 @@ mod tests { | ||
("x", "", None), | ||
] | ||
); | ||
let waits: Vec<String> = w[1..5].iter().map(Waiting::waits_for).collect(); | ||
assert_eq!( | ||
waits, | ||
[ | ||
"pre-flight test", | ||
"failed test", | ||
"pre-flight test", | ||
"queued" | ||
] | ||
); | ||
let said: Vec<String> = w.iter().take(6).map(Waiting::said).collect(); | ||
assert_eq!( | ||
said, | ||
[ | ||
"In a batch of its own being checked", | ||
"Queued, 2nd of 8, running its checks", | ||
"Queued, 3rd of 8, running its checks again on main as it is now", | ||
"Queued, 3rd of 8, but its test failed. The runner ejects it, or runs it again if that run was on an older main", | ||
"Queued, 4th of 8, running its checks again on main as it is now", | ||
"Queued, 5th of 8", | ||
"Queued, 6th of 8, ready for the next batch", | ||
docs/design/merge-queue.md+2−1
| @@ -491,7 +491,8 @@ batch is read from its own pre-flight runs, the newest of each workflow: | ||
| --- | --- | --- | | ||
| `queued` | `Queued, 2nd of 5` | no pre-flight run of it yet, or the newest was cancelled | | ||
| `preflight` | `Queued, 2nd of 5, running its checks` | one is still running | | ||
| `again` | `Queued, 2nd of 5, running its checks again on main as it is now` | one failed at that head before, which while it is still queued was on an older tip, so the runner runs it again | | ||
| `again` | `Queued, 2nd of 5, running its checks again on main as it is now` | one is running at a head that failed before, on an older tip | | ||
| `failed` | `Queued, 2nd of 5, but its test failed. The runner ejects it, or runs it again if that run was on an older main` | the newest of one failed and none is running: the runner has yet to eject it or run it again, and the runs do not say which it will | | ||
| `ready` | `Queued, 2nd of 5, ready for the next batch` | all passed | | ||
| `review` | `Queued, 2nd of 5, waiting for its review` | all passed, and a gate has not reviewed its head | | ||
| ||
docs/reference/cli.md+1−1
| @@ -174,7 +174,7 @@ What they print: | ||
| `merge` | the commit the branch is now at, then `deleted <branch>` and a `closed <issue>` line per issue it closed; queued instead, `<base> takes pulls through the merge queue: queued, <n> of <of>`, then where to follow it | | ||
| `merge --queue` | `queued into <base>: <n> of <of>`; queued again at the head the queue last ejected, a line saying so and why; then where to follow it, and to read an ejection's logs before queueing again | | ||
| `merge --check` | `<state>`, `<method>` and `checks <checks>`, and `reviews <passed or blocked>` when its branch asks for reviews, tab-separated, then a `missing <what>` line for each thing its reviews lack (`reviews 1 of 2`, `owners of web/: @alice or @bob`, `changes asked by @carol`, `a review of its head from @bot`; owners who are the pull's author alone add `, its author alone, whose own approval never counts, so web/ needs another owner in CODEOWNERS`), an `uncounted <why>` line for each approval that does not count (`@dan's approval does not count: only the owner's and collaborators' approvals count`), a `warning <what>` line for each CODEOWNERS line skipped, a `conflict <path>` line per file that conflicts, `deletes <branch>` for the branch it would delete, `may <what>`, `queue only` when its branch takes pulls only through the merge queue, and last why it cannot merge, in the words `merge` would refuse with, and that the caller may not merge it when it may not | | ||
| `queue` | a line per pull: `<position>`, `<base>`, `<state>`, `<uri>`, `<title>`, tab-separated, where the state is `queued`, `pre-flight <workflows>` while the [pre-flights](workflows.md#the-merge-queue) it waits for run, comma-separated, or `<checking or landing> at <commit>`; an ejected pull has `-` and `ejected`, and a line after it with the reason; `the merge queue is empty` if it is | | ||
| `queue` | a line per pull: `<position>`, `<base>`, `<state>`, `<uri>`, `<title>`, tab-separated, where the state is `queued`, `pre-flight <workflows>` while the [pre-flights](workflows.md#the-merge-queue) it waits for run, comma-separated, `failed <workflows>` once the newest run of one failed and none runs, as the runner has yet to eject it or run it again, or `<checking or landing> at <commit>`; an ejected pull has `-` and `ejected`, and a line after it with the reason; `the merge queue is empty` if it is | | ||
| `queue --json` | each pull waiting, with its `place` and `batch`, or the `preflight` workflows it waits for | | ||
| `queue --leave` | `left the merge queue` | | ||
| `edit`, `delete-comment`, `close`, `reopen`, `retarget` | `edited`, `deleted`, `closed`, `open`, `retargeted` | | ||
docs/reference/mcp.md+3−1
| @@ -130,7 +130,9 @@ reviews](../design/collaboration.md#required-reviews)). | ||
| ||
`standing.said` reads as `Queued, 3rd of 5`, `Queued, 1st of 2, running | ||
its checks`, `Queued, 1st of 2, running its checks again on main as it is | ||
now`, `Queued, 2nd of 2, ready for the next batch`, `In a batch of 4 being | ||
now`, `Queued, 1st of 2, but its test failed. The runner ejects it, or | ||
runs it again if that run was on an older main`, `Queued, 2nd of 2, ready | ||
for the next batch`, `In a batch of 4 being | ||
checked`, `Landing`, or `Ejected: <reason>`, with the failed checks. | ||
| ||
## Long files | ||
web/src/lib/format.ts+1−1
| @@ -94,7 +94,7 @@ export function ago(iso: string, before = ''): string { | ||
* whichever is later. Null while landing, or with nothing to go by. | ||
*/ | ||
export function landsAt(q: { stage?: string; checks?: { status: string; startedAt: string }[] }, typical: { batchSeconds: number; preflightSeconds: number } | undefined, batchStartedAt: string | undefined, aheadStartedAt: string | undefined, now: number): number | null { | ||
if (!typical || !q.stage || q.stage === 'landing') return null; | ||
if (!typical || !q.stage || q.stage === 'landing' || q.stage === 'failed') return null; | ||
const batch = typical.batchSeconds * 1000; | ||
if (q.stage === 'checking') return batchStartedAt ? Math.max(Date.parse(batchStartedAt) + batch, now) : null; | ||
const going = (q.checks ?? []).filter((c) => c.status === 'running' || c.status === 'queued').map((c) => Date.parse(c.startedAt)); | ||