feat(mocha-to-node-test-runner): migrate mocha@8.x to `node:test` (v22, v24+) by Xstoudi · Pull Request #313 · nodejs/userland-migrations

Xstoudi · 2025-12-22T21:39:39Z

Should fix #103

AugustinMauroy

Oh nice that really cool !

Few points that can be improved:

md format on readme
little bit of JSdoc
small missing unit test

recipes/mocha-to-node-test-runner/README.md

recipes/mocha-to-node-test-runner/src/workflow.ts

utils/src/is-esm.ts

Co-authored-by: Augustin Mauroy <97875033+AugustinMauroy@users.noreply.github.com>

Xstoudi · 2025-12-23T19:12:27Z

I wrote test for is-esm, let me know if you see something is missing.

I applied your recommendations then reverted two of them:

one was changing logic wrongly
replacing array.length === 0 with !array.length make it less readable, I'd prefer to avoid doing so

Let me now if there is anything else.

AugustinMauroy · 2025-12-24T08:05:14Z

recipes/mocha-to-node-test-runner/tests/input/1_basic.js

+		it('should return -1 when the value is not present', function() {
+			const arr = [1, 2, 3];
+			assert.strictEqual(arr.indexOf(4), -1);
+		});


maybe add an other it that use arrow function as call back
and a fonction as arg like

function myBasicTest = () => { assert(true) }

The second case will be can super complicated to migrate.
in a first time let's just have test that proof that's ins't handled and document it.
In second time (follow up pr) having systems that handle this case. The logic to catch a refercen is already furnished by the codemod context https://docs.codemod.com/jssg/semantic-analysis

Xstoudi · 2025-12-26T22:08:12Z

Let me know if anything more is required.

AugustinMauroy

LGMT ! for the code transformation part missing the removing dep you should take code from chalk-to-styletext.

recipes/mocha-to-node-test-runner/README.md

Xstoudi · 2025-12-30T18:11:09Z

Added that, let me know if something is missing.

AugustinMauroy

For me it's good ! we need to wait a second approval to land !

AugustinMauroy · 2025-12-30T20:49:21Z

F*ck what happened here. !Windows!

cc @nodejs/userland-migrations any idea

recipes/mocha-to-node-test-runner/src/workflow.ts

AugustinMauroy · 2026-01-17T17:25:05Z

hey 👋 @Xstoudi

Co-authored-by: Bruno Rodrigues <swe@brunocroh.com>

AugustinMauroy

LGTM ! I stil don't get why windows give this error
Maybe use lower comparison level on testing https://docs.codemod.com/jssg/testing#strictness-levels

brunocroh

LGTM

JakobJingleheimer · 2026-03-24T06:49:20Z

The windows testing is failing with:

---- tests_7_timeouts.js ----
Output mismatch for test 'tests_7_timeouts.js':
-const assert = require('assert');
-const { describe, it } = require('node:test');
-describe('Timeout Test', { timeout: 500 }, function() {
-
-	it('should complete within 100ms', { timeout: 100 }, (t, done) => {
-		setTimeout(done, 500); // This will fail
-	});
-
-	it('should complete within 200ms', { timeout: 200 }, function(t, done) {
-		setTimeout(done, 100); // This will pass
-	});
-});

Where it looks like the entire contents of the expected file isn't there. @mohebifar have you seen anything like this before?

JakobJingleheimer

Thanks for this! 🙌

A few nits here and there.

For the test files, could the names please be more descriptive (eg 3_hooks is not so illuminating).

Also, there are 2 options for organising test files. We do have a bit of a mix, but option 1 is far superior to the "junk-drawer" option 2. Would you mind adjusting these to option 1? If you don't want to bother with it, that's fine—but would you mind if one of us made the adjustment within this PR?

I haven't reviewed the test cases yet because it's so cumbersome with option 2.

JakobJingleheimer · 2026-03-24T06:59:25Z

recipes/mocha-to-node-test-runner/src/workflow.ts

