From 33305d3391f4fe51f0cc0789d6d8c93caa34fe71 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 7 Nov 2026 12:42:35 -0800 Subject: [PATCH 1/2] chore(quest): claim quest/m0/queue-watcher/flaky-quest.md Quest-Claim: 0cabe110835968f9 From dad0aa06126f245b6b59639e75a9c7be0507dfcb Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 7 Nov 2026 12:46:49 -0800 Subject: [PATCH 2/2] feat(land): the watcher writes flaky tests' follow-up quests quest land watch picks each flaky test's follow-up quest path at its first verdict and lists it in the fixer's known-flaky input for the skip to name. The fixer's disable commit now holds only the skip: the watcher refuses one that touches quest/, then commits the quest from a fixed template on top, with the flaky epic's README when the base has none. No headless agent writes quest prose. Co-Authored-By: Claude Opus 5.5 --- README.md | 14 +-- assets/land-fix.md | 25 ++--- quest/m0/queue-watcher/README.md | 1 - quest/m0/queue-watcher/flaky-quest.md | 43 -------- quest/m0/token-waste/child-lists.md | 3 - src/cli/land-fix.ts | 3 + src/cli/land-watch-flaky.ts | 136 +++++++++++++++++++++++--- test/land-watch.test.ts | 114 +++++++++++++++------ test/land.test.ts | 6 +- 9 files changed, 230 insertions(+), 115 deletions(-) delete mode 100644 quest/m0/queue-watcher/flaky-quest.md diff --git a/README.md b/README.md index 497b7c6e..63cb23b0 100644 --- a/README.md +++ b/README.md @@ -932,13 +932,15 @@ requeue is a verdict: a failure on the base or from the runner never counts toward disabling a test. Each failed build's round is told the tests that flaked before and are not yet disabled, in `.git/quest-land-flaky.json` in its checkout, which `quest land fix` passes to -the agent. One verdict alone only re-enqueues the PR, since one judgment could +the agent, with the follow-up quest path the watcher chose for each when it +first flaked. One verdict alone only re-enqueues the PR, since one judgment could hide a real bug. When a listed test flakes again, the fixer also commits its -disabling on a `disable` branch from the base: the skip in the test -framework's own form, naming the follow-up quest it adds to the repository's -m0 `flaky` epic, `quest/m0/flaky/.md`, to fix or delete the test. The -watcher validates that commit as it does a base fix, requiring exactly one new -follow-up quest, pushes it to `quest-land/flaky-`, opens it as a pull +disabling on a `disable` branch from the base: only the skip in the test +framework's own form, naming that follow-up quest. The watcher validates that +commit as it does a base fix, refusing any edit under `quest/`, then commits +the follow-up quest on top from a fixed template, `quest/m0/flaky/.md` +in the repository's m0 `flaky` epic, to fix or delete the test, creating the +epic's README when the base has none. It pushes the result to `quest-land/flaky-`, opens it as a pull request with its Review recorded, and enqueues it like any other, with no maintainer's pick. A refused disable waits for the test's next flake, and a pull request GitHub refuses to enqueue says so and waits for a maintainer. diff --git a/assets/land-fix.md b/assets/land-fix.md index 8e4066bf..e150881e 100644 --- a/assets/land-fix.md +++ b/assets/land-fix.md @@ -157,24 +157,17 @@ A failure on the base or from the runner is not flaky: name its cause, never `disable`. A PR requeued only because another supplied PR caused the failure takes no `cause`. When you cannot tell why the failure happened, hand the PR back. The input may list the repository's known flaky tests, each by the -`failure` an earlier round named and the `merge_group` jobs that showed it. +`failure` an earlier round named, the `merge_group` jobs that showed it, and +the `quest` path of the follow-up quest that tracks it once disabled. When the test you judge flaky is one of them, name it exactly as listed, and disable it in a commit of its own: create a `disable` branch from `base`, and -commit on it only - -- the skip, in the test framework's own form, whose reason names the follow-up - quest, such as `#[ignore = "flaky: /quest/m0/flaky/.md"]` in Rust, - `test.skip` with that path in a comment in JavaScript, or - `pytest.mark.skip(reason="flaky: /quest/m0/flaky/.md")` in Python; -- that follow-up quest, `quest/m0/flaky/.md`, with `` the test's - name in lowercase kebab-case, whose Goal says to fix the test or delete it - and links each listed job and this failure's, which the failure log names; -- its link in the `Required` list of `quest/m0/flaky/README.md`, the epic - holding one quest per disabled test, created with a link in - `quest/m0/README.md`'s `Required` if it is missing. - -Follow the repository's quest format, and run `quest check` when the -repository has it. A disabled test that is another quest's acceptance test +commit on it only the skip, in the test framework's own form, whose reason +names that `quest` path, such as `#[ignore = "flaky: /quest/m0/flaky/.md"]` +in Rust, `test.skip` with that path in a comment in JavaScript, or +`pytest.mark.skip(reason="flaky: /quest/m0/flaky/.md")` in Python. +Never edit `quest/`: quest land writes the follow-up quest itself. + +A disabled test that is another quest's acceptance test leaves that quest as it is: the test still counts as its work. Run the [repository checks](#repository-checks) on the branch, and add its full SHA to the PR's entry: diff --git a/quest/m0/queue-watcher/README.md b/quest/m0/queue-watcher/README.md index f71904bd..1507f994 100644 --- a/quest/m0/queue-watcher/README.md +++ b/quest/m0/queue-watcher/README.md @@ -150,7 +150,6 @@ Shared shape, for the children to follow: - [The other repositories follow](/quest/m0/queue-watcher/follow.md) - quest, rind and rust2 are cut over and landing; waits for a fork PR to land through one of their queues - [Harden the queue runners](/quest/m0/queue-watcher/runners.md) - each queue runner runs only its repository's queue workflow, and every outside fork PR needs approval -- [The watcher writes flaky quests](/quest/m0/queue-watcher/flaky-quest.md) - a disable's follow-up quest comes from a template in `quest land watch`, so the headless land fixer writes no quest - [Daemons prune their logs](/quest/m0/queue-watcher/log-retention.md) - the watcher and gate delete their finished logs after a window, so state stays bounded (4.1 GiB on nixos on 2026-11-04) - [The watcher runs its own serial queue](/quest/m0/queue-watcher/own-queue.md) - `quest land watch` lands a base without GitHub's merge queue, testing one pick at a time in the repository's queue workflow - [moq.pro lands main through the own queue](/quest/m0/queue-watcher/pro-cutover.md) - moq.pro's `main` leaves the batching gate for the watcher's own queue diff --git a/quest/m0/queue-watcher/flaky-quest.md b/quest/m0/queue-watcher/flaky-quest.md deleted file mode 100644 index 4d3c9f57..00000000 --- a/quest/m0/queue-watcher/flaky-quest.md +++ /dev/null @@ -1,43 +0,0 @@ -# [S] The watcher writes flaky quests - -## Goal - -When the queue watcher disables a test seen flaking twice, the follow-up -quest (`quest/m0/flaky/.md`, with its epic created when missing) -comes from a fixed template in `quest land watch`, not from the land fixer. -The fixer's `disable` commit holds only the skip, whose reason names the -quest path the watcher chose. No headless agent writes quest prose. - -## Plan - -Decided 2026-11-05 (user, /quest-plan). The user's rule: quests are written -only by a coordinator (`/quest-complete`, `/quest-spawn`) on Opus, never by a -sub-agent. The skills already follow it: only `/quest-merge` and -`/quest-complete` plan follow-ups, and `/quest-plan` refuses to run in an -agent that cannot prompt. The land fixer, a headless Codex agent, is the one -exception: `assets/land-fix.md` has it write the flaky quest and its -`quest/m0/flaky/README.md` link in the disable commit, and -`src/cli/land-watch-flaky.ts` requires exactly one added quest file. - -Decision prompts, chosen options marked ✅: - -- Who writes it: ✅ the watcher, from a template (rejected: the fixer - reports the test and a coordinator runs `/quest-plan` on it later, which - leaves the skipped test untracked until then; keep the exception). -- Milestone: ✅ m0, in the queue watcher epic, whose end-to-end run - disables a flaky test (rejected: m1). - -Recommendations for the implementer: - -- The watcher already knows the test (the `failure` the fixer names, from - its known-flaky input) and the `merge_group` jobs that showed it, so it - picks `` and passes the quest path in the fixer's input for the skip - reason, then adds the quest file to the disable commit itself, or as a - second commit on the disable branch. -- The template's Goal says to fix the test or delete it and links each job. - Its size is a fixed guess, such as `[S]`, for the next coordinator to - correct. -- [Epics list no children](/quest/m0/token-waste/child-lists.md) is in - flight and drops the epic README links; follow whichever format the base - has when this lands. -- `validateFix` then rejects a disable commit that touches `quest/` at all. diff --git a/quest/m0/token-waste/child-lists.md b/quest/m0/token-waste/child-lists.md index 8f059249..766d49cc 100644 --- a/quest/m0/token-waste/child-lists.md +++ b/quest/m0/token-waste/child-lists.md @@ -36,9 +36,6 @@ This quest is the second PR: no history to read. #518's commit caa32fc3 has both, with tests. - Run `quest check --fix` on this tree and on prose that describes the lists. - Turn `requires X, which this epic holds` into a `quest check` error. -- `assets/land-fix.md` stops adding the flaky quest's link to - `quest/m0/flaky/README.md`, and - [the flaky quest](/quest/m0/queue-watcher/flaky-quest.md) says so. - The PR is breaking: other repositories migrate with `quest check --fix` on their pin bump past it, as [moq-dev pins](/quest/m0/model-routing/pins.md) describes. diff --git a/src/cli/land-fix.ts b/src/cli/land-fix.ts index 141bd7c9..d141219f 100644 --- a/src/cli/land-fix.ts +++ b/src/cli/land-fix.ts @@ -23,6 +23,8 @@ export const FLAKY_INPUT = "quest-land-flaky.json"; /** A test the fixer judged flaky before, by the `failure` it named, with the `merge_group` jobs that showed it. */ export interface KnownFlaky { failure: string; + /** The follow-up quest quest land watch adds when it is disabled, for the skip's reason to name. */ + quest: string; jobs: string[]; } @@ -53,6 +55,7 @@ function knownFlaky(root: string): KnownFlaky[] { (item) => item && typeof item.failure === "string" && + typeof item.quest === "string" && Array.isArray(item.jobs) && item.jobs.every((job: unknown) => typeof job === "string"), ) diff --git a/src/cli/land-watch-flaky.ts b/src/cli/land-watch-flaky.ts index b3d0e961..2adea5e4 100644 --- a/src/cli/land-watch-flaky.ts +++ b/src/cli/land-watch-flaky.ts @@ -1,8 +1,8 @@ // The merge queue watcher's record of flaky tests: each flaky verdict the fixer gives, by the test it // names, and the commit that disables a test seen flaking twice, which the watcher opens as a PR. -import { existsSync, readFileSync, writeFileSync } from "node:fs"; -import { join } from "node:path"; -import { git } from "./land"; +import { existsSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { join, resolve } from "node:path"; +import { git, tryGit } from "./land"; import { FLAKY_INPUT, type FixResult, type KnownFlaky, signature } from "./land-fix"; import { validateFix } from "./land-fixer"; import { importFixer } from "./land-fixer-repository"; @@ -38,6 +38,8 @@ export interface Disable { /** A test the fixer judged flaky, by the `failure` it named. */ export interface FlakyTest { failure: string; + /** Its follow-up quest, `quest/m0/flaky/.md`, chosen at its first verdict for the skip to name. */ + quest?: string; verdicts: FlakyVerdict[]; disable?: Disable; } @@ -53,6 +55,76 @@ export const RECENT = 10; const KNOWN_BYTES = 50_000; /** The follow-up quest a disable adds, one per test, in the repository's m0 `flaky` epic. */ const QUEST = /^quest\/m0\/flaky\/([a-z0-9][a-z0-9._-]*)\.md$/; +const EPIC = "quest/m0/flaky/README.md"; +/** The longest follow-up quest name, cut at a word. */ +const NAME_LIMIT = 60; +const BOT = { + GIT_AUTHOR_NAME: "moq-bot", + GIT_AUTHOR_EMAIL: "moq-bot@users.noreply.github.com", + GIT_COMMITTER_NAME: "moq-bot", + GIT_COMMITTER_EMAIL: "moq-bot@users.noreply.github.com", +}; + +/** + * The follow-up quest's name for `failure`: its last segment after a `:` or `>`, the test itself, in + * lowercase kebab-case. + */ +export function flakyName(failure: string): string { + const segment = + signature(failure) + .split(/[:>]/) + .map((part) => part.trim()) + .filter(Boolean) + .pop() ?? ""; + let name = segment + .toLowerCase() + .replace(/[^a-z0-9]+/g, "-") + .replace(/^-+|-+$/g, ""); + if (name.length > NAME_LIMIT) { + name = name.slice(0, NAME_LIMIT + 1); + const cut = name.lastIndexOf("-"); + name = (cut > 0 ? name.slice(0, cut) : name.slice(0, NAME_LIMIT)).replace(/-+$/, ""); + } + return name || "test"; +} + +/** A follow-up quest path for `failure` that neither another test nor the base at `base` holds. */ +function questFor(root: string, base: string, tests: FlakyTest[], failure: string): string { + const taken = new Set(tests.map((test) => test.quest ?? test.disable?.quest)); + const name = flakyName(failure); + for (let index = 1; ; index++) { + const path = `quest/m0/flaky/${index === 1 ? name : `${name}-${index}`}.md`; + if (!taken.has(path) && tryGit(root, ["cat-file", "-e", `${base}:${path}`]).status !== 0) return path; + } +} + +/** GitHub's own job URLs, the only links a quest takes from the fixer's round. */ +const GITHUB_URL = /^https:\/\/github\.com\/[^\s()<>[\]]+$/; + +/** The follow-up quest a disable adds: a fixed template, sized `[S]` for a coordinator to correct. */ +export function flakyQuest(failure: string, jobs: string[]): string { + const name = failure.replace(/`/g, "").replace(/\s+/g, " ").trim(); + const links = [...new Set(jobs)] + .filter((url) => GITHUB_URL.test(url)) + .map((url, index) => `- [Job ${index + 1}](${url})`); + return `# [S] Fix the flaky test ${flakyName(failure)} + +## Goal + +Fix \`${name}\` so it passes reliably, or delete it, and remove its skip. +quest land watch disabled it after the land fixer judged it flaky in +more than one \`merge_group\` build.${links.length ? `\n\nFailing jobs:\n\n${links.join("\n")}` : ""} +`; +} + +/** The flaky epic's README, written when the base keeps none. */ +export const FLAKY_EPIC = `# Fix disabled flaky tests + +## Goal + +Each disabled flaky test is fixed or deleted, with its skip removed when +fixed. Each quest here records the failures that led to its skip. +`; const flakyPath = (state: string) => join(state, "flaky.json"); @@ -77,7 +149,12 @@ export function writeKnownFlaky(state: string, checkout: string): void { for (const test of readFlaky(state) .filter((test) => test.disable === undefined && test.verdicts.length > 0) .sort((a, b) => latest(b).localeCompare(latest(a)))) { - const item = { failure: test.failure, jobs: test.verdicts.slice(-RECENT).flatMap((verdict) => verdict.jobs) }; + if (test.quest === undefined) continue; + const item = { + failure: test.failure, + quest: test.quest, + jobs: test.verdicts.slice(-RECENT).flatMap((verdict) => verdict.jobs), + }; bytes += Buffer.byteLength(JSON.stringify(item)) + 1; if (bytes > KNOWN_BYTES) break; known.push(item); @@ -112,6 +189,7 @@ export function recordFlaky(root: string, state: string, round: FlakyRound): str test = { failure: round.failure, verdicts: [] }; tests.push(test); } + test.quest ??= questFor(root, round.base.sha, tests, round.failure); test.verdicts = [ ...test.verdicts.filter((verdict) => verdict.job !== round.job), { job: round.job, number: round.number, at: round.dropped, jobs: round.jobs }, @@ -122,9 +200,20 @@ export function recordFlaky(root: string, state: string, round: FlakyRound): str if (test.verdicts.length < FLAKY_VERDICTS) output.push(`disable ignored ${round.number}: first flaky verdict`); else try { - const quest = disabling(root, state, round, head); - test.disable = { head, base: round.base, quest, attribution: round.attribution }; - output.push(`disable ${round.number} ${head}`); + const quest = test.quest; + const added = disabling( + root, + state, + round, + head, + quest, + flakyQuest( + test.failure, + test.verdicts.flatMap((verdict) => verdict.jobs), + ), + ); + test.disable = { head: added, base: round.base, quest, attribution: round.attribution }; + output.push(`disable ${round.number} ${added}`); } catch (error) { // A later flake of the same test tries again. output.push(`disable refused ${round.number}: ${String(error)}`); @@ -136,16 +225,35 @@ export function recordFlaky(root: string, state: string, round: FlakyRound): str /** * Import the fixer's `disable` branch at `head` and check it: new commits on the base it was given, - * leaving every protected path as the base has it, that add exactly one follow-up quest. Returns its path. + * leaving every protected path and the quest tree as the base has them. Then commit the follow-up + * `quest` on top, with the flaky epic's README when the base keeps none. Returns that commit. */ -function disabling(root: string, state: string, round: FlakyRound, head: string): string { +function disabling(root: string, state: string, round: FlakyRound, head: string, quest: string, body: string): string { importFixer(root, state, round.checkout, "disable", head, round.packs); validateFix(root, { head: round.base.sha }, head, round.base.sha); - const added = git(root, ["diff", "--name-only", "--no-renames", "--diff-filter=A", round.base.sha, head, "--"]) - .split("\n") - .filter((path) => QUEST.test(path) && path !== "quest/m0/flaky/README.md"); - if (added.length !== 1) throw new Error("a disable must add exactly one follow-up quest in quest/m0/flaky/"); - return added[0]; + const quests = git(root, ["diff", "--name-only", "--no-renames", round.base.sha, head, "--", "quest"]); + if (quests) throw new Error("a disable must leave quest/ as the base has it; quest land watch adds its quest"); + if (tryGit(root, ["cat-file", "-e", `${head}:quest/m0/README.md`]).status !== 0) + throw new Error("the base keeps no quest/m0/ for the follow-up quest"); + if (tryGit(root, ["cat-file", "-e", `${head}:${quest}`]).status === 0) throw new Error(`${quest} already exists`); + const index = resolve(root, git(root, ["rev-parse", "--git-path", `quest-land-flaky-${process.pid}.index`])); + const env = { GIT_INDEX_FILE: index }; + try { + git(root, ["read-tree", head], { env }); + const add = (path: string, text: string) => { + const blob = git(root, ["hash-object", "-w", "--stdin"], { input: text }); + git(root, ["update-index", "--add", "--cacheinfo", `100644,${blob},${path}`], { env }); + }; + add(quest, body); + if (tryGit(root, ["cat-file", "-e", `${head}:${EPIC}`]).status !== 0) add(EPIC, FLAKY_EPIC); + const tree = git(root, ["write-tree"], { env }); + return git(root, ["commit-tree", tree, "-p", head], { + input: `quest: add ${quest} for the disabled flaky test\n\nquest land watch writes the follow-up quest for the test the land fixer skipped.\n`, + env: { ...BOT }, + }); + } finally { + rmSync(index, { force: true }); + } } /** The follow-up quest's name, as a disable's branch and title name it. */ diff --git a/test/land-watch.test.ts b/test/land-watch.test.ts index 5bfa8754..b1470163 100644 --- a/test/land-watch.test.ts +++ b/test/land-watch.test.ts @@ -16,7 +16,15 @@ import { COLLECTION_ATTEMPTS, REQUEUE_LIMIT, history } from "../src/cli/land-fix import { privateRepository } from "../src/cli/land-serve"; import { jobsSection, readRemoval, SETTLE, watchPass, watchPermissions } from "../src/cli/land-watch"; import { type FixContext, HELD } from "../src/cli/land-watch-fix"; -import { type FlakyTest, RECENT, readFlaky, saveFlaky, writeKnownFlaky } from "../src/cli/land-watch-flaky"; +import { + type FlakyTest, + flakyName, + flakyQuest, + RECENT, + readFlaky, + saveFlaky, + writeKnownFlaky, +} from "../src/cli/land-watch-flaky"; import { FLAKY_INPUT, type KnownFlaky } from "../src/cli/land-fix"; import { configure, GIT_SETTINGS, initRepository, tempDir } from "./fixture"; @@ -830,6 +838,7 @@ test("a round is told the most recently seen flaky tests not yet disabled, withi // Long-lived flakes that never got a valid disable keep collecting verdicts. const tests: FlakyTest[] = Array.from({ length: 200 }, (_, index) => ({ failure: `just test: flaky_${index}`, + quest: `quest/m0/flaky/flaky-${index}.md`, verdicts: Array.from({ length: 30 }, (_, round) => ({ job: `watch-job-${index}-${round}.json`, number: round + 1, @@ -848,7 +857,7 @@ test("a round is told the most recently seen flaky tests not yet disabled, withi const text = readFileSync(join(checkout, ".git", FLAKY_INPUT), "utf8"); expect(text.length).toBeLessThan(100_000); const known = JSON.parse(text) as KnownFlaky[]; - expect(known[0].failure).toBe("just test: flaky_199"); + expect(known[0]).toMatchObject({ failure: "just test: flaky_199", quest: "quest/m0/flaky/flaky-199.md" }); expect(known.every((item) => item.jobs.length === RECENT)).toBe(true); expect(known.some((item) => item.failure === "just test: flaky_0")).toBe(false); }); @@ -1444,21 +1453,20 @@ printf '[{"number":%s,"action":"fixed","head":"%s","reason":"preserve both confl async (held) => { const { repo, state, fix, context } = setup(); const base = repo.main(); - const heads = [5, 6, 7].map((number) => repo.pull(number, { [`feature-${number}.txt`]: "new\n" })); + const heads = [5, 6, 7, 8].map((number) => repo.pull(number, { [`feature-${number}.txt`]: "new\n" })); const github = { ...world([], []), origin: repo.origin }; /** A flaky requeue naming render_timeout, disabling it at the checkout's HEAD after `script`. */ const flaky = (script: string) => `${script} printf '[{"number":%s,"action":"requeue","reason":"healthy; flaky test","cause":"flaky","failure":"%s","disable":"%s"}]' "$2" "$FAILURE" "$(git rev-parse HEAD)" > quest-land-result.json `; - /** A disabling commit on base that skips the test, adds its follow-up quest, and writes `extra`. */ - const disabling = (extra: string) => `grep -q 'just test: render_timeout' .git/quest-land-flaky.json + /** A disabling commit on base that skips the test, naming the quest it was told, and writes `extra`. */ + const disabling = ( + extra: string, + ) => `grep -q '"quest":"quest/m0/flaky/render-timeout.md"' .git/quest-land-flaky.json git checkout -q -b disable base -mkdir -p quest/m0/flaky -printf '# [S] Fix or delete render_timeout\\n\\n## Goal\\n\\nFix the flaky test or delete it.\\n' > quest/m0/flaky/render-timeout.md -printf '# Flaky tests\\n\\n## Goal\\n\\nFix or delete each disabled test.\\n\\n## Required\\n\\n- [Render timeout](/quest/m0/flaky/render-timeout.md)\\n' > quest/m0/flaky/README.md -printf 'skip render_timeout\\n' > skip.txt +printf 'skip render_timeout: /quest/m0/flaky/render-timeout.md\\n' > skip.txt ${extra} -git add quest skip.txt ${extra ? "flake.nix" : ""} +git add -A git commit -qm 'disable render_timeout'`; // The first verdict's disable is ignored: one judgment alone never disables a test. The second's edits a // protected path and is refused, and the third's is opened as its own pull request. @@ -1468,9 +1476,15 @@ git commit -qm 'disable render_timeout'`; 6, flaky(`${disabling("printf 'changed\\n' > flake.nix")}\nFAILURE='just test: \\"render_timeout\\"'`), ); - fix.round(7, flaky(`${disabling("")}\nFAILURE='just test: render_timeout'`)); + fix.round( + 7, + flaky( + `${disabling("mkdir -p quest/m0/flaky\nprintf '# x\\n' > quest/m0/flaky/render-timeout.md")}\nFAILURE='just test: render_timeout'`, + ), + ); + fix.round(8, flaky(`${disabling("")}\nFAILURE='just test: render_timeout'`)); const said: string[][] = []; - for (const [index, number] of [5, 6, 7].entries()) { + for (const [index, number] of [5, 6, 7, 8].entries()) { const minute = (index + 1) * 10; github.pulls.push({ number, @@ -1495,48 +1509,60 @@ git commit -qm 'disable render_timeout'`; await exited(state); github.now = Date.parse(at(minute + 2)); // A session takes the disable's pull request as soon as it opens. - if (held && number === 7) + if (held && number === 8) github.onPull = (pull) => { - if (pull.number === 8) pull.labels = ["quest:active"]; + if (pull.number === 9) pull.labels = ["quest:active"]; }; said.push(await watchPass(REPOSITORY, { state }, deps(github), context)); } - const branch = github.pulls.find((pull) => pull.number === 8)?.branch; + const branch = github.pulls.find((pull) => pull.number === 9)?.branch; expect(branch).toMatch(/^quest-land\/flaky-[0-9a-f]{12}$/); const head = git(repo.origin, "rev-parse", `refs/heads/${branch}`); expect(said).toEqual([ ["disable ignored 5: first flaky verdict", `requeued 5 ${heads[0]}`], [expect.stringMatching(/^disable refused 6: Error: off-limits fixer edit/), `requeued 6 ${heads[1]}`], [ - `disable 7 ${head}`, + "disable refused 7: Error: a disable must leave quest/ as the base has it; quest land watch adds its quest", `requeued 7 ${heads[2]}`, - held ? "disable held render-timeout 8" : "disabled render-timeout 8", + ], + [ + `disable 8 ${head}`, + `requeued 8 ${heads[3]}`, + held ? "disable held render-timeout 9" : "disabled render-timeout 9", ], ]); if (held) { // Left for a maintainer, even once the hold clears. github.onPull = undefined; - github.pulls.find((pull) => pull.number === 8)!.labels = []; - expect(github.comments[8].map((comment) => comment.body)).toEqual([expect.stringContaining(HELD)]); + github.pulls.find((pull) => pull.number === 9)!.labels = []; + expect(github.comments[9].map((comment) => comment.body)).toEqual([expect.stringContaining(HELD)]); } - // The disable is the fixer's commit on the base, enqueued like any other pull request with no pick. - expect(git(repo.origin, "rev-parse", `${head}^`)).toBe(base); - expect(git(repo.origin, "show", `${head}:quest/m0/flaky/render-timeout.md`)).toContain( - "Fix the flaky test", - ); + // The disable is the fixer's skip on the base and the watcher's quest from its template on top, with + // the epic it creates, enqueued like any other pull request with no pick. + expect(git(repo.origin, "rev-parse", `${head}^^`)).toBe(base); + expect(git(repo.origin, "diff", "--name-only", base, `${head}^`)).toBe("skip.txt"); + expect(git(repo.origin, "diff", "--name-only", `${head}^`, head).split("\n")).toEqual([ + "quest/m0/flaky/README.md", + "quest/m0/flaky/render-timeout.md", + ]); + const quest = git(repo.origin, "show", `${head}:quest/m0/flaky/render-timeout.md`); + expect(quest).toContain("# [S] Fix the flaky test render-timeout"); + expect(quest).toContain("- [Job 1](https://github.com/upstream/project/actions/runs/50/job/71)"); + expect(git(repo.origin, "show", `${head}:quest/m0/flaky/README.md`)).toContain("## Goal"); expect(github.mutations).toEqual([ `enqueuePullRequest 5 ${heads[0]}`, `enqueuePullRequest 6 ${heads[1]}`, `enqueuePullRequest 7 ${heads[2]}`, - ...(held ? [] : [`enqueuePullRequest 8 ${head}`]), + `enqueuePullRequest 8 ${heads[3]}`, + ...(held ? [] : [`enqueuePullRequest 9 ${head}`]), ]); - const opened = github.pulls.find((pull) => pull.number === 8)!; + const opened = github.pulls.find((pull) => pull.number === 9)!; expect(opened.title).toBe("quest land: disable the flaky test render-timeout"); expect(opened.body).toContain( - "The land fixer judged `just test: render_timeout` flaky 3 times, most recently in", + "The land fixer judged `just test: render_timeout` flaky 4 times, most recently in", ); expect(opened.body).toContain( - "#5 ([job 1](https://github.com/upstream/project/actions/runs/50/job/71)), #6 ([job 1](https://github.com/upstream/project/actions/runs/60/job/71))", + "#6 ([job 1](https://github.com/upstream/project/actions/runs/60/job/71)), #7 ([job 1](https://github.com/upstream/project/actions/runs/70/job/71))", ); expect(opened.body).toContain("adds its follow-up quest, `quest/m0/flaky/render-timeout.md`"); expect(opened.body).toContain(`## Review\n\nmechanical fix by Codex: \``); @@ -1544,11 +1570,12 @@ git commit -qm 'disable render_timeout'`; expect(readFlaky(state)).toMatchObject([ { failure: "just test: render_timeout", - verdicts: [{ number: 5 }, { number: 6 }, { number: 7 }], + quest: "quest/m0/flaky/render-timeout.md", + verdicts: [{ number: 5 }, { number: 6 }, { number: 7 }, { number: 8 }], disable: { head, quest: "quest/m0/flaky/render-timeout.md", - number: 8, + number: 9, done: held ? "held" : "enqueued", }, }, @@ -1556,7 +1583,7 @@ git commit -qm 'disable render_timeout'`; // A restart neither opens nor enqueues it again, and the disabled test is no longer offered as known. expect(await watchPass(REPOSITORY, { state }, deps(github), context)).toEqual([]); - expect(github.mutations).toHaveLength(held ? 3 : 4); + expect(github.mutations).toHaveLength(held ? 4 : 5); }, ); @@ -1904,3 +1931,28 @@ git commit -qm 'disable render_timeout'`; expect(await watchPass(REPOSITORY, { state }, deps(github), context)).toEqual([]); }); }); + +test("a flaky test's follow-up quest is named for the test and written from a fixed template", () => { + expect(flakyName("cargo test -p moq-relay: interop::render_timeout")).toBe("render-timeout"); + expect(flakyName("test/review.test.ts > a reviewer's usage limit exits 3")).toBe("a-reviewers-usage-limit-exits-3"); + expect(flakyName("x".repeat(30) + " " + "y".repeat(40))).toBe("x".repeat(30)); + expect(flakyName("::")).toBe("test"); + const quest = flakyQuest("just test: `render_timeout`", [ + "https://github.com/o/r/actions/runs/1/job/2", + "https://github.com/o/r/actions/runs/1/job/2", + "https://evil.example/x", + "https://github.com/o/r/actions/runs/1/job/3)[x](https://evil.example", + ]); + expect(quest).toBe(`# [S] Fix the flaky test render-timeout + +## Goal + +Fix \`just test: render_timeout\` so it passes reliably, or delete it, and remove its skip. +quest land watch disabled it after the land fixer judged it flaky in +more than one \`merge_group\` build. + +Failing jobs: + +- [Job 1](https://github.com/o/r/actions/runs/1/job/2) +`); +}); diff --git a/test/land.test.ts b/test/land.test.ts index efa0383a..7e673917 100644 --- a/test/land.test.ts +++ b/test/land.test.ts @@ -3330,7 +3330,11 @@ ${requeueScript} // The daemon's known flaky tests, in the checkout's .git, follow the PRs as data. const known = [ - { failure: "cargo test: render_timeout", jobs: ["https://github.com/o/r/actions/runs/1/job/2"] }, + { + failure: "cargo test: render_timeout", + quest: "quest/m0/flaky/render-timeout.md", + jobs: ["https://github.com/o/r/actions/runs/1/job/2"], + }, ]; writeFileSync(join(dir, ".git", FLAKY_INPUT), JSON.stringify(known)); await fix(dir, log, [1], agent, ["-"]);