Skip to content

Conversation

@renovate
Copy link
Contributor

@renovate renovate bot commented Aug 12, 2025

This PR contains the following updates:

Package Change Age Confidence
mocha (source) 11.1.0 -> 11.7.2 age confidence

Release Notes

mochajs/mocha (mocha)

v11.7.2

Compare Source

🩹 Fixes
📚 Documentation
🧹 Chores
🤖 Automation
  • deps: bump actions/checkout in the github-actions group (#​5419) (03ac2d0)

v11.7.1

Compare Source

🩹 Fixes
🧹 Chores

v11.7.0

Compare Source

🌟 Features

v11.6.0

Compare Source

🌟 Features

v11.5.0

Compare Source

🌟 Features

v11.4.0

Compare Source

🌟 Features
📚 Documentation
  • added CHANGELOG.md note around 11.1 yargs-parser update (#​5362) (618415d)

v11.3.0

Compare Source

🌟 Features
📚 Documentation
🧹 Chores

v11.2.2

Compare Source

🩹 Fixes
📚 Documentation

v11.2.1

Compare Source

🩹 Fixes
📚 Documentation
🧹 Chores

v11.2.0

Compare Source

🌟 Features
📚 Documentation
🧹 Chores

Configuration

📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 Ignore: Close this PR and you won't be reminded about this update again.


  • If you want to rebase/retry this PR, check this box

This PR was generated by Mend Renovate. View the repository job log.

@renovate renovate bot requested a review from a team as a code owner August 12, 2025 08:15
@renovate renovate bot added the dependencies Pull requests that update a dependency file label Aug 12, 2025
@codecov
Copy link

codecov bot commented Aug 12, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.06%. Comparing base (2d54408) to head (0c4dce1).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5850   +/-   ##
=======================================
  Coverage   95.06%   95.06%           
=======================================
  Files         307      307           
  Lines        8031     8031           
  Branches     1627     1627           
=======================================
  Hits         7635     7635           
  Misses        396      396           
🚀 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.

@trentm
Copy link
Contributor

trentm commented Aug 13, 2025

[Marc]

I think that the lastest version for mocha would allow us to unflag it again (mochajs/mocha#5366 addresses the failures), but tests are for @opentelemetry/instrumentation are failing for Node.js v20.19.4, but works on v20.6.0 for some reason.

I understand what is happening here now.

So when npm run test:esm runs node --experimental-loader=./hook.mjs ./node_modules/mocha/bin/mocha test/node/*.test.mjs, the "EsmInstrumentation.test.mjs" file is being loaded by mocha with require(), which (in Node v20.19.0) means that the ESM loader hooks specified by --experimental-loader=./hook.mjs are NOT run, so IITM hooking doesn't happen and the tests fail.

We can manully see the tests pass by temporarily disabling require(esm) support via --no-experimental-require-module:

cd experimental/packages/opentelemetry-instrumentation
node --no-experimental-require-module --experimental-loader=./hook.mjs ../../../node_modules/mocha/bin/mocha 'test/node/*.test.mjs'

However, that isn't a good solution.

So why doesn't this fail in Node.js v22 which also has require(esm) support? The answer is that it does fail: for <22.15.0.
In Node v22.15.0 (https://nodejs.org/en/blog/release/v22.15.0) there was nodejs/node#55698

- [b2e44a8079] - (SEMVER-MINOR) module: implement module.registerHooks() (Joyee Cheung) #55698

IIUC, this added support for running ES module hooks for files loaded via require(esm).
While require(esm) is in Node v20, the implement module.registerHooks() change is not.

I think a solution here is to patch mocha module loading (again) to skip using require(esm) if the file has a .mjs extension. The .mjs extension is a clear signal that the file is intended to be loaded via import(). This special-casing of .mjs files already exists for another related function in mocha: https://github.com/mochajs/mocha/blob/a908b3b86604d41d5751cccfaff505d7092c114f/lib/nodejs/esm-utils.js#L42-L44

This change to [email protected] fixes this test (and, I believe, any other mocha .../foo.test.mjs cases we have in the core repo):

--- ../../../node_modules/mocha/lib/nodejs/esm-utils.js	2025-08-13 09:17:47
+++ ./node_modules/mocha/lib/nodejs/esm-utils.js	2025-08-13 12:06:44
@@ -90,6 +90,10 @@
 // and `require.cache` effective, while allowing us to load ESM modules
 // and CJS modules in the same way.
 const requireModule = async (file, esmDecorator) => {
+  if (path.extname(file) === '.mjs') {
+    return formattedImport(file, esmDecorator);
+  }
+
   try {
     return require(file);
   } catch (err) {

I'll open a mocha PR.

@trentm
Copy link
Contributor

trentm commented Aug 15, 2025

So why doesn't this fail in Node.js v22 which also has require(esm) support? The answer is that it does fail: for <22.15.0. In Node v22.15.0 (https://nodejs.org/en/blog/release/v22.15.0) there was nodejs/node#55698

- [b2e44a8079] - (SEMVER-MINOR) module: implement module.registerHooks() (Joyee Cheung) #55698

This section is not completely correct. While creating a small repro-case for my mocha bug report, I found that Node v22.15.0 does not fix the smaller repro case: https://github.com/trentm/repro-mocha-11-mjs-import-hooks-issue

So there is some detail that I'm missing. I think the difference is related to my smaller repro case depending on the test file itself (test/repro.test.mjs) needing to have been hooked by the --experimental-loader ./hook.mjs loader. For the opentelemetry-js/experimental/packages/opentelemetry-instrumentation/test/EsmInstrumentation.test.mjs case, the requirement is that the files imported by EsmInstrumentation.test.mjs need to be hooked by the custom loader. So I think there is some detail in Node.js >=22.15.0 where require(some-esm-file) will run custom loader hooks on (a) files imported by "some-esm-file", but (b) not on "some-esm-file" itself.

(Side note: I don't have a good alternative answer, so this amounts to just a complaint. I don't like the pile of complexity in our testing here: mocha running TypeScript test files via a runtime loader that transpiles, with mocha having internal complex logic to second guess whether files are CommonJS or ESM. Add to that that mocha runs separate test files in the same process, so they can cross-talk.)

@renovate renovate bot force-pushed the renovate/mocha-11.x branch from a724925 to 0c4dce1 Compare September 1, 2025 20:40
@renovate renovate bot changed the title chore(deps): update dependency mocha to v11.7.1 chore(deps): update dependency mocha to v11.7.2 Sep 1, 2025
Copy link
Member

@pichlermarc pichlermarc left a comment

Choose a reason for hiding this comment

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

New version works now. Thank you @legendecas (original PR) and @trentm (follow-up issue) for doing the heavy lifting 🎉

@pichlermarc pichlermarc added this pull request to the merge queue Sep 2, 2025
Merged via the queue into main with commit a171a5c Sep 2, 2025
24 checks passed
@pichlermarc pichlermarc deleted the renovate/mocha-11.x branch September 2, 2025 08:20
@opentelemetrybot
Copy link
Contributor

Thank you for your contribution @renovate[bot]! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey.

Joozty pushed a commit to Joozty/opentelemetry-js that referenced this pull request Sep 9, 2025
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants