Skip to content

refactor(smoke): discover test files instead of registering them - #2406

Merged
chubes4 merged 4 commits into
mainfrom
refactor/2402-test-discovery
Aug 29, 2026
Merged

refactor(smoke): discover test files instead of registering them#2406
chubes4 merged 4 commits into
mainfrom
refactor/2402-test-discovery

Conversation

@chubes4

@chubes4 chubes4 commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator

Closes #2402 item 4.

The smoke manifest enumerated 128 commands by hand. A test ran only if someone remembered to register it — which is how #2402 found 153 test: scripts that no gate invoked, and why package.json absorbs 15% of all commits acting as the de facto test registry.

This replaces enumeration with discovery.

What changes

scripts/smoke-discovery.ts finds tests/*.test.{ts,mjs} and scripts/*-smoke.{ts,php} by convention. Adding a test file is now enough to make it run. Anything that must not run is listed in DISCOVERY_EXCLUSIONS with a stated reason, so exclusion is an explicit, reviewable act rather than the silent default.

The manifest keeps only work that is not a single test file:

kept why
build tsc -b for runtime-core/playground/cli, plus the CLI bin and provenance steps
test:cloudflare-runtime carries tsc -p packages/runtime-cloudflare --noEmit

scripts/smoke-manifest.ts drops from 198 lines and 128 entries to 50 lines and 2.

Coverage is verified, not assumed

The risk in this change is silently losing coverage. Measured against the old manifest:

files covered by the OLD manifest: 147
of those, NOT in the discovered set: 2
   scripts/ensure-cli-bin-executable.mjs
   scripts/write-cli-build-provenance.ts

Both are build helpers that npm run build still runs. 145 of 147 are discovered directly; the remaining 2 are covered by the retained build command. Nothing is lost, and discovery adds 212 files that were never gated.

Concurrency, because 357 files sequentially is 30 minutes

Discovered files are independent processes, so the discovered phase runs at concurrency 8 and captures output per file, reporting every failure rather than stopping at the first. The old runner failed fast, which is why #2402 needed an out-of-tree harness to produce a full picture.

Five files contend on the shared Playground WordPress archive cache and fail when run alongside each other. They are correct in isolation, so rather than excluding them they run in a serial phase afterwards, listed in DISCOVERY_SERIAL. Verified: all five pass serially in 90s total.

Measured

npm run check
# [smoke] check: 2 declared, 352 discovered (concurrency 8), 5 serial
# [smoke] check passed: 359 command(s)
# real 7m59.321s
before after
manifest lines 198 50
manifest entries 128 2
files executed 147 357
wall time ~5 min ~8 min

143% more files for about three more minutes.

Exclusions

14 total, each with a reason in the source. Three categories:

  • Environment (3): need Docker, exceed the per-file budget, or need the 427 MB plugin zip. All already run in the agent-task-contracts workflow or the Homeboy gate.
  • The runner itself (1): scripts/run-smoke.ts matches *-smoke.ts and would recurse.
  • Failing and unmaintained (10): added June 2026, never wired to a gate, untouched since. Discovery is what surfaced them.

Three of those ten fail for one shared root cause worth recording: packages/runtime-playground/src/query-recorder.ts:80 emits PHP inside a JS template literal containing \0 and \x0B, which JavaScript interprets as actual control characters. The generated PHP therefore carries a raw NUL byte, and Node refuses to pass it as an argv argument. The string value PHP ends up with is the same either way, so this is a portability defect rather than a behavior bug, but it blocks any harness that shells out with php -r.

I have deliberately not fixed those ten here. This PR should be evaluated on whether discovery is correct, not bundled with ten unrelated repairs. They are #2402 item 5.

Update: four CI rounds to converge

Local runs passed on the first try. CI did not, and the gap was instructive — my machine has enough cores to hide the problems. Each round found a distinct class:

round failures cause fix
1 19 prepare-declaration-rebuild deletes runtime-core/dist and runs npm install mid-run, breaking every concurrent import of @automattic/wp-codebox-core excluded as destructive to shared state
2 2 browser + Playground tests starved at concurrency 8 on a 2-core runner moved to the serial phase
3 1 cancellation-timing test, same starvation class moved to the serial phase
4 1 native-mariadb-runtime-service lost the ordering test:runtime-services gave it chains restored, their files excluded from discovery

Round 4 corrected a design mistake. My original analysis flagged four commands as must-keep; I overrode two of them because their member files were individually discoverable. Being discoverable is not the same as being safe standalone or concurrently. test:generic-primitives and test:runtime-services are back as declared commands, and the 22 files they own are excluded from discovery so nothing runs twice.

Round 1 was worth the trip on its own: a test that deletes shared build output while other tests import from it is a real defect, and it was invisible while the suite ran sequentially.

Final CI: smoke aggregate green in 11m16s.

Final shape

before after
manifest lines 198 55
manifest entries 128 4
files executed 147 356
CI wall time n/a (not run) 11m16s

Coverage verified against origin/main: 145 of the 147 files the old manifest executed are still covered, and the other two are build helpers build still runs.

Exclusions total 15, each with a reason, plus 22 chain-owned files executed by their declared chain.

Follow-up

The 218 test: scripts in package.json are now largely redundant with discovery, but removing them is a separate diff with its own review surface — docs and operator habits reference them. Item 5 covers that alongside the ten excluded files.


AI assistance disclosure: authored by Claude (Sonnet 4.5) running in OpenCode, directed by @chubes4. The model probed all 226 previously-ungated files individually to establish their status, measured sequential vs concurrent execution to size the runtime, identified the five archive-cache contenders empirically and confirmed they pass serially, and proved the 145-of-147 coverage overlap before collapsing the manifest. All figures are measured. Reviewed by a human before opening.

The smoke manifest enumerated 128 commands by hand, and a test only ran if
someone remembered to add it. #2402 measured the result: 153 test scripts
that no gate invoked, and a package.json that absorbed 15 percent of all
commits acting as the de facto test registry.

Discover tests/*.test.{ts,mjs} and scripts/*-smoke.{ts,php} by convention.
Adding a test file is now enough to make it run. Anything that must not run
is listed in DISCOVERY_EXCLUSIONS with a reason, so exclusions stay short and
reviewable rather than being the silent default.

The manifest keeps only work that is not a single test file: 'build' for
tsc -b plus the CLI bin and provenance steps, and 'test:cloudflare-runtime'
for tsc -p packages/runtime-cloudflare --noEmit. It drops from 198 lines and
128 entries to 50 lines and 2.

Coverage is verified, not assumed. Of the 147 files the old manifest
executed, 145 are in the discovered set; the other two are build helpers that
'build' still runs. Discovery adds 212 files that were never gated.

Discovered files are independent processes, so they run at concurrency 8 and
report every failure instead of stopping at the first. Five files contend on
the shared Playground archive cache and run in a serial phase afterwards;
they are correct in isolation and are listed in DISCOVERY_SERIAL.

Measured: 359 commands over 357 files in 7m59s, against 128 commands over
147 files before. 143 percent more files for about three more minutes.
…el phase

CI surfaced two failure classes the local run did not.

tests/prepare-declaration-rebuild.test.ts deletes packages/runtime-core/dist
and runs npm install at the repository root. Under concurrency that removed
@automattic/wp-codebox-core/dist/index.js while 17 other files were importing
it, producing a burst of ERR_MODULE_NOT_FOUND. It is destructive to shared
state rather than merely order-sensitive, so it is excluded; the Homeboy gate
still runs it through test:prepare-declaration-rebuild.

tests/browser-actions-navigation-capture.browser.test.ts and
tests/editor-actions-save.integration.test.ts each boot a browser and a
WordPress runtime. On a two-core runner at concurrency 8 they were starved and
timed out, one after 75 seconds on savePost. Both move to the serial phase.

Local runs passed because the machine has enough cores to absorb the
oversubscription and the dist deletion window stayed narrow.
tests/browser-adaptive-exploration.test.ts asserts cancellation timing, so
CPU starvation makes it fail rather than merely run slower. Same class as the
two tests serialized in the previous commit.
tests/native-mariadb-runtime-service.test.ts failed on CI because dissolving
test:runtime-services lost the ordering its members rely on. Being individually
discoverable is not the same as being safe to run standalone or concurrently.

Restore test:generic-primitives and test:runtime-services alongside build and
test:cloudflare-runtime as declared commands, and exclude the 22 files those
chains own from discovery so nothing runs twice. This is what the original
must-keep analysis indicated; I overrode it for two chains on the grounds that
their files were discoverable, which was the wrong test.

Coverage unchanged: 145 of the 147 files the old manifest executed are covered,
and the remaining two are build helpers that 'build' still runs.

Discovery now contributes 334 files, 326 parallel and 8 serial.
@chubes4
chubes4 merged commit ad9a4c3 into main Aug 29, 2026
3 checks passed
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.

Homeboy gate runs 5 of 103 manifest commands; full aggregate is 4.7 min and green

1 participant