diff --git a/scripts/coverage-floor.test.mjs b/scripts/coverage-floor.test.mjs index e971af2..43529af 100644 --- a/scripts/coverage-floor.test.mjs +++ b/scripts/coverage-floor.test.mjs @@ -42,7 +42,6 @@ export const TEST_HELPER_BASELINE = { "scripts/jsonld-contract.test.mjs": 14, "scripts/llms-txt.test.mjs": 3, "scripts/no-js-controls.test.mjs": 5, - "scripts/owners-file.test.mjs": 4, "scripts/page-404.test.mjs": 5, "scripts/page-images.test.mjs": 19, "scripts/page-markup.test.mjs": 2, diff --git a/scripts/owners-file.test.mjs b/scripts/owners-file.test.mjs index 6a1bb06..22e3d3a 100755 --- a/scripts/owners-file.test.mjs +++ b/scripts/owners-file.test.mjs @@ -139,9 +139,9 @@ export function checkOwners(text, { isRoot = true, aliases = new Set() } = {}) { } if (isRoot && !(Array.isArray(doc.approvers) && doc.approvers.length)) problems.push("root OWNERS must list at least one approver (`approvers:`), or nothing can be /approve'd"); for (const key of PEOPLE_KEYS) { + // parseOwners only ever builds a map for `options`; every other key is a list. const list = doc[key]; if (list === undefined) continue; - if (!Array.isArray(list)) { problems.push(`\`${key}\` must be a list`); continue; } const seen = new Set(); for (const entry of list) { const lower = entry.toLowerCase(); @@ -214,6 +214,7 @@ test("tabs, scalars where lists belong and stray lines are reported with line nu assert.match(checkOwners("approvers: alice\n").join("\n"), /line 1: `approvers` must introduce a list/); assert.match(checkOwners(" - alice\napprovers:\n - bob\n").join("\n"), /line 1: list item before any key/); assert.match(checkOwners("approvers:\n - alice\n nested: x\n").join("\n"), /line 3: nested key `nested` under `approvers`/); + assert.match(checkOwners(" nested: x\napprovers:\n - alice\n").join("\n"), /line 1: nested key `nested` under `\(none\)`, which is not a map/); assert.match(checkOwners("approvers:\n - alice\n:::\n").join("\n"), /line 3: unparseable line/); assert.match(checkOwners("approvers:\n - \n").join("\n"), /line 2: empty list item/); assert.match(checkOwners("approvers:\n - alice\napprovers:\n - bob\n").join("\n"), /line 3: duplicate top-level key `approvers`/); @@ -225,6 +226,13 @@ test("only keys prow knows are accepted", () => { assert.deepEqual(checkOwners(GOOD + "emeritus_approvers:\n - dave\nlabels:\n - area/site\n"), []); }); +test("a quoted whitespace-only label survives parsing and is reported as empty", () => { + // `scalar` strips the quotes after trimming, so `" "` reaches the list as + // two spaces rather than tripping the parser's empty-item rule. + assert.deepEqual(parseOwners('labels:\n - " "\n').problems, []); + assert.deepEqual(checkOwners(GOOD + 'labels:\n - " "\n'), ["`labels` has an empty entry"]); +}); + test("options must be a map of known boolean flags", () => { assert.match(checkOwners(GOOD + "options:\n - no_parent_owners\n").join("\n"), /list item under `options`, which is a map/); assert.match(checkOwners(GOOD + "options: [no_parent_owners]\n").join("\n"), /`options` must be a map/); @@ -267,6 +275,7 @@ test("OWNERS_ALIASES parses alias names and rejects malformed members", () => { assert.match(parseAliases("approvers:\n - alice\n").problems.join("\n"), /only an `aliases:` block/); assert.match(parseAliases("aliases:\n - alice\n").problems.join("\n"), /login before any alias name/); assert.match(parseAliases("aliases:\n\tx:\n").problems.join("\n"), /tab indentation/); + assert.match(parseAliases("aliases:\n x:\n :::\n").problems.join("\n"), /line 3: unparseable line " :::"/); }); // In-memory tree so the walker and the per-file rule wiring are exercised