Skip to content

refactor(tools): read test code with the TypeScript parser in the async test-cleanup invariant - #637

Merged
mrgoonie merged 2 commits into
mainfrom
refactor/634-tokenizer-test-cleanup-invariant
Oct 8, 2026
Merged

mrgoonie merged 2 commits into
mainfrom
refactor/634-tokenizer-test-cleanup-invariant

Conversation

@mrgoonie

@mrgoonie mrgoonie commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #634. Refs #629, #632, #563.

What changed

tools/invariants/test-cleanup-retries-asynchronously.mjs no longer reads test code with a hand-written scanner. It parses each file with the TypeScript parser (the typescript dev dependency the repo already pins), choosing TypeScript, TSX or JavaScript with JSX by the file's extension.

  • Calls are found in the syntax tree: a call expression whose callee is rmSync or a name the file gives it, x.rmSync, x["rmSync"] / x[`rmSync`], optional calls, calls through parentheses or type assertions, and fs.rmSync.call(...) / .apply(...). The call asks for retries when maxRetries is named among its arguments, including as a quoted key.
  • Aliases come from the tree too: import specifiers, object destructuring, const name = ... and name = .... A mock factory's { rmSync: vi.fn() } object literal no longer counts as an alias.
  • The exemption marker still has to be in a comment on the call's line or the line above. Comments are found by walking every token's leading trivia. JSX text is never read as trivia, so a marker written as JSX text does not count. commentsOf(source, path?) exposes this classification.
  • Removed export: blankCommentsAndStrings is gone. Nothing used it since calls are found on the tree, and nothing outside the spec imported it.
  • Unparsable files: a file that does not parse is named in a warning: note, because error recovery may read code after the error differently from how it was written.
  • A fast path: files that never spell both rmSync and maxRetries are not parsed.

Skipped is reported as skipped

  • check(name) results now carry skipped.
    • formatReport() (in tools/invariants/context.mjs, used by tools/check-invariants.mjs) prints SKIP for a skipped check.
    • The summary then reads 14 invariant check(s) passed, 1 skipped instead of all 15 invariant checks passed.
    • A check with failures is FAIL even if it also said it skipped.
  • When the check skips: only when dependencies are not installed at all, meaning the error is ERR_MODULE_NOT_FOUND for typescript itself and there is no node_modules at the repository root.
  • When it fails: a missing typescript with node_modules present, any other missing module, or any other load error. The failure carries the error message.
  • CI: .github/workflows/ci.yml gets a second step, Repository invariants after install, right after Install and under the same if:. The pre-install step stays for prose-only changes.

A fresh worktree with no install was run end to end:

SKIP  test-cleanup-retries-asynchronously
      · dependencies are not installed, so the TypeScript parser is not there; CI runs this check again after install
14 invariant check(s) passed, 1 skipped

The four cases from the #632 review, each with a unit test

Case main this PR
Regex literal holding a backtick and a quote (/say: `Đã /u, /it's "(/g), then a call call missed found
String right after a keyword (return'(', case'(', typeof'(') calls missed found
Bracket access across a line break (fs[\n"rmSync"], fs?.[\n`rmSync`\n]) missed found
Marker written as JSX text (<p>// invariant-allow: ...</p>), or inside a regex honoured not honoured; {/* ... */} inside JSX still is

There are also tests for:

  • reading a .ts file as TypeScript and a .jsx file as JSX;
  • (await import("node:fs")).rmSync(...);
  • .call/.apply;
  • the parse warning.

Agreement with the parser

The test agrees with the TypeScript parser on where the comments are, in every file it reads covers every file the invariant scans (708 today). Comments are what the marker depends on.

  • What it compares: commentsOf character by character against an independent AST view. That view takes getLeadingCommentRanges/getTrailingCommentRanges at every node and node-list edge, and excludes JSX text.
  • Result: 0 disagreements.
  • Mutation check: a deliberately broken comment walk, one that ignores comments before }, is caught.
  • Runtime: about 3.2 s, down from about 5–6 s for the earlier whole-code comparison. It keeps an explicit 60 s timeout.

The repository-wide test finds no retrying synchronous removal in the repository's test code also stays.

For the record, from round 1: the earlier whole-code version of this comparison, run against main's scanner, found real code blanked in 32 test-scoped files. Among them were apps/runtime/test/action-speech.spec.ts:94, packages/contracts/test/activity-timeline.spec.ts:198 and tools/smoke-widget-tooling.mjs:55.

Detection compared with main, as a one-off script over all 1528 source files under apps packages packs tools examples: 0 files differ in the reported lines. There are no new false positives.

Performance

node tools/check-invariants.mjs, whole tree, Windows 11, three runs each:

run 1 run 2 run 3
main 1630 ms 1473 ms 1585 ms
this PR 1306 ms 1193 ms 1217 ms

Verification

  • tools/test/test-cleanup-retries.spec.ts + tools/test/invariants-context.spec.ts: 33/33 pass.
    • The new tests for the four cases, extensions and comment agreement fail against main's module.
    • The skip and fail tests run a copy of the check in a separate Node in three states: no node_modules, an empty node_modules, and a typescript package that throws on load. The formatReport tests cover the SKIP, FAIL and summary lines. Neither the exports nor the skipped field exist on main.
  • pnpm typecheck: exit 0.
  • eslint on the 5 changed JS/TS files: exit 0.
  • pnpm invariants: all 15 pass with dependencies installed. In a fresh worktree with no install: 14 passed, 1 skipped, exit 0.
  • .github/workflows/ci.yml parses with yaml with no errors. The verify job's steps are Repository invariants, Install [if], then Repository invariants after install [if].
  • pnpm verify: exit 0, 7797 tests passed.

Docs impact

None. This is an internal invariant and its runner. No doc in docs/ or on the official site describes them.

…nc test-cleanup invariant

The hand-written scanner lost sync on regular expression literals holding a quote or backtick, on a string right after a keyword, on bracket access split across a line break, and took the exemption marker in JSX text for a comment. The check now parses each file by its extension and finds rmSync calls, aliases and the marker from the syntax tree. A test holds the classification of code to the parser across every file the check reads, and runs the check over the repository after install, since CI runs the invariants before it.
…nts the marker relies on

The runner now prints SKIP for a check that could not run and counts it apart from the passes. The async test-cleanup check skips only when dependencies are not installed at all; any other failure to load the parser fails it. CI runs the invariants again after install. The parser-agreement test now covers the comments the exemption marker is read from, and the unused code-blanking export is gone. Calls through call and apply are found, and a file that does not parse is named in a note.
@mrgoonie

mrgoonie commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

Review attestation: ready to merge at eca2c89892012982e3fb1c9425259ce902f39aec, reviewed by agent:code-reviewer.

A push to this PR makes this attestation stale; the new head needs its own review.

@mrgoonie
mrgoonie enabled auto-merge (squash) October 8, 2026 03:04
@mrgoonie
mrgoonie merged commit d9b6572 into main Oct 8, 2026
41 of 42 checks passed
@mrgoonie
mrgoonie deleted the refactor/634-tokenizer-test-cleanup-invariant branch October 8, 2026 03:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Read test code with a real tokenizer in the async test-cleanup invariant

1 participant