Skip to content

test: bring CLI coverage to 100% and enforce it - #916

Open
Tony133 wants to merge 1 commit into
mainfrom
test/coverage-100
Open

test: bring CLI coverage to 100% and enforce it#916
Tony133 wants to merge 1 commit into
mainfrom
test/coverage-100

Conversation

@Tony133

@Tony133 Tony133 commented Sep 8, 2026

Copy link
Copy Markdown
Member

Proposal:

Follow-up to #913. Now that suite-runner.js actually runs the tests, c8 reports real numbers for the CLI sources, and they were around 87%. This brings every file under *.js and lib/** to 100% statements / branches / functions / lines, and adds a .c8rc.json with check-coverage: true so npm test fails if it drops.

The c8 report is also scoped to the shipped sources: test/, templates/, examples/ and suite-runner.js are excluded, since they are fixtures and test infrastructure rather than CLI code.

New and extended tests:

  • generate-readme, generate-swagger, print-plugins, print-routes, generate, generate-plugin, eject, cli: the cli() paths and the require.main === module blocks are exercised by spawning the scripts as child processes (c8 follows children via NODE_V8_COVERAGE), so process.exit paths are tested for real. Error paths in generate() use proxyquire to fail generify and node:fs.
  • util.test.js: exit, requireModule*, requireFastifyForModule failure, showHelpForCommand with an unknown command, isKubernetes.
  • start-extra.test.js: missing file parameter, fastify not loadable, --address, --trust-proxy-enabled (verified through request.ip), the err branch of the close-with-grace handler. The two is-docker mocks in start.test.js now cover both module shapes (CJS function and ESM { default }).
  • watch-unit.test.js: lib/watch/index.js driven with fake chokidar / child_process.fork (restart on change, verbose logging, ready emitted once, error → stop, uncaughtException → restart).
  • watch-fork.test.js: lib/watch/fork.js forked for real (app crash, EADDRINUSE, graceful shutdown, forced exit after the close timeout, shutdown before the server is up). Each test has a timeout and kills its child on teardown, so a stuck child fails the test instead of hanging the suite.
  • print-plugins / print-routes: the skip under coverage is gone. It was needed with nyc, which instrumented the sources and changed the printed function bodies; c8 uses native V8 coverage and does not touch the code (verified with NODE_V8_COVERAGE set).

Bugs found on the way:

  • node eject.js --esm crashed: the standalone entry point called cli() without arguments. Now cli(process.argv.slice(2)).
  • In generate.js, a failing npm init -y threw synchronously inside the generify callback and escaped the promise: an uncaught exception instead of log('error') + exit 1. It is now caught and rejected.

Dead code removed:

  • util.js: the moduleSupport check (Node < 14). semver is no longer needed at runtime and moves to devDependencies (tests still use it).
  • start.js: the "debug mode not compatible with Node < 6" branch.
  • lib/watch/index.js: the childs.length !== 0 branch in the change handler — at most one child is ever in the array.
  • lib/watch/fork.js: the try/catch around fastify.ready() after listen() already succeeded; a rejection there is handled by main().catch, with the same exit code.
  • generate-readme.js: stop(error) was only ever called without an argument.
  • args.js: commandLineArguments['--'] || []'--' is always populated with populateRest: true.
  • lib/parse-args.js: token.original is never set on positional tokens.
  • pkg.scripts = Object.assign(pkg.scripts || {}, …) → object spread, same semantics without the branch.

Based on #913 — merge that one first.


Screenshot Test Coverage 100%

screenshot

see also here: https://github.com/fastify/fastify-cli/actions/runs/34277129667/job/102232798581?pr=916#step:5:589

@Tony133
Tony133 marked this pull request as ready for review September 8, 2026 20:50
@Tony133
Tony133 requested a review from mcollina September 8, 2026 21:03
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