Skip to content

fix(test): make npm test run and report a summary on Node 22 and 24 - #537

Merged
kevinchappell merged 1 commit into
mainfrom
fix/test-summary
Oct 20, 2026
Merged

kevinchappell merged 1 commit into
mainfrom
fix/test-summary

Conversation

@kevinchappell

Copy link
Copy Markdown
Collaborator

Tracked in Draggable/formeo.io#89 ('npm test reports no summary').

Root causes

  1. Crash on Node 22 LTS: --loader=./tools/svg-loader.mjs combined with --require ./tools/test-setup.cjs fails immediately with ERR_METHOD_NOT_IMPLEMENTED: resolveSync(). An async --loader can't serve require() of ES modules. The suite never ran, so no summary was printed. The PR workflow uses Node 24, which hid this.
  2. Shell glob expansion: src/**/*.test.{js,mjs} was unquoted, so /bin/sh (no brace expansion, no **) expanded it, not Node.
  3. Reporter: in a non-TTY run, Node's default reporter is TAP, whose summary is easy to miss.

Fix

  • Hooks: the SVG hook moves to tools/register-test-hooks.mjs, loaded with --import. It uses module.registerHooks() (synchronous) and falls back to module.register().
  • Glob: the test pattern is quoted, so Node expands it. All 74 test files run.
  • Reporter: --test-reporter=spec is set explicitly, so a readable tests/pass/fail summary prints in a terminal and in CI.
  • test:watch and test:updateSnapshots get the same changes.

Verified

  • Node 22.23 and 24.22: 1186 tests pass, exit 0.
  • A deliberately broken test gives fail 1 and exit 1.
  • test:ci (npm test --coverage) and the release workflow keep using npm test unchanged.

Note: merging to main triggers the Publish workflow (semantic-release). As a fix: commit, this would release a patch version. So I opened this PR but haven't merged it.

On Node 22.23 `npm test` crashed before running any test with
ERR_METHOD_NOT_IMPLEMENTED ("The resolveSync() method is not implemented"):
the async `--loader` SVG hook cannot serve require() of ES modules, which
test-setup.cjs triggers. With no run there was no summary.

- Register the SVG load hook from `--import=./tools/register-test-hooks.mjs`
  using the synchronous, in-thread `module.registerHooks()`, falling back to
  `module.register()` on Node versions without it. Drop the deprecated
  `--loader` flag.
- Quote the test glob so Node expands it rather than the shell (`/bin/sh`
  has no brace expansion or `**`); all 74 test files run.
- Use `--test-reporter=spec` so the tests/pass/fail summary is human-readable
  on non-TTY output too (Node 22 defaults to TAP there).
- Apply the same flags to test:watch and test:updateSnapshots.

Closes formeo.io#89

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 20, 2026 10:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@kevinchappell
kevinchappell merged commit b24dc68 into main Oct 20, 2026
2 checks passed
@kevinchappell
kevinchappell deleted the fix/test-summary branch October 20, 2026 18:03
github-actions Bot pushed a commit that referenced this pull request Oct 20, 2026
## [5.19.4](v5.19.3...v5.19.4) (2026-10-20)

### Bug Fixes

* **styles:** raise success/warning/error button contrast to WCAG AA ([#539](#539)) ([1907f65](1907f65)), closes [formeo.io#89](https://github.com/formeo.io/issues/89)
* **test:** make npm test run and report a summary on Node 22 and 24 ([#537](#537)) ([b24dc68](b24dc68)), closes [formeo.io#89](https://github.com/formeo.io/issues/89)
@kevinchappell

Copy link
Copy Markdown
Collaborator Author

🎉 This PR is included in version 5.19.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants