Skip to content

Commit e5a1732

Browse files
committed
test(e2e): unreadable scenario roots fail loudly; pin the layering guide's refusal rows (#117)
Two small harness fixes, both test-only plus one docs table. ## 1. An unreadable scenario root fails loudly **Before** ```text $ chmod 000 src/sections/labels/scenarios && bun test/e2e/run.ts no scenario .yml files found under test/e2e/scenarios, ... $ echo $? 0 ``` `collectYmlFiles` swallowed every readdir error and returned `[]`, so an unreadable corpus was indistinguishable from an empty one. **After** ```text $ chmod 000 src/sections/labels/scenarios && bun test/e2e/run.ts Error: cannot read the scenario directory .../src/sections/labels/scenarios: EACCES: permission denied, scandir '...' $ echo $? 1 ``` **How** - The loader decides absence versus unreadability once, at the owner (`test/e2e/schema.ts`): `ENOENT` still yields `[]` (a section may have no `scenarios/` yet), any other readdir failure throws naming the directory and the error, with `cause`. - `loadScenarios` propagates it, so `run.ts` and the coverage tripwire both fail instead of reporting an empty corpus. The `run.ts` comment on the empty-set branch now says so. - `scenarioRoots` no longer filters section paths with `existsSync` (false for an absent AND an unreadable directory, so a mode-000 section dropped its root silently). It returns the cross-cutting root plus `<sectionsDir>/<key>/scenarios` for every key in `SECTION_KEYS`, unfiltered; absence is decided exactly once, in `collectYmlFiles`. `sectionsDir` is injectable so a test can stage a tree. - No caller relied on the old swallow: every root today comes from `scenarioRoots()` (already `existsSync`-filtered) or is `test/e2e/scenarios`. ## 2. The layering guide's refusal rows are pinned to the engine **Before** The two tables under `## Refusals` in `docs/operate/layering.md` quoted nine engine messages by hand, with no test. Only one had been verified. **After** Each row's first cell ends with the layer that triggers it, so the page is the single source: ```text | Two entries under one key in one list (`labels: [{name: bug}, {name: docs}, {name: Bug}]`) | `labels[0] and labels[2] both claim one name; ...` | ``` A guides test parses both tables, folds each row's layer through the merge step's real gate order (`foldLayers`: per-layer validation, then the fold) under the layer name the page quotes as the fold's prefix, and asserts the whole produced error equals the wrapper around the quoted message. Validation rows are also held equal to the standalone validator's error (the page's "same messages a standalone file gets" claim). The wrapper text around a validation problem (`<layer> has malformed section entries: ... Fix these values in ...`) comes from `src/engine/validate.ts`, and the test holds the whole error to that wrapper around the one quoted problem. **How** - `refusalRows` in `test/docs/guides.test.ts` reads tables as GFM renders them: a piped line opens a table only when the delimiter row follows it, exactly that one line is skipped, every non-blank line after it is a row (outer pipes optional, indentation allowed), and a blank line closes the table. A row without its parenthesized input, a row whose message is not one code span, a one- or three-cell row, an unknown or malformed header, or a piped line with no delimiter after it fails by name. A row appended without its leading pipe, or with a dash-run first cell, is pinned rather than skipped (gate review findings). - A mutation-check block proves each of those parser branches. - One row's prose was tightened: "a rule with no `type`" was dropped from the keyed-entry row because its message differs from the one the row quotes; the label form is the one reproduced. The "no layering key" row names both forms the page claims (wrapper and file-level), and both are reproduced. ## Proof - `bun run check`: green (2613 pass, 1 skip, 0 fail; lint, lint:arch, typecheck, knip, build:check all green). - Mutation checks run by hand, then reverted: swallowing all errno codes in the loader fails the unreadable-root test; changing one word of a quoted refusal on the page fails that row's test; indenting a row and changing its message fails that row's test; appending an unpiped last row with a wrong message fails that row's test. - Codex rubber-duck, read-only: round 1 one BLOCKING (indented table rows were skipped), fixed and covered; round 2 no blocking, no non-blocking; round 3 (on the gate-review fixes) no blocking, no non-blocking. Tests added: - `scenario corpus loader (collectYmlFiles)`: a root that does not exist yields `[]`; a readable empty root yields `[]`; an unreadable root fails naming it and the error (skipped as root); `scenarioRoots` names every registered section's path, present or not, and an empty tree loads as `[]`; an unreadable section `scenarios/` fails the whole corpus naming it (skipped as root). - `the layering guide's refusal tables quote the messages the merge step emits`: both tables present; one test per row (3 validation, 6 fold). - `refusal table parser (mutation checks)`: 13 cases.
1 parent 3381d3f commit e5a1732

