-
-
Notifications
You must be signed in to change notification settings - Fork 27
feat(mocha-to-node-test-runner): add new Mocha v8 to Node v22, v24 test runner migration codemod #268
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
AugustinMauroy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't forget to run npm ci to add this new workspace to the lock file
Mention on readme that is design for mocha v8 & node v22, v24 is a super point.
Also I saw that now your tests keep function keyword for callback. maybe add example with arrow function
Instead good first commit. And wow, you really put your foot in it for a first contribution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have some utility for ast-grep, you may need them you have doc here https://github.com/nodejs/userland-migrations/tree/main/utils
Also genre note about the codemod too much regex which is unsafe and to complex. To avoid that you have to use ast-grep ast query api that look like that
const statement = root.findAll({
rule: {
pattern: `${base}.propriety`,
not: {
inside: {
kind: "assignment_expression",
has: {
kind: "member_expression",
pattern: `${base}.fips`,
},
},
},
},
});To have representation of ast / testing query you can use ast-grep playground https://ast-grep.github.io/playground.html
|
Thanks for your review and patience @AugustinMauroy and kind words, yep it is my first open source contribution 😅. I will continue working to address your comments. |
|
@AugustinMauroy I'm having a lot of trouble trying to setup the codemod. I'm used to jscodeshift in which you modify the AST but in this type of codemod you are generating location based edits and each of your edits can conflict with each other which makes this kind of transform very brittle. |
Wow I'm surprised to see that happened. For example we have the whole repo. |
|
hey @eliOcs! thanks a lot for contributing to node codemods. i’m sharing a few pointers that might help. if you have more questions, feel free to join the Codemod community
|
| // ESM - add import statement | ||
| const importStatement = `import { ${testImports.join(', ')} } from "node:test";\n`; | ||
| // Insert after first import or at the beginning | ||
| const firstImportMatch = sourceCode.match( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you just have to catch import_statement kind in your ast. You always need to try to remove avoid string manipulation
| // CJS | ||
| const requireStatement = `\n\nconst {\n ${testImports.join(',\n ')}\n} = require("node:test");\n`; | ||
| // Insert after first require or at the beginning | ||
| const firstRequireMatch = sourceCode.match( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for that you have to catch require call inside of variable_declaration or variable_declaration then use methods range on node and the obtain position.
Transform function that converts Mocha tests to Node.js test runner.
Handles:
Related to #103