chore(e2e-test): add regression test for dynamic import() MONGOSH-1062 #2458
Conversation
| expect(result).to.match(/^B-ESM$/m); | ||
| }); | ||
|
|
||
| it('import() works when interleaved with GC', async function () { |
There was a problem hiding this comment.
what is the relationship between the garbage collector and import that would cause issues?
There was a problem hiding this comment.
@gagik Sorry, completely missed this comment! The original ticket is nodejs/node#38695, I'll link it here in the comments as well – basically, Node.js internals had a memory management bug where a C++ object describing modules could get deleted during garbage collection while the "public" JS module instance was still alive, and then accessing that C++ object from JS would crash
| it('import() works when interleaved with GC', async function () { | |
| // Regression test for https://github.com/nodejs/node/issues/38695 | |
| it('import() works when interleaved with GC', async function () { |
Obviously this is really more of a Node.js issue and one could argue that we don't need a specific regression test here in mongosh, but I'd feel more comfortable having it in place for when we start having a "proper" ESM story in mongosh (i.e. mongosh scripts with import/export).
There was a problem hiding this comment.
thank you! the link in comments is helpful.
There was a problem hiding this comment.
Pull Request Overview
This PR adds a regression test for dynamic import() functionality, specifically addressing issue MONGOSH-1062. The changes enhance existing tests to cover both require() and import() module resolution, and add a specific test case for import() when interleaved with garbage collection.
- Updates test title and adds comprehensive testing for both require() and import() module resolution
- Adds regression test for dynamic import() with garbage collection interleaving
- Adds NODE_OPTIONS environment variable to expose garbage collection functionality
Comments suppressed due to low confidence (2)
packages/e2e-tests/test/e2e.spec.ts:1188
- The test assumes that 'b-esm' module has a 'value' property, but this assumption is not validated. Consider adding a check to ensure the module structure is as expected before accessing the 'value' property.
result = await shell.executeLine('require("b-esm").value');
packages/e2e-tests/test/e2e.spec.ts:1200
- [nitpick] The variable name 'importESM' is inconsistent with naming conventions. Consider using camelCase like 'importEsm' or a more descriptive name like 'importBEsmModule'.
await shell.executeLine('importESM = () => import("b-esm")');
No description provided.