Migrate regenerator tests - #18020
Conversation
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/61708 |
There was a problem hiding this comment.
Pull request overview
Migrates the @babel/plugin-transform-regenerator upstream regenerator test suite from being executed via Mocha to being executed via Jest, aiming to simplify the test pipeline and improve CI/runtime performance.
Changes:
- Replace Mocha-based fixture execution with a Jest-based runner invoked from
test/regenerator.js. - Add a dedicated Jest config for regenerator fixtures and adjust fixture syntax to be Jest-compatible.
- Remove the package-level Mocha dependency and update the lockfile accordingly.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Removes mocha and its transitive dependencies from the lockfile. |
| packages/babel-plugin-transform-regenerator/package.json | Drops mocha from devDependencies for this workspace package. |
| packages/babel-plugin-transform-regenerator/test/regenerator.js | Replaces Mocha spawning with a queued/single Jest invocation for regenerator fixture tests. |
| packages/babel-plugin-transform-regenerator/test/regenerator-fixtures/tests.transform.js | Replaces Mocha-only context() usage with describe() for Jest compatibility. |
| packages/babel-plugin-transform-regenerator/test/regenerator-fixtures/jest.config.js | Adds Jest configuration for running the regenerator fixture tests. |
馃挕 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
2c111fa to
2b854fa
Compare
|
commit: |
nicolo-ribaudo
left a comment
nicolo-ribaudo
left a comment
There was a problem hiding this comment.
Can we remove the yargs resolution from the root package.json?
Frankly I don't know if it works although tests on my local machine pass. The |
No, it doesn't work because |
| const { require, __dirname } = commonJS(import.meta.url); | ||
|
|
||
| const mochaDir = dirname(require.resolve("mocha")); | ||
| const jestDir = dirname(require.resolve("jest/package.json")); |
There was a problem hiding this comment.
We can use the run or runCLI functions in jest, but that's fine.
There was a problem hiding this comment.
Will runCLI run jest in a standalone process? The tests here unfortunately require per-test process isolation because of mutation of global states.
There was a problem hiding this comment.
I guess so. It will probably initialize a new jest-light-runner.
There was a problem hiding this comment.
Updated to use runCLI.
This reverts commit 2de536c.
7512aa2 to
79e95c4
Compare
In this PR we migrate the test runner of regenerator tests, previously imported from the upstream repo, from
mochatojest. This should simplify the build pipelineIt will also unblock #18015 since only mocha 10 depends on
yargs@16which is broken on Node.js 26. The v7 backport of this PR passes all the tests on my local machine.