+	const globalIdentifiers = ['describe'];
+
+	const usedGlobalIdentifiers = globalIdentifiers.filter((globalIdentifier) =>
+		['', '.skip', '.only']


nit: this is constant / static, so better to move to the root of this module (in which case, please make the name screaming snake_case: USED_GLOBAL_IDENTIFIERS

JakobJingleheimer · 2026-03-24T07:01:59Z

recipes/mocha-to-node-test-runner/src/workflow.ts

+	const usedMochaGlobals = [
+		...new Set(
+			mochaGlobalsNodes.map(
+				(mochaGlobalsNode) =>
+					mochaGlobalsNode.getMatch('MOCHA_GLOBAL_FN').text().split('.')[0],
+			),
+		),
+	];
+
+	// if mocha isn't found, don't try to apply changes
+	if (usedMochaGlobals.length === 0) return [];


nit: check the size of the Set (and abort) before converting it to an array to avoid the expense (micro-op, but no need to leave money on the table).

JakobJingleheimer · 2026-03-24T07:04:29Z

recipes/mocha-to-node-test-runner/src/workflow.ts

+			if (thisKeyword) {
+				edits.push(thisKeyword.replace('t'));
+			}


nit: some places inline this but others don't; would be nice to keep it consistent (I personally prefer concise)

Suggested change

if (thisKeyword) {

edits.push(thisKeyword.replace('t'));

}

if (thisKeyword) edits.push(thisKeyword.replace('t'));

JakobJingleheimer · 2026-03-24T07:04:43Z

recipes/mocha-to-node-test-runner/src/workflow.ts

+			if (!fn) {
+				return edits;
+			}


same nit

Suggested change

if (!fn) {

return edits;

}

if (!fn) return edits;

JakobJingleheimer · 2026-03-24T07:33:58Z

recipes/mocha-to-node-test-runner/codemod.yaml

+capabilities:
+  - fs
+  - child_process
+description: Migrate Mocha v8 tests to Node.js test runner (v22, v24+)


Suggested change

description: Migrate Mocha v8 tests to Node.js test runner (v22, v24+)

description: Migrate Mocha 8.x tests to Node.js (22.x, 24.x) test runner

JakobJingleheimer · 2026-03-24T07:47:57Z

utils/src/is-esm.ts

+	const isCjsFile = filename.endsWith('.cjs') || filename.endsWith('.cts');
+	const isMjsFile = filename.endsWith('.mjs') || filename.endsWith('.mts');
+	if (isMjsFile) {
+		return true;
+	}
+	if (isCjsFile) {
+		return false;
+	}


nit for 2 reasons:

those consts are not used below (and it's pretty clear what they're holding) so no need to keep them in memory after they're read the once

micro-op: you don't need to check isCjsFile when isMjsFile

Suggested change

const isCjsFile = filename.endsWith('.cjs') || filename.endsWith('.cts');

const isMjsFile = filename.endsWith('.mjs') || filename.endsWith('.mts');

if (isMjsFile) {

return true;

}

if (isCjsFile) {

return false;

}

if (filename.endsWith('.mjs') || filename.endsWith('.mts')) return true;

if (filename.endsWith('.cjs') || filename.endsWith('.cts')) return false;

JakobJingleheimer · 2026-03-24T07:48:25Z

utils/src/is-esm.ts

+	const rootNode = root.root();
+	const usingImport = rootNode.find({


nit (these are not coupled)

Suggested change

const rootNode = root.root();

const usingImport = rootNode.find({

const rootNode = root.root();

const usingImport = rootNode.find({

JakobJingleheimer · 2026-03-24T07:48:40Z

utils/src/is-esm.ts

+	if (usingImport) {
+		return true;
+	}


nit for consistency

Suggested change

if (usingImport) {

return true;

}

if (usingImport) return true;

JakobJingleheimer · 2026-03-24T07:49:02Z

utils/src/is-esm.ts

+	if (usingRequire) {
+		return false;
+	}


same nit

Suggested change

if (usingRequire) {

return false;

}

if (usingRequire) return false;

JakobJingleheimer · 2026-03-24T07:50:18Z

utils/src/is-esm.ts

+		return false;
+	}
+
+	const packageJsonPath = join(process.cwd(), 'package.json');


@AugustinMauroy what was the verdict on node:module.findPackageJSON()?

Xstoudi added 4 commits December 21, 2025 01:31

wip

7dc0c74

wip

11d3fd6

wip

99e0ab4

wip

ef337d5

Xstoudi changed the title ~~feature(mocha-to-node-test-runner) : Migrate Mocha v8 tests to Node.js test runner (v22, v24+)~~ feat(mocha-to-node-test-runner) : Migrate Mocha v8 tests to Node.js test runner (v22, v24+) Dec 22, 2025

wip

759291a

AugustinMauroy reviewed Dec 23, 2025

View reviewed changes

Xstoudi and others added 2 commits December 23, 2025 19:12

Apply suggestions from code review

7262ab1

Co-authored-by: Augustin Mauroy <97875033+AugustinMauroy@users.noreply.github.com>

wip

ebdc996

AugustinMauroy reviewed Dec 24, 2025

View reviewed changes

fix

be89c04

AugustinMauroy reviewed Dec 28, 2025

View reviewed changes

recipes/mocha-to-node-test-runner/README.md Outdated Show resolved Hide resolved

fix

ead6abc

AugustinMauroy approved these changes Dec 30, 2025

View reviewed changes

fix

cf520d2

brunocroh reviewed Jan 5, 2026

View reviewed changes

recipes/mocha-to-node-test-runner/src/workflow.ts Outdated Show resolved Hide resolved

recipes/mocha-to-node-test-runner/src/workflow.ts Show resolved Hide resolved

recipes/mocha-to-node-test-runner/src/workflow.ts Show resolved Hide resolved

brunocroh added the awaiting author Reviewer has requested something from the author label Jan 9, 2026

Xstoudi and others added 6 commits March 11, 2026 20:26

Update recipes/mocha-to-node-test-runner/src/workflow.ts

4377e8d

Co-authored-by: Bruno Rodrigues <swe@brunocroh.com>

Merge branch 'nodejs:main' into feature/mocha-to-node-test-runner

003b2df

factorize

1192ded

cover esm

e59e09b

update remove dependencies step

5a35723

lint

d7ebcff

AugustinMauroy approved these changes Mar 11, 2026

View reviewed changes

brunocroh approved these changes Mar 23, 2026

View reviewed changes

JakobJingleheimer changed the title ~~feat(mocha-to-node-test-runner) : Migrate Mocha v8 tests to Node.js test runner (v22, v24+)~~ feat(mocha-to-node-test-runner): Migrate Mocha v8 tests to Node.js test runner (v22, v24+) Mar 24, 2026

JakobJingleheimer changed the title ~~feat(mocha-to-node-test-runner): Migrate Mocha v8 tests to Node.js test runner (v22, v24+)~~ feat(mocha-to-node-test-runner): migrate Mocha v8 tests to node:test (v22, v24+) Mar 24, 2026

JakobJingleheimer changed the title ~~feat(mocha-to-node-test-runner): migrate Mocha v8 tests to node:test (v22, v24+)~~ feat(mocha-to-node-test-runner): migrate mocha@8.x tests to node:test (v22, v24+) Mar 24, 2026

JakobJingleheimer changed the title ~~feat(mocha-to-node-test-runner): migrate mocha@8.x tests to node:test (v22, v24+)~~ feat(mocha-to-node-test-runner): migrate mocha@8.x to node:test (v22, v24+) Mar 24, 2026

JakobJingleheimer reviewed Mar 24, 2026

View reviewed changes

	description: Migrate Mocha v8 tests to Node.js test runner (v22, v24+)
	description: Migrate Mocha 8.x tests to Node.js (22.x, 24.x) test runner

		const rootNode = root.root();
		const usingImport = rootNode.find({

Uh oh!

Conversation

Xstoudi commented Dec 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AugustinMauroy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Xstoudi commented Dec 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Xstoudi commented Dec 26, 2025

Uh oh!

AugustinMauroy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Xstoudi commented Dec 30, 2025

Uh oh!

AugustinMauroy left a comment

Choose a reason for hiding this comment

Uh oh!

AugustinMauroy commented Dec 30, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AugustinMauroy commented Jan 17, 2026

Uh oh!

AugustinMauroy left a comment

Choose a reason for hiding this comment

Uh oh!

brunocroh left a comment

Choose a reason for hiding this comment

Uh oh!

JakobJingleheimer commented Mar 24, 2026

Uh oh!

JakobJingleheimer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Xstoudi commented Dec 22, 2025 •

edited

Loading