Skip to content

chore: fix esm-utils tests when run without coverage - #6160

Merged
mark-wiemer merged 1 commit into
mochajs:mainfrom
mostafaNazari702:esm-utils-cts-fixture
Aug 9, 2026
Merged

mark-wiemer merged 1 commit into
mochajs:mainfrom
mostafaNazari702:esm-utils-cts-fixture

Conversation

@mostafaNazari702

@mostafaNazari702 mostafaNazari702 commented Jul 21, 2026 •

Copy link
Copy Markdown
Contributor

PR Checklist

Overview

The reason CI passes is nyc, its .ts hook loads the fixture as CommonJS. Running the spec directly skips that hook and exposes the bug.

@mostafaNazari702

Copy link
Copy Markdown
Contributor Author
bild bild

@github-actions

This comment has been minimized.

@codecov

codecov Bot commented Jul 21, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.93%. Comparing base (badf733) to head (3ad96dc).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6160   +/-   ##
=======================================
  Coverage   81.93%   81.93%           
=======================================
  Files          64       64           
  Lines        4866     4866           
  Branches     1072     1100   +28     
=======================================
  Hits         3987     3987           
  Misses        879      879           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mark-wiemer

Copy link
Copy Markdown
Member

@mostafaNazari702 I don't want cts if we can avoid it, can we add type: commonjs to a nested package.json that applies only to the fixtures in that small folder? There should be other examples in the codebase.

@mark-wiemer mark-wiemer added the status: waiting for author waiting on response from OP or other posters - more information needed label Aug 2, 2026
@mark-wiemer mark-wiemer removed the status: waiting for author waiting on response from OP or other posters - more information needed label Aug 9, 2026
@mark-wiemer mark-wiemer changed the title test: load the mocha-error esm-utils fixture as .cts chore: fix esm-utils tests on newer Node versions Aug 9, 2026
@mark-wiemer

Copy link
Copy Markdown
Member

the reason CI passes is nyc, its .ts hook loads the fixture as CommonJS. Running the spec directly skips that hook and exposes the bug.

Ah, this means we should tag it as @bare I believe

@mark-wiemer mark-wiemer changed the title chore: fix esm-utils tests on newer Node versions chore: fix esm-utils tests when run without coverage Aug 9, 2026
@mark-wiemer

Copy link
Copy Markdown
Member

Ref #5494

@mark-wiemer

mark-wiemer commented Aug 9, 2026 •

Copy link
Copy Markdown
Member

Ah, we only added @bare in the v11.x branch: https://github.com/mochajs/mocha/blob/v11.x/test/integration/esm.spec.js#L85

I'll think about this a bit more.

@mark-wiemer mark-wiemer added the status: needs review a maintainer should (re-)review this pull request label Aug 9, 2026
@mark-wiemer

Copy link
Copy Markdown
Member

Well, that bare stuff isn't a blocker for this PR, which works in both cases now with minimal big changes. Merging, thanks!

@mark-wiemer
mark-wiemer merged commit c54539f into mochajs:main Aug 9, 2026
255 of 256 checks passed
@mostafaNazari702
mostafaNazari702 deleted the esm-utils-cts-fixture branch August 10, 2026 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: needs review a maintainer should (re-)review this pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🛠️ Repo: esm-utils test fails on newer Node 24 because a .ts fixture uses require()

2 participants