fix(plan): below 1.9 a test file after a note, and, plus or + is read

Operator decisions 28 (13.10) and 31 (14.10); review finding bd2520b0.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-10-14 10:44:47 +02:00
commit 8e6229b01d
Signed by: ktg
SSH key fingerprint: SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
2 changed files with 122 additions and 6 deletions

View file

@ -637,6 +637,11 @@ const DOC_RE = /\.(md|markdown|mdx|txt|rst|adoc)$/i;
const DATA_RE = /\.(ya?ml|json|toml|xml|csv)$/i; const DATA_RE = /\.(ya?ml|json|toml|xml|csv)$/i;
const UNDER_TESTS_RE = /(^|\/)(tests?|__tests__)\//; const UNDER_TESTS_RE = /(^|\/)(tests?|__tests__)\//;
/** A path that is clearly a test: a test by its path, and under tests/ or neither a document nor a data file. */
function clearlyTest(p) {
return isTestPath(p) && (UNDER_TESTS_RE.test(p) || !(DOC_RE.test(p) || DATA_RE.test(p)));
}
/** /**
* Whether every file in a step's `Files:` is a test: the step's own `Test * Whether every file in a step's `Files:` is a test: the step's own `Test
* first:` file, or a path that is clearly a test (lib/util/test-path.mjs). Such * first:` file, or a path that is clearly a test (lib/util/test-path.mjs). Such
@ -662,6 +667,52 @@ const TEST_FIRST_FILE_LINE = /\bFile:\**[ \t]*(?=`)/gi;
// From plan_version 1.9: a line that opens with `File:` (after an optional // From plan_version 1.9: a line that opens with `File:` (after an optional
// list marker of the set LIST_ITEM accepts, and `**`), backticked or not. // list marker of the set LIST_ITEM accepts, and `**`), backticked or not.
const TEST_FIRST_FILE_LINE_19 = /^\s*(?:(?:[-*+]|\d+[.)])\s+)?(?:\*\*)?File:(?:\*\*)?/i; const TEST_FIRST_FILE_LINE_19 = /^\s*(?:(?:[-*+]|\d+[.)])\s+)?(?:\*\*)?File:(?:\*\*)?/i;
// Below plan_version 1.9: one glue token at the start of the rest of a `File:`
// run (operator decisions 28, 13.10, and 31, 14.10): whitespace or a comma
// (group 1); a note in brackets holding no bracket, backtick or newline,
// optionally in one `*` or `_` (`(new)`, `*(new)*`, `(existing, the parser)`);
// the whole word `and` or `plus`, any case (`android` is no glue); or `+`.
const OLDER_GLUE = /^(?:(\s+|,)|[*_]?\([^()`\n]*\)[*_]?|(?:and|plus)(?!\w)|\+)/i;
/**
* The files of one below-1.9 `File:` run, read from just after the marker:
* the leading backticked span, then each span after one or more glue tokens
* (OLDER_GLUE). While every glue token so far was whitespace or a comma, a
* span with the shape of a path is kept and any other span is skipped
* (operator decision 20). Once a glue token was a note or a word, a span is
* kept only when it has the shape of a path and is clearly a test, and the
* first span that is not ends the run (operator decisions 28 and 31). Any text
* that is neither a span nor glue ends the run.
* @param {string} text - the line after one `File:` marker
* @returns {string[]} the kept spans, trimmed, not normalised
*/
function olderFileRun(text) {
const out = [];
let rest = String(text ?? '');
let loose = false;
let m = LEADING_SPAN.exec(rest);
while (m) {
const s = m[1].trim();
if (!loose) {
if (tickedPath(s) || BUILD_FILE.test(s)) out.push(s);
} else if (tickedPath(s) && clearlyTest(norm(s))) {
out.push(s);
} else {
break;
}
rest = rest.slice(m[0].length);
let glued = false;
let g = OLDER_GLUE.exec(rest);
while (g) {
glued = true;
if (g[1] === undefined) loose = true;
rest = rest.slice(g[0].length);
g = OLDER_GLUE.exec(rest);
}
m = glued ? LEADING_SPAN.exec(rest) : null;
}
return out;
}
/** /**
* plan_version 1.9: the rest of each `Test first:` `File:` line (after `File:` * plan_version 1.9: the rest of each `Test first:` `File:` line (after `File:`
@ -704,13 +755,18 @@ export function testFirstFileEntries(fields) {
* Below plan_version 1.9 (or without a version) a line counts as that line * Below plan_version 1.9 (or without a version) a line counts as that line
* exactly when TEST_FIRST_FILE_LINE matches it. Each `File:` marker on the * exactly when TEST_FIRST_FILE_LINE matches it. Each `File:` marker on the
* line opens a run: the backticked spans that stand right after it, separated * line opens a run: the backticked spans that stand right after it, separated
* only by commas or whitespace (leadingSpans), and every span in that run with * only by commas or whitespace (olderFileRun), and every span in that run with
* the shape of a path is a test file (a call `parse()`, an option, a URL is * the shape of a path is a test file (a call `parse()`, an option, a URL is
* skipped and the run goes on). A path in the description after the run is no * skipped and the run goes on). A path in the description after the run is no
* test file (operator decision 20, 12.10; review `6c31212c`: "covers `parse()` * test file (operator decision 20, 12.10; review `6c31212c`: "covers `parse()`
* in `src/parser.mjs`" made the source a test and the step tests-only). The * in `src/parser.mjs`" made the source a test and the step tests-only). The
* markers are matched globally because 6.3.1 read the first span after each * markers are matched globally because 6.3.1 read the first span after each
* `File:`, so a line with two markers keeps at least 6.3.1's files. * `File:`, so a line with two markers keeps at least 6.3.1's files. Below 1.9
* the run also goes on past a note in brackets (`(new)`, `*(new)*`,
* `(existing ...)`) and the words `and`, `plus` and `+`, to a further path
* only when it is clearly a test, and a source path there ends it
* (olderFileRun; operator decisions 28 and 31, 13.10 and 14.10; review
* `bd2520b0`; review `6c31212c` for the guard).
* *
* From 1.9 a line counts when it opens with `File:` (testFirstFileEntries), * From 1.9 a line counts when it opens with `File:` (testFirstFileEntries),
* backticked or not, after an optional list marker (`1.` and `1)` included); * backticked or not, after an optional list marker (`1.` and `1)` included);
@ -739,8 +795,7 @@ export function testFirstFiles(fields, version) {
} }
for (const line of String(fields.get('Test first').text ?? '').split('\n')) { for (const line of String(fields.get('Test first').text ?? '').split('\n')) {
for (const at of line.matchAll(TEST_FIRST_FILE_LINE)) { for (const at of line.matchAll(TEST_FIRST_FILE_LINE)) {
for (const s of leadingSpans(line.slice(at.index + at[0].length))) { for (const s of olderFileRun(line.slice(at.index + at[0].length))) {
if (!(tickedPath(s) || BUILD_FILE.test(s))) continue;
const p = norm(s); const p = norm(s);
if (!out.includes(p)) out.push(p); if (!out.includes(p)) out.push(p);
} }
@ -756,8 +811,7 @@ export function testFirstFiles(fields, version) {
*/ */
function stepTestFile(f, version) { function stepTestFile(f, version) {
const testFirst = testFirstFiles(f, version); const testFirst = testFirstFiles(f, version);
return (p) => testFirst.includes(p) || return (p) => testFirst.includes(p) || clearlyTest(p);
(isTestPath(p) && (UNDER_TESTS_RE.test(p) || !(DOC_RE.test(p) || DATA_RE.test(p))));
} }
/** `stepTestsOnly` for a caller that already holds the step's parsed fields. */ /** `stepTestsOnly` for a caller that already holds the step's parsed fields. */

View file

@ -13,6 +13,7 @@ import { dirname, join } from 'node:path';
import { fileURLToPath } from 'node:url'; import { fileURLToPath } from 'node:url';
import * as m from '../../lib/parsers/plan-schema.mjs'; import * as m from '../../lib/parsers/plan-schema.mjs';
import { validatePlanContent } from '../../lib/validators/plan-validator.mjs'; import { validatePlanContent } from '../../lib/validators/plan-validator.mjs';
import { isTestPath } from '../../lib/util/test-path.mjs';
import { buildPlan } from '../helpers/plan-v18-builder.mjs'; import { buildPlan } from '../helpers/plan-v18-builder.mjs';
const HERE = dirname(fileURLToPath(import.meta.url)); const HERE = dirname(fileURLToPath(import.meta.url));
@ -187,6 +188,67 @@ test('decision 20: a 1.8 line with two File: markers, each followed by a test, g
assert.deepEqual(testFirstFiles(body, null), ['tests/a.test.mjs', 'tests/b.test.mjs']); assert.deepEqual(testFirstFiles(body, null), ['tests/a.test.mjs', 'tests/b.test.mjs']);
}); });
// ---- operator decisions 28 (13.10) and 31 (14.10), review bd2520b0 ---------
// Below 1.9 the run after `File:` also goes on past a note in brackets
// (`(new)`, `*(new)*`, `(existing ...)`) and the whole words `and`, `plus` and
// `+`, to a further path only when it is clearly a test; a path that is not
// ends the run, and so does any other text. At 1.9 nothing changes.
const OLDER = ['1.8', '1.7', null];
const fileLine = (rest) => step('`tests/a.test.mjs`, `tests/b.test.mjs`', [` - File: ${rest}`, ' - Verifies: x']);
test('decisions 28 and 31: below 1.9 a test after a note, and, plus or + is read', () => {
const lines = [
'`tests/a.test.mjs` (new), `tests/b.test.mjs` (new),',
'`tests/a.test.mjs` and `tests/b.test.mjs`',
'`tests/a.test.mjs` (existing) + `tests/b.test.mjs`',
'`tests/a.test.mjs` (existing, the parser) plus `tests/b.test.mjs`',
'`tests/a.test.mjs` *(new)*, `tests/b.test.mjs`',
];
for (const rest of lines) {
for (const v of OLDER) {
assert.deepEqual(testFirstFiles(fileLine(rest), v), ['tests/a.test.mjs', 'tests/b.test.mjs'], `version ${v}: ${rest}`);
}
}
});
test('decisions 28 and 31: below 1.9 a near miss gives only the first test', () => {
// Both read as tests by lib/util/test-path.mjs alone (a specs/ folder); the
// run refuses them all the same: a document or a data file outside tests/.
assert.equal(isTestPath('docs/specs/api.md'), true);
assert.equal(isTestPath('api/specs/openapi.yaml'), true);
const lines = [
'`tests/a.test.mjs` (new) - covers `src/a.mjs`',
'`tests/a.test.mjs` (new), `src/a.mjs`',
'`tests/a.test.mjs` (new) or `tests/b.test.mjs`',
'`tests/a.test.mjs` (new), covers `tests/b.test.mjs`',
'`tests/a.test.mjs` (new) and `src/a.mjs`',
'`tests/a.test.mjs` (new) and `https://example.invalid/tests/b.test.mjs`',
'`tests/a.test.mjs` (new) and `docs/specs/api.md`',
'`tests/a.test.mjs` (new), `api/specs/openapi.yaml`',
'`tests/a.test.mjs` android `tests/b.test.mjs`',
'`tests/a.test.mjs` andand `tests/b.test.mjs`',
'`tests/a.test.mjs` plusand `tests/b.test.mjs`',
];
for (const rest of lines) {
for (const v of OLDER) {
assert.deepEqual(testFirstFiles(fileLine(rest), v), ['tests/a.test.mjs'], `version ${v}: ${rest}`);
}
}
});
test('decisions 28 and 31: below 1.9 the run after a note stops at the first source path', () => {
const rest = '`tests/a.test.mjs` (new), `tests/b.test.mjs`, `src/a.mjs`, `tests/c.test.mjs`';
for (const v of OLDER) {
assert.deepEqual(testFirstFiles(fileLine(rest), v), ['tests/a.test.mjs', 'tests/b.test.mjs'], `version ${v}`);
}
});
test('decisions 28 and 31: at 1.9 the two-test line with notes still gives only the first test', () => {
const body = fileLine('`tests/a.test.mjs` (new), `tests/b.test.mjs` (new),');
assert.deepEqual(testFirstFiles(body, '1.9'), ['tests/a.test.mjs']);
});
// ---- one definition in the prose (plan step 11, review cf241526) ----------- // ---- one definition in the prose (plan step 11, review cf241526) -----------
// The executor and the step helper define the step's test files once, as // The executor and the step helper define the step's test files once, as
// decision 20 reads them; every other place points at that definition. Red // decision 20 reads them; every other place points at that definition. Red