Skip to content

fix: restore CI test execution and repair CJS/CLI regressions from dependency bumps - #910

Open
kilisamemarisaaa wants to merge 6 commits into
fastify:mainfrom
kilisamemarisaaa:fix/restore-ci-and-cjs-interop
Open

fix: restore CI test execution and repair CJS/CLI regressions from dependency bumps#910
kilisamemarisaaa wants to merge 6 commits into
fastify:mainfrom
kilisamemarisaaa:fix/restore-ci-and-cjs-interop

Conversation

@kilisamemarisaaa

@kilisamemarisaaa kilisamemarisaaa commented Aug 28, 2026

Copy link
Copy Markdown

Summary

The test suite has been silently running zero tests in CI since glob was bumped 11 -> 13 (#852, Dec 2025): glob(pattern, cb) no longer accepts a callback (it returns a promise), so suite-runner.js never executed any test file and every CI run stayed green on an empty suite. That hollow green covered five separate regressions introduced by dependency bumps and migrations, all of which this PR fixes.

What was broken and why

  1. suite-runner.js ran nothing - glob(pattern, cb) no longer invokes the callback with glob v13. Evidence: CI logs showed every suite completing in about 0.1s with no test output, and c8 only reported suite-runner.js. Fix: await glob(pattern) inside an async main() and fail loudly on errors.

  2. ESM-only dependency upgrades broke CommonJS call sites - pkg-up v5 exposes a named pkgUp export, while chalk v6 and is-docker v4 expose default exports. Existing require() calls received module namespace objects, causing pkgUp is not a function, chalk.x is not a function, and the equivalent isDocker failure. Fix: select each package's actual export at the existing call sites.

  3. CLI parsing broke after the util.parseArgs migration (Replace yargs-parser with Node.js built-in util.parseArgs #887) - cli.js read argv._, which util.parseArgs().values never contains, so fastify --help and fastify <cmd> --help threw while reading splice; generate-swagger --yaml was also rejected because yaml was not registered. Fix: read the command from parsed.positionals[0] and register yaml in args.js.

  4. fastify generate-plugin emitted an empty "tstyche": {} - assigning into pkg.tstyche || {} when the template had no tstyche field created an empty object. Fix: only assign that field when the template provides it.

  5. TypeScript 6 could not resolve Node test globals in generated TypeScript suites - the inherited fastify-tsconfig does not declare a types list, and the restored suites failed with TS2591 for node:test and node:assert. Fix: declare "types": ["node"] in the two ts-node test configs.

  6. Node 26 registered the ts-node ESM loader twice in test workers - invoking the JavaScript runner itself with --loader ts-node/esm allowed the loader to be inherited and registered again by node:test workers, producing false TS7006 diagnostics for already-transpiled code. Fix: pass --loader=ts-node/esm only to TS-ESM workers through run({ execArgv }); ordinary suites omit the option and retain their existing inherited arguments.

The application-facing regressions in items 2-4 did not reach published versions (7.4.1/8.0.0 still use the earlier dependency set), but they currently break main and were hidden by item 1.

Verification

  • npm run lint passes.
  • TS-ESM suite: 3/3 passing on Node 22.23.2, 24.20.0, and 26.8.1.
  • npm run all-suites: 12/12 passing across CJS, ESM, TS-CJS, and TS-ESM on Node 22.23.2 / Windows.
  • Full Windows CLI baseline: 75 passing / 5 failing / 2 skipped. The failures are the existing Windows EBUSY and temporary-directory cleanup chain; they are outside this PR's diff.
  • Node 20 follows the repository's existing policy in should-skip-test-suites.js and skips generated-template suites; the normal CLI suite remains in the CI matrix.

Notes

  • First-time contributor - CI approval may be needed.
  • Six focused commits, one for each root cause above.

kilisamemarisaaa and others added 5 commits August 28, 2026 16:36
glob v13 removed the callback API: glob(pattern, cb) now returns a
promise and the callback never fires, so suite-runner silently ran zero
tests and CI stayed green since the glob 11 -> 13 bump (fastify#852).
Switch to async/await and fail loudly on glob errors.

Co-Authored-By: EvoX <evox@evomap.ai>
Dependabot bumps moved several runtime deps to ESM-only releases that
were never exercised because CI ran zero tests:
- pkg-up@5 (ESM named exports): require() returned a namespace, breaking
  `fastify start` with 'pkgUp is not a function'
- is-docker@4 (ESM default export): broke `fastify start` the same way
- chalk@6 (ESM default export): broke generate/generate-plugin and watch
Use named imports / .default interop at each require site.

Co-Authored-By: EvoX <evox@evomap.ai>
The util.parseArgs migration (fastify#887) left two breakages that CI never
caught because the test suite was silently no-op:
- cli.js read argv._ which util.parseArgs values never contains; --help
  and <cmd> --help crashed with 'Cannot read properties of undefined'
- generate-swagger --yaml was rejected by strict mode because yaml was
  not registered as a known option; register it in args.js (restores
  the lenient pre-migration behaviour)

Co-Authored-By: EvoX <evox@evomap.ai>
…des it

Object.assign(pkg.tstyche || {}, template.tstyche) emitted an empty
'tstyche': {} into generated plugin package.json because the plugin
template has no tstyche section (regression from the tsd -> tstyche
migration, fastify#886). Guard the assignment.

Co-Authored-By: EvoX <evox@evomap.ai>
Under TypeScript 6 the node:test / node:assert types in
templates/app-ts(-esm) tests no longer resolve (fastify-tsconfig does
not set a 'types' field and @types/node is not picked up
automatically), failing compilation with TS2591 — the first time these
suites actually ran since CI went hollow. Equivalent CLI flags compile
with 7 errors without --types node and 0 with it.

Co-Authored-By: EvoX <evox@evomap.ai>
@kilisamemarisaaa

Copy link
Copy Markdown
Author

CI note on the remaining red job (Test (26, macos-latest)): the two failing suites are the templates/app-ts-esm route tests (root.test.ts, example.test.ts). These suites never actually executed on main before this PR — restoring their execution is part of the point of #910 (the glob in suite-runner never awaited, so tests silently didn't run) — so the macos/node-26 failures are newly exposed, not introduced. The other OS jobs in that run were cancelled by fail-fast before producing results. If it helps, I can dig into the two route tests on node 26 / macos-26-arm64, or we can land this PR and track that combination in a separate issue so restored test execution isn't blocked on it.

Pass the ts-node ESM loader only to node:test workers. This avoids Node 26 registering an inherited loader twice while leaving ordinary suites' execArgv untouched.
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.

1 participant