open/voyage
1
0
Fork 0
Code Issues Releases 13 Activity

fix(plan): a 1.9 entry keeps its wrapped paths, and a numbered File line names its test

Browse source 
Review findings b4fa1927 and 4b6460e5.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-10-13 12:43:26 +02:00
commit fddd0a078c
Signed by: ktg
SSH key fingerprint: SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
2 changed files with 103 additions and 10 deletions
Download patch file Download diff file Expand all files Collapse all files

53
lib/parsers/plan-schema.mjs
View file

@ -474,9 +474,13 @@ export function fieldPaths(text) {
// as before and gets one warning, PLAN_STEP_PATH_UNTICKED):
//
// - The entries are the field's first non-empty line and each list-item line
// (`-`, `*`, `+`, `1.`, `1)`). Any other line continues the entry above it,
// so a wrapped description line is no entry, and an empty first line before
// a nested list is no entry either.
// (`-`, `*`, `+`, `1.`, `1)`). Any other non-empty line continues the entry
// above it and is appended to that entry's text with one space before the
// entry is read, so a list wrapped after a comma ("`tests/a.test.mjs`," and
// "`src/a.mjs`" on the next line) names both files, a wrapped description
// line stays description, and an empty first line before a nested list is
// no entry. An issue carries the line number of the entry's first line
// (review `b4fa1927`).
// - An entry's files are the backticked spans it opens with, separated only by
// commas or whitespace. Everything from the first other text on is
// description and is never read ("`a.test.mjs` - covers `src/parse.mjs`"
@ -486,6 +490,15 @@ export function fieldPaths(text) {
// A leading span that fails isPathShape (`parse()`, an option, a URL) is
// refused as no path (PLAN_STEP_PATH_NOT_A_PATH).
//
// - A `Test first:` `File:` line is a line that opens with `File:` (or
// `**File:**`) after an optional list marker of the same set (`-`, `*`, `+`,
// `1.`, `1)`); its rest is one entry under this grammar. The non-list lines
// right after it are appended to it with one space, as above; any list item
// (a `Verifies:` or `Pattern:` line too) ends it, and a non-list line after
// such an item belongs to that item, never to the `File:` entry. A line that
// opens with `File:` always starts a new entry; a `File:` mid-line names no
// test (review `4b6460e5`).
//
// isPathShape is the test fieldPaths puts on a span in the leading position of
// an entry (a path, a build file, or a capitalised name such as `LICENSE`), so
// a 1.9 entry's leading span is judged exactly as the reader below 1.9 judges
@ -533,8 +546,15 @@ export function entryPaths(text) {
const entries = [];
String(text ?? '').split('\n').forEach((line, i) => {
const item = LIST_ITEM.exec(line);
const rest = line.trim();
if (item) entries.push({ line: i + 1, entry: line.slice(item[0].length).trim() });
else if (entries.length === 0 && line.trim() !== '') entries.push({ line: i + 1, entry: line.trim() });
else if (rest === '') return;
else if (entries.length === 0) entries.push({ line: i + 1, entry: rest });
else {
// A wrapped continuation line belongs to the entry above it.
const last = entries[entries.length - 1];
last.entry = `${last.entry} ${rest}`.trim();
}
});
if (entries.length === 0) {
issues.push({ line: 1, entry: '', code: 'PLAN_STEP_PATH_NOT_FIRST' });
@ -599,12 +619,14 @@ export function stepTestsOnly(body, version) {
// selector); global, so read it with matchAll only: one match per `File:`.
const TEST_FIRST_FILE_LINE = /\bFile:\**[ \t]*`/gi;
// From plan_version 1.9: a line that opens with `File:` (after an optional
// list marker and `**`), backticked or not.
const TEST_FIRST_FILE_LINE_19 = /^\s*(?:[-*+]\s+)?(?:\*\*)?File:(?:\*\*)?/i;
// list marker of the set LIST_ITEM accepts, and `**`), backticked or not.
const TEST_FIRST_FILE_LINE_19 = /^\s*(?:(?:[-*+]|\d+[.)])\s+)?(?:\*\*)?File:(?:\*\*)?/i;
/**
* plan_version 1.9: the rest of each `Test first:` `File:` line (after `File:`
* and any `**`), one entry per line, in order. The one selector both
* and any `**`), with the non-list lines right after it appended (one space
* each), one entry per `File:` line, in order. Any list item ends the entry;
* a line that opens with `File:` starts a new one. The one selector both
* testFirstFiles and the plan validator's grammar check read.
* @param {Map<string, {text: string}>} fields - parseStepFields(body)
* @returns {string[]}
@ -612,9 +634,18 @@ const TEST_FIRST_FILE_LINE_19 = /^\s*(?:[-*+]\s+)?(?:\*\*)?File:(?:\*\*)?/i;
export function testFirstFileEntries(fields) {
if (!fields || !fields.has('Test first')) return [];
const out = [];
// Whether the line above is the current `File:` line or its continuation.
let open = false;
for (const line of String(fields.get('Test first').text ?? '').split('\n')) {
const at = TEST_FIRST_FILE_LINE_19.exec(line);
if (at) out.push(line.slice(at[0].length).trim());
if (at) {
out.push(line.slice(at[0].length).trim());
open = true;
} else if (LIST_ITEM.test(line) || line.trim() === '') {
open = false;
} else if (open) {
out[out.length - 1] = `${out[out.length - 1]} ${line.trim()}`.trim();
}
}
return out;
}
@ -634,8 +665,10 @@ export function testFirstFileEntries(fields) {
* `File:`, so a line with two markers keeps at least 6.3.1's files.
*
* From 1.9 a line counts when it opens with `File:` (testFirstFileEntries),
* backticked or not, and its rest is one entry read by entryPaths: the
* leading backticked paths are the test files, the rest is description.
* backticked or not, after an optional list marker (`1.` and `1)` included);
* its rest, with its wrapped continuation lines, is one entry read by
* entryPaths: the leading backticked paths are the test files, the rest is
* description.
*
* Normalised, in order, deduplicated. A `Verifies:` line without a `File:` is
* never read.

60
tests/validators/plan-version-1-9.test.mjs
View file

@ -269,3 +269,63 @@ test('below 1.9: the step check reads a mid-line File: as before', () => {
const r = validatePlanContent(planTestFirst19('1.8', ' - see File: `tests/mod1.test.mjs` for the test'), { strict: true });
assert.equal(r.errors.find((e) => e.code === 'PLAN_STEP_NO_TEST_FIRST'), undefined, errCodes(r).join(','));
});
// ---- Step 3 (trial 4): a wrapped 1.9 entry keeps its paths, and a numbered
// `File:` line names its test (reviews b4fa1927, 4b6460e5) ----
const WRAPPED = '`tests/a.test.mjs`,\n `src/a.mjs`';
const step1Fields = (text) => schema.parseStepFields(text.split('### Step 1:')[1].split('### Step 2:')[0]);
test('1.9: a Files list wrapped after a comma names both files and no issue', () => {
const r = entryPaths(WRAPPED);
assert.deepEqual(r.files, ['tests/a.test.mjs', 'src/a.mjs']);
assert.deepEqual(r.issues, []);
const v = validatePlanContent(plan19(WRAPPED));
assert.deepEqual(pathIssues(v.errors), [], JSON.stringify(v.errors));
// An issue keeps the entry's first line number.
const bad = entryPaths('- `src/a.mjs`\n- the CLI,\n `src/b.mjs`');
assert.deepEqual(bad.files, ['src/a.mjs']);
assert.deepEqual(bad.issues.map((i) => [i.code, i.line]), [['PLAN_STEP_PATH_NOT_FIRST', 2]]);
});
test('1.9: a step whose wrapped Files list names its source is not tests-only', () => {
const body = `- **Files:** ${WRAPPED}\n- **Changes:** x\n- **Test first:**\n - File: \`tests/a.test.mjs\` (new)\n - Verifies: x\n`;
assert.deepEqual(schema.stepTestsOnly(body, '1.9'), { testsOnly: false, files: ['tests/a.test.mjs', 'src/a.mjs'] });
});
test('1.9: K6 plans the second file of a wrapped Files list', () => {
const text = buildPlan({ version: '1.9', steps: [
{ filesRaw: WRAPPED, test: 'tests/a.test.mjs', manifestPaths: ['tests/a.test.mjs'] },
{ endToEnd: 'bash tests/e2e.sh' },
] });
assert.ok(text.includes(`- **Files:** ${WRAPPED}\n`), 'the builder writes the raw Files value');
assert.ok(plannedPaths(text).includes('src/a.mjs'), JSON.stringify(plannedPaths(text)));
});
test('1.9: a numbered File line (`1.` and `1)`) names its test - no PLAN_STEP_NO_TEST_FIRST', () => {
for (const line of [' 1. File: `tests/mod1.test.mjs` (new)', ' 1) File: `tests/mod1.test.mjs` (new)']) {
const text = planTestFirst19('1.9', line);
assert.ok(text.includes(`${line}\n`), 'the builder writes the raw File line');
assert.deepEqual(schema.testFirstFiles(step1Fields(text), '1.9'), ['tests/mod1.test.mjs'], line);
const r = validatePlanContent(text, { strict: true });
assert.equal(r.errors.find((e) => e.code === 'PLAN_STEP_NO_TEST_FIRST'), undefined, `${line}: ${errCodes(r).join(',')}`);
}
});
test('1.9: a wrapped File line keeps its second test; a wrapped description stays description', () => {
const wrapped = planTestFirst19('1.9', ' - File: `tests/mod1.test.mjs`,\n `tests/mod1b.test.mjs` (new)');
assert.deepEqual(schema.testFirstFiles(step1Fields(wrapped), '1.9'), ['tests/mod1.test.mjs', 'tests/mod1b.test.mjs']);
const described = planTestFirst19('1.9', ' - File: `tests/mod1.test.mjs` - covers\n `src/parse.mjs` and the CLI');
assert.deepEqual(schema.testFirstFiles(step1Fields(described), '1.9'), ['tests/mod1.test.mjs']);
});
test('1.9: a Verifies item that wraps onto a backticked source path gives only the File line\'s test', () => {
const text = planTestFirst19('1.9',
' - File: `tests/mod1.test.mjs` (new)\n - Verifies: the parser in\n `src/mod1.mjs` reads the CLI options');
assert.deepEqual(schema.testFirstFiles(step1Fields(text), '1.9'), ['tests/mod1.test.mjs']);
});
test('1.9: two unmarked File lines in a row give both tests', () => {
const text = planTestFirst19('1.9', ' File: `tests/mod1.test.mjs`\n File: `tests/mod1b.test.mjs`');
assert.deepEqual(schema.testFirstFiles(step1Fields(text), '1.9'), ['tests/mod1.test.mjs', 'tests/mod1b.test.mjs']);
});