👷 ci: derive test:node and test:bun scope - #196
Merged
Conversation
Closes #144. Both scripts named individual test files. That list dates from 9db5eae and has only ever been path-renamed since, so the Node and Bun jobs covered less of the repository with every test added: 19 of 105 files, 241 of the assertions Deno ran. Scope is now derived the way `deno task test` derives its own. Discovery walks each workspace member's `tests` directory through the existing `listWorkspacePaths`, so a new `*.test.ts` joins all three runtime jobs without editing anything. Walking concrete `tests` directories rather than the repository root keeps nested worktrees, `site`, and the deliberately malformed `scripts/tests/fixtures` out without naming them. Measured the whole corpus individually under both runtimes rather than trusting the counts in #144, which were taken at 81 files. Of 104 discovered, 101 ran unchanged under each. Of the three that did not: - build-npm and cli-npm-bin test the dnt toolchain and call Deno.execPath() / Deno.readTextFileSync. They are a real boundary, and are excluded. - bump-version failed only because scripts/lib/bump-version.ts imported npm:zod@^4. That is an import specifier, not a runtime boundary, so it is fixed rather than excluded: zod is already a root dependency and packages/cli/deno.json already maps it. Two exclusions remain. Each names a discovered file, says why, and links the issue where the boundary is argued. The integrity test checks that structure; its module comment is explicit that it cannot show an excluded test has become portable, because nothing re-runs excluded files. The launcher lives in scripts/lib/ with a thin entrypoint over it, matching bump-version, so the exit-code mapping can be asserted without importing a module that calls main() and exits. tsconfig.node.json gains acp, cli and test-agent — 58 files typechecked for the first time, with no new errors — plus the five portable runtime-test scripts. Not scripts/**/*.ts: that would pull in the malformed fixtures and the Deno-only build tools. Execution and typechecking stay separate axes. Both runtimes now run 1127 tests across 103 files, 0 failures.
PR #196: 👷 ci: derive test:node and test:bun scope13 files, +425 / -24 Scope🟡 449 lines changed. PRs under 400 receive more thorough review. 🟡 PR mixes config and source changes. Structural✅ No structural bloat detected. Slop✅ Slop indicators look low. Static Analysis✅ Oxlint found no issues. CorrectnessNo extraneous code patterns detected. |
`relativeTo` matched a URL pathname inside a walked filesystem path. Those agree on POSIX and disagree on Windows — `/C:/repo/tests/` never occurs inside `C:\repo\tests\a.test.ts` — so the function fell through to returning the whole absolute path while its contract promised a repository-relative one, and the launcher would have handed absolute paths to the runner. It now compares `toPath(tests)` against the walked path with separators normalized, requires the directory as a boundary-aware prefix so a sibling like `tests-extra/` cannot match, and throws rather than returning an absolute path when an entry falls outside. Covered by four cases including Windows-style input, and the repository-relative assertion now rejects a drive letter. Also corrects two claims that promised more than the code delivers: AGENTS.md said "a new test file anywhere" when discovery covers `tests/` beneath workspace members plus `scripts/tests/`, and the integrity suite's preamble claimed to catch an exclusion outliving its reason, which it cannot — the manifest already says so. Drops comments restating the names beneath them, and restores the literal em dash in the package description that a JSON rewrite had escaped.
test-bun failed while test-node and test-deno passed, on one root cause: the Bun job runs `bun install` alone, and that links only the workspace packages the root manifest depends on. `@executablemd/test-support` was the sole entry, added by #188 for exactly this reason, so nothing else appeared under node_modules/@executablemd. Every failure followed from it. Eval blocks compile to `.xmd-eval/<uuid>.ts` beside node_modules and could not resolve `@executablemd/core`, which is why the provider, sample and agent suites reported ERROR in their rendered output rather than failing an assertion outright. It passed locally because pnpm links all nine, so the pnpm and Bun layouts disagreed and only CI ran the Bun one. Declaring the packages at the root makes `bun install` produce the same layout: verified by moving node_modules aside, running `bun install` alone, and getting all nine links and 1131 passing tests where the same layout previously failed 45. The old hand-picked `test:bun` list never reached these suites, so the gap existed before this branch and only became visible once the scope was derived.
The last three Bun failures were all `this test timed out after 5000ms`. Bun caps each test at 5s by default; Deno and Node impose no comparable limit, so nothing in the corpus was written for it. The suites that failed spawn CLI subprocesses and real test-agent workers, and carry their own timeboxes — 15s to 180s — chosen for exactly that work. Bun's cap preempted them, killing the child and leaving `result.code` null rather than the exit status the assertion expected. Raising it past the longest leaves each suite's own timeout in charge, which is what the other two runners already do. This is a runner default that does not suit the corpus, not a defect in the tests.
The module comment claimed discovery covers every *.test.ts in the repository. It covers tests/ beneath each workspace member plus scripts/tests/ — the boundary AGENTS.md already states, and the one that keeps nested worktrees and the malformed fixtures out.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #144.
Why
test:nodeandtest:bunnamed individual test files. That selection datesfrom
9db5eaeand has only ever been path-renamed since, so the Node and Bunjobs covered less of the repository with every test added — 19 of 105 files,
and 241 of the assertions Deno ran. Nothing signalled whether a new test was
runtime-portable, so the gap widened silently.
What changes
Scope is derived the way
deno task testderives its own (#140/#142). A new*.test.tsruns under all three runtimes by default; staying out of one is adeliberate, reviewable act.
Before: 19 files under Node and Bun, hardcoded.
After: 1131 tests across 103 files, 0 failures, under each.
The measured corpus
I measured every discovered file individually under both runtimes rather than
trusting #144's counts, which were taken at 81 files.
tsx --tsconfig tsconfig.node.json --test)bun test)The same three failed on both:
scripts/tests/build-npm.test.tsDeno.execPath(); subject is the dnt buildscripts/tests/cli-npm-bin.test.tsDeno.readTextFileSync; builds the npm package with dntscripts/tests/bump-version.test.tsscripts/lib/bump-version.tsimportednpm:zod@^4bumpManifestsis pure manifest manipulation and its five tests are portable —only the import specifier was Deno-only.
zodis already a root dependency andpackages/cli/deno.jsonalready maps it, so that is a one-line fix, not aboundary. Excluding it would have recorded an artifact as if it were a runtime
limitation, and the integrity test could never have told the two apart later.
Two measurement artifacts I ruled out, recorded so nobody re-derives them as
defects: a first sweep failed all seven CLI-launching suites under Node because
my harness invoked
./node_modules/.bin/tsxdirectly, leavingtsxoffPATHfor the child processes those suites spawn; and
worker-lifecyclehit a 300 scap under three-way parallelism while passing 11/11 in isolation.
How it works
scripts/lib/test-files.ts—listTestFiles(root)reuses the existinglistWorkspacePaths, so discovery follows the same workspace contract asbuild-npm,bump-versionandgen-publish-workflow. Walking concretetestsdirectories rather than the repository root keeps nested worktrees,site, andscripts/tests/fixturesout without naming them. Returns sortedrepository-relative POSIX paths.
scripts/runtime-test-exclusions.ts— the one place a test opts out.Each entry names a discovered file, says why, and links the issue.
scripts/runtime-tests.ts— an Effection entrypoint overscripts/lib/runtime-tests.ts, matching thebump-versionsplit. That is notdecoration: it lets the exit-code mapping be asserted without importing a
module that calls
main()and exits.Both the launcher and the integrity test call
listTestFiles. Discovery logicexists once.
Two Bun-specific changes come with the widened scope, both pre-existing gaps the
19-file list never reached:
@executablemd/*workspace as a devDependency.Bun's isolated install links only what the root manifest depends on, and
generated
.xmd-eval/<uuid>.tsmodules execute from the repository root — sothey need those packages declared there to resolve. ♻️ refactor: own cross-runtime test infrastructure as @executablemd/test-support #188 had already added
@executablemd/test-supportfor the same reason; this completes the set.--timeout=300000. Bun caps each test at 5s bydefault where Deno and Node impose no comparable limit. Suites that spawn a
CLI or a worker carry their own 15–180s timeboxes, and the cap preempted them;
the ceiling lets those govern.
What must stay true
test:nodeandtest:bunenumerate no test files.carries a reason and an issue — checked by
scripts/tests/runtime-exclusions.test.ts.crashed runner cannot pass CI.
tsconfig.node.jsonlists only the portable runtime-test scripts.What the integrity test cannot do, stated in the manifest's module comment:
it detects invalid, renamed, deleted or malformed exclusions, but nothing
re-runs excluded files, so it cannot show that one has become portable.
Removing a stale entry stays a manual act.
How to verify it
Four repository checks with the CI-pinned Deno 2.9.1: lint 0 errors,
typecheck clean, 148 passed / 0 failed, JSR
Success. Plus:Both runs print each exclusion with its reason and issue before starting, so
the gap appears in the CI log rather than reading as full coverage.
Scope
Included
bump-versionspecifier fix, which is what keeps the exclusion list togenuine boundaries.
tsconfig.node.json:packages/acp,packages/cli,packages/test-agentplus their
paths, and the five portable runtime-test scripts. Notscripts/**/*.ts— that would pull in the deliberately malformed fixtures andthe Deno-only build tools.
Intentionally unchanged
.github/workflows/ci.yml. Both jobs already call the scripts; Node 22 andthe pinned Bun handle the widened argument lists — confirmed by both jobs
running all 103 files.
packages/test-supportruntime-detection exemption from ♻️ refactor: own cross-runtime test infrastructure as @executablemd/test-support #188.scripts/build-npm.tskeeps itsnpm:/jsr:specifiers — it is genuinelyDeno-only.
Risks and limitations
packages/test-agent/tests/worker-lifecycle.test.tsuses realsleep()callsfor cancellation timing. It passes in both derived runs here, but it timed out
under artificial parallelism during measurement, so it is the first place to
look if the widened jobs flake on slower CI hardware. That would be a timing
problem to fix, not grounds for an exclusion.
tsxonPATH;pnpmsupplies it, and ✅ test: launch the CLI through the host runtime #186'slauncher already carries the same requirement.
Scope confirmation
7a365b9: 8 modified files and 5 new, 13 in total; 1131 tests across 103files under Node and Bun; 2 exclusions.