Skip to content

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

Open
kilisamemarisaaa wants to merge 5 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 5 commits into
fastify:mainfrom
kilisamemarisaaa:fix/restore-ci-and-cjs-interop

Conversation

@kilisamemarisaaa

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 four separate regressions introduced by dependency bumps/migrations, all of which this PR fixes.

What was broken and why

  1. suite-runner.js ran nothingglob(pattern, cb) callback never fires with glob v13; run({ files: [] }) passed instantly with exit code 0. Evidence: CI logs show every suite completing in ~0.1s with zero test output, and c8 coverage only reported suite-runner.js itself. Fix: await glob(pattern) inside an async main(), failing loudly on glob errors.

  2. fastify start crashed with TypeError: pkgUp is not a functionpkg-up bumped to v5 (chore: bump the dependencies group with 3 updates #896, ESM-only, named exports) while util.js did const pkgUp = require('pkg-up'), receiving the module namespace. Fix: const { pkgUp } = require('pkg-up').

  3. fastify start / generate / watch crashed with chalk.x is not a functionchalk bumped to v6 (chore: bump chalk from 4.1.2 to 6.0.0 in the dependencies group #908, ESM-only, default export) while six files did const chalk = require('chalk'). Fix: require('chalk').default in log.js, generate.js, generate-plugin.js, lib/watch/{fork,index,utils}.js. is-docker v4 (same class of bug in start.js) is fixed the same way.

  4. 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 Cannot read properties of undefined (reading 'splice'); and generate-swagger --yaml was rejected by strict mode because yaml was not a registered option. Fix: read parsed.positionals[0] for the help command, and register yaml in args.js CLI options (restoring the lenient pre-migration behaviour).

  5. fastify generate-plugin emitted an empty "tstyche": {} into the generated package.jsonObject.assign(pkg.tstyche || {}, template.tstyche) with template.tstyche === undefined yields {} (regression from the tsd -> tstyche migration, refactor(types): migrate from tsd to tstyche #886). Fix: guard the assignment.

None of 2–5 was visible to users of published versions (7.4.1/8.0.0 still pin pkg-up@^3.1.0), and none was caught by CI because of 1.

Verification

  • npm run lint passes.
  • Local (Node 24, Windows): test/**/*.test.js now actually executes — 75/83 pass. The 6 remaining failures are Windows-only EBUSY (rmdir on in-use temp/workdirs) and its knock-on effects (chdir leak, leftover generated workdir being globbed); they do not reproduce on Linux (rimraf retry semantics + Linux allows removing a cwd). I left them out of scope rather than masking them.
  • The first CI run on this PR will be the first real test run in ~9 months; if unrelated pre-existing failures surface on the matrix, they are separate from this PR's four fixes.

Notes

  • First-time contributor — CI approval may be needed.
  • 4 commits, one per root cause: suite-runner glob, ESM/CJS interop, parseArgs migration, tstyche emission.

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>
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