5 files changed

Lines changed: 397 additions & 31 deletions

File tree

‎docs/operate/layering.md‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -254,19 +254,19 @@ The per-layer validation catches what a standalone settings file could not say,
254254
| The layer has | Caught by |
255255
|---|---|
256256
| A list section that is not a list or an `{entries}` wrapper (`labels: oops`) | Validation: `labels: Invalid input: expected a list of entries, or a mapping with "entries" (and an optional "_undeclared" policy), but this section parsed as string` |
257-
| A keyed entry without its key (a label with no `name`, a rule with no `type`) | Validation: `labels[1].name: Invalid input: expected string, received undefined` |
257+
| A keyed entry without its key, here a label with no `name` (`labels: [{name: bug}, {color: d73a4a}]`) | Validation: `labels[1].name: Invalid input: expected string, received undefined` |
258258
| A non-mapping entry (`milestones: [v2]`) | Validation: `milestones[0]: Invalid input: expected object, received string` |
259259

260260
The fold itself refuses what only a merge can judge (`layer ".github/settings/repo.yml": ...`). A fold refusal names the key path (entries by index) and the kind of problem, never a value from the document: the merge runs without a repository's redaction context, so a label name or rule type echoed here could put a private repository's settings into a public log.
261261

262262
| The layer has | The fold says |
263263
|---|---|
264-
| Two entries under one key in one list | `labels[0] and labels[2] both claim one name; each name belongs to one entry within a layer` |
265-
| Two rules of one type in one ruleset | `rulesets[0].rules[0] and rulesets[0].rules[1] both claim one type; each type belongs to one entry within a layer` |
266-
| `_layering: merge` on a section with no layering key, on its wrapper or reached from the file level | `milestones has no layering key, so it cannot be layered by "merge"; declare _layering: replace or drop the directive` |
267-
| An unknown `_layering` value on a wrapper | `labels._layering must be "merge" or "replace"; got a string that is neither` |
268-
| An unknown `_layering` value at the file's top level | `_layering must be "merge" or "replace"; got a string that is neither` |
269-
| A YAML anchor aliased inside its own node | `the document contains a reference cycle (a YAML anchor that includes itself); layers must be trees` |
264+
| Two entries under one key in one list (`labels: [{name: bug}, {name: docs}, {name: Bug}]`) | `labels[0] and labels[2] both claim one name; each name belongs to one entry within a layer` |
265+
| Two rules of one type in one ruleset (`rulesets: [{name: main, rules: [{type: deletion}, {type: deletion}]}]`) | `rulesets[0].rules[0] and rulesets[0].rules[1] both claim one type; each type belongs to one entry within a layer` |
266+
| `_layering: merge` on a section with no layering key, on its wrapper or reached from the file level (`milestones: {_layering: merge, entries: [{title: v1}]}` or `{_layering: merge, milestones: [{title: v1}]}`) | `milestones has no layering key, so it cannot be layered by "merge"; declare _layering: replace or drop the directive` |
267+
| An unknown `_layering` value on a wrapper (`labels: {_layering: union, entries: [{name: bug}]}`) | `labels._layering must be "merge" or "replace"; got a string that is neither` |
268+
| An unknown `_layering` value at the file's top level (`_layering: union`) | `_layering must be "merge" or "replace"; got a string that is neither` |
269+
| A YAML anchor aliased inside its own node (`_notes: &loop {self: *loop}`) | `the document contains a reference cycle (a YAML anchor that includes itself); layers must be trees` |
270270

271271
## Where to go next
272272

‎test/docs/guides.test.ts‎

Lines changed: 247 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { join, posix } from "node:path";
1515
import { DecodingMode, decodeHTML } from "entities";
1616
import { parse as parseYaml } from "yaml";
1717
import { MERGE_REJECTED_INPUTS } from "../../src/action/inputs.js";
18+
import { foldLayers } from "../../src/action/layers.js";
1819
import { type Layer, mergeLayers, stripNulls } from "../../src/engine/layers.js";
1920
import { validateSettingsDoc } from "../../src/engine/orchestrate.js";
2021
import { SECTION_KEYS } from "../../src/schema.js";
@@ -288,6 +289,125 @@ function headingSlugs(markdown: string, source: string): Set<string> {
288289
return slugs;
289290
}
290291

292+
/** One row of the layering guide's refusal tables, with the layer(s) that trigger it. */
293+
interface RefusalRow {
294+
/** Which gate the table documents, from its header's second cell. */
295+
readonly gate: "validation" | "fold";
296+
/** The first cell, the row's name in a failure. */
297+
readonly has: string;
298+
/** The refused layer(s), flow YAML, one document each. */
299+
readonly inputs: string[];
300+
/** The message the second cell quotes, whole. */
301+
readonly quoted: string;
302+
}
303+
304+
/**
305+
* The cells of one GFM table row: the outer pipes are optional, so a row
306+
* written without them (or with only one) is still a row and still pinned.
307+
*/
308+
function tableCells(line: string): string[] {
309+
let body = line.trim();
310+
if (body.startsWith("|")) {
311+
body = body.slice(1);
312+
}
313+
if (body.endsWith("|")) {
314+
body = body.slice(0, -1);
315+
}
316+
return body.split("|").map((cell) => cell.trim());
317+
}
318+
319+
/** A GFM delimiter row (`|---|:--:|`), the one line that opens a table after its header. */
320+
function isDelimiterRow(line: string | undefined): boolean {
321+
return line !== undefined && /^\s*\|?(\s*:?-+:?\s*\|)*\s*:?-+:?\s*\|?\s*$/.test(line);
322+
}
323+
324+
/**
325+
* The rows of the layering guide's two refusal tables. The page is the single
326+
* source: a row's first cell ends with the layer that triggers it, as one code
327+
* span in parentheses (or several joined by "or" when the page names
328+
* alternatives), and its second cell quotes the message the engine emits, in
329+
* one code span. Rows are recognized more permissively than GFM renders them:
330+
* a header line followed by a delimiter row opens a table (the delimiter's
331+
* cell count is not checked, so `|---|` under a two-cell header opens one here
332+
* and renders as prose), every non-blank line after that is a row (outer pipes
333+
* optional; a list item shaped like a row counts), and a blank line closes it.
334+
* Every divergence adds a pin, never loses one. A row missing its input or its
335+
* message fails here by name, so a new row cannot land unpinned.
336+
*/
337+
function refusalRows(section: readonly string[], source: string): RefusalRow[] {
338+
const GATES: Record<string, RefusalRow["gate"]> = {
339+
"Caught by": "validation",
340+
"The fold says": "fold",
341+
};
342+
const rows: RefusalRow[] = [];
343+
let gate: RefusalRow["gate"] | null = null;
344+
for (let index = 0; index < section.length; index++) {
345+
const line = section[index] ?? "";
346+
if (line.trim() === "") {
347+
gate = null;
348+
continue;
349+
}
350+
if (gate === null) {
351+
if (!line.includes("|")) {
352+
continue; // prose between the tables
353+
}
354+
// A piped line opens a table only when the delimiter row follows it;
355+
// one that renders as prose is an authoring slip, not a row to skip.
356+
if (!isDelimiterRow(section[index + 1])) {
357+
throw new Error(`${source}: refusal table row outside a known table: ${line.trim()}`);
358+
}
359+
const header = tableCells(line);
360+
if (header.length !== 2) {
361+
throw new Error(`${source}: refusal table header is not two cells: ${line.trim()}`);
362+
}
363+
const [has = "", says = ""] = header;
364+
gate = has === "The layer has" ? (GATES[says] ?? null) : null;
365+
if (gate === null) {
366+
throw new Error(`${source}: unknown refusal table header "${line.trim()}"`);
367+
}
368+
index++; // the delimiter row, the only line a table skips
369+
continue;
370+
}
371+
const cells = tableCells(line);
372+
if (cells.length !== 2) {
373+
throw new Error(`${source}: refusal table row is not two cells: ${line.trim()}`);
374+
}
375+
const [has = "", says = ""] = cells;
376+
const trigger = has.match(/\((`[^`]+`(?: or `[^`]+`)*)\)$/);
377+
if (!trigger) {
378+
throw new Error(
379+
`${source}: refusal row "${has}" does not end with its layer in parentheses as a code span (or code spans joined by "or"), so it cannot be reproduced`,
380+
);
381+
}
382+
const quoted = says.match(/^(?:Validation: )?`([^`]+)`$/);
383+
if (!quoted) {
384+
throw new Error(`${source}: refusal row "${has}" does not quote one message in a code span`);
385+
}
386+
rows.push({
387+
gate,
388+
has,
389+
inputs: [...(trigger[1] ?? "").matchAll(/`([^`]+)`/g)].map((match) => match[1] ?? ""),
390+
quoted: quoted[1] ?? "",
391+
});
392+
}
393+
return rows;
394+
}
395+
396+
/**
397+
* The validator's wrapper around its problem list. The page quotes the problem
398+
* alone ("with the same messages a standalone file gets"); holding the whole
399+
* error to the wrapper around ONE quoted problem proves the row's layer raises
400+
* that problem and nothing else.
401+
*/
402+
function malformedSectionEntries(layer: string, problem: string): string {
403+
return (
404+
`${layer} has malformed section entries: ${problem}. Fix these values in ` +
405+
`the settings file (only the named keys are validated; extra fields pass through, except ` +
406+
`in closed sections and strict nested objects like actions.cache, which reject ` +
407+
`unrecognized keys)`
408+
);
409+
}
410+
291411
describe("docs/ guide pages", () => {
292412
test("every required guide page exists, and no page exists outside the set", () => {
293413
// Exact equality, not inclusion: after the tree restructure this is what
@@ -619,6 +739,53 @@ describe("docs/ guide pages", () => {
619739
expect(new Set(named)).toEqual(new Set(MERGE_REJECTED_INPUTS));
620740
});
621741

742+
describe("the layering guide's refusal tables quote the messages the merge step emits", () => {
743+
// Each row's layer runs through the merge step's own gates in the page's
744+
// order (per-layer validation, then the fold), and the row's quoted
745+
// message must equal the whole error, so neither table can drift from the
746+
// engine and no row can quote a message its own layer does not raise.
747+
const PAGE = "docs/operate/layering.md";
748+
const section = sectionLines(
749+
readFileSync(join(DOCS, "operate", "layering.md"), "utf8"),
750+
"Refusals",
751+
PAGE,
752+
);
753+
// The page names the refused layer in the fold's prefix it quotes
754+
// (`layer "<name>": ...`); every row's layer is folded under that name.
755+
const prefix = section.join("\n").match(/`layer "([^"`]+)": \.\.\.`/);
756+
if (!prefix) {
757+
throw new Error(
758+
`${PAGE}'s Refusals section no longer quotes the fold's \`layer "<name>": ...\` prefix`,
759+
);
760+
}
761+
const layerName = prefix[1] ?? "";
762+
const rows = refusalRows(section, PAGE);
763+
764+
test("both tables are present and populated", () => {
765+
expect(rows.filter((row) => row.gate === "validation").length).toBeGreaterThan(0);
766+
expect(rows.filter((row) => row.gate === "fold").length).toBeGreaterThan(0);
767+
});
768+
769+
test.each(rows.map((row) => [row.has, row] as const))("%s", (_has, row) => {
770+
for (const input of row.inputs) {
771+
const doc = parseYaml(input);
772+
const expected =
773+
row.gate === "validation"
774+
? malformedSectionEntries(layerName, row.quoted)
775+
: `layer "${layerName}": ${row.quoted}`;
776+
const folded = foldLayers([{ name: layerName, doc }], "merged", "merge", silentIo());
777+
expect("error" in folded ? folded.error : null, `layer ${input}`).toBe(expected);
778+
if (row.gate === "validation") {
779+
// "with the same messages a standalone file gets"
780+
const standalone = validateSettingsDoc(doc, layerName, new Set(), silentIo());
781+
expect("error" in standalone ? standalone.error : null, `standalone ${input}`).toBe(
782+
expected,
783+
);
784+
}
785+
}
786+
});
787+
});
788+
622789
test("the v2-to-v3 guide quotes the complete wrapper-key rename error the validator emits", () => {
623790
// The guide's text fence is the reader's search string, so it is held to
624791
// the error the validator emits for a v2 wrapper, not to a source substring.
@@ -867,6 +1034,86 @@ describe("links-leaving-docs guard (mutation checks)", () => {
8671034
});
8681035
});
8691036

1037+
describe("refusal table parser (mutation checks)", () => {
1038+
// Each mutation is a realistic authoring slip on the layering guide's
1039+
// refusal tables; the parser must pin or reject every one, never skip a row.
1040+
const header = ["| The layer has | The fold says |", "|---|---|"];
1041+
const row = (has: string, says = "`the message`"): string => `| ${has} | ${says} |`;
1042+
1043+
test("an indented row is parsed as a row, not skipped", () => {
1044+
const rows = refusalRows([...header, ` ${row("Indented (`labels: oops`)")}`], "page");
1045+
expect(rows.map((parsed) => parsed.inputs)).toEqual([["labels: oops"]]);
1046+
});
1047+
1048+
test("a row without its outer pipes is parsed as a row, not prose", () => {
1049+
// GFM renders a row with no leading pipe (or no trailing one) as a row,
1050+
// so a last row appended that way must be pinned rather than skipped.
1051+
const unpiped = "New refusal (`_layering: union`) | `WRONG MESSAGE` |";
1052+
const bare = "Bare (`a: 1`) | `also wrong`";
1053+
const rows = refusalRows([...header, row("First (`labels: oops`)"), unpiped, bare], "page");
1054+
expect(rows.map((parsed) => [parsed.inputs, parsed.quoted])).toEqual([
1055+
[["labels: oops"], "the message"],
1056+
[["_layering: union"], "WRONG MESSAGE"],
1057+
[["a: 1"], "also wrong"],
1058+
]);
1059+
});
1060+
1061+
test("a blank line closes a table; the next table needs its own header", () => {
1062+
const rows = refusalRows(
1063+
[...header, row("A (`a: 1`)"), "", "Prose with no pipes.", "", ...header, row("B (`b: 2`)")],
1064+
"page",
1065+
);
1066+
expect(rows.map((parsed) => parsed.inputs)).toEqual([["a: 1"], ["b: 2"]]);
1067+
});
1068+
1069+
test("two alternatives joined by or are both inputs", () => {
1070+
const rows = refusalRows([...header, row("Either (`a: 1` or `b: 2`)")], "page");
1071+
expect(rows.map((parsed) => parsed.inputs)).toEqual([["a: 1", "b: 2"]]);
1072+
});
1073+
1074+
test.each<[string, string[], RegExp]>([
1075+
[
1076+
"a row without its layer in parentheses",
1077+
[...header, row("No input here")],
1078+
/does not end with its layer/,
1079+
],
1080+
[
1081+
"a data row whose first cell is a dash run",
1082+
[...header, row("---", "`hidden message`")],
1083+
/refusal row "---" does not end with its layer/,
1084+
],
1085+
[
1086+
"a row whose message is prose, not one code span",
1087+
[...header, row("X (`a: 1`)", "prose")],
1088+
/does not quote one message/,
1089+
],
1090+
["a row with three cells", [...header, "| a | b | c |"], /is not two cells/],
1091+
[
1092+
"a prose line run into the table without a blank line",
1093+
[...header, row("X (`a: 1`)"), "A paragraph GFM reads as a one-cell row."],
1094+
/is not two cells/,
1095+
],
1096+
[
1097+
"a table under an unknown header",
1098+
["| The layer has | Something |", "|---|---|"],
1099+
/unknown refusal table header/,
1100+
],
1101+
[
1102+
"a table whose header is not two cells",
1103+
["| The layer has | The fold says | Extra |", "|---|---|---|"],
1104+
/header is not two cells/,
1105+
],
1106+
["a row before any header", [row("X (`a: 1`)")], /outside a known table/],
1107+
[
1108+
"a piped line with no delimiter row after it (renders as prose)",
1109+
["| The layer has | The fold says |", row("X (`a: 1`)")],
1110+
/outside a known table/,
1111+
],
1112+
])("%s is rejected by name", (_case, section, error) => {
1113+
expect(() => refusalRows(section, "page")).toThrow(error);
1114+
});
1115+
});
1116+
8701117
describe("fence policy guard (mutation checks)", () => {
8711118
// Each mutation is a realistic authoring mistake that would make an
8721119
// example invisible to fencedBlocks; the guard must reject every one,

0 commit comments

Comments
 (0)