Skip to content
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

feat(ioredis): prototype running tests in ESM mode #1794

Closed

Conversation

pichlermarc
Copy link
Member

@pichlermarc pichlermarc commented Nov 13, 2023

Which problem is this PR solving?

This is a prototype for running esm tests using our existing tests, based on my comment on #1731.

This PR is not intended for merging, but rather to illustrate how we could run ESM tests if we'd want to have full coverage with our existing tests.

  • esm.mocharc.json is used by the esm tests, and specifies both ts-node and otel esm loaders
  • npm run prepare:test:esm
    • patches the mocha cli by removing an if statement that checks if it's being called from the command line, which evaluates to false due to import-in-the-middle
    • sets the type to module in package.json, I've not found a way around this yet.
  • npm run test:esm:local lets one run the tests in ESM mode (the instrumentation code is also transpiled to ESM mode, which does not 100% reflect real-life as we don't publish ESM with the npm package)
    • I've addd the log statement from test(ioredis): test ioredis ESM usage #1752 so that we have a similar log output here
    • we need to use import in the tests so that they also work with ESM, it transpiles to require for cjs, so that should be ok.
  • restore:test:cjs sets the type back to commonjs again and un-patches mocha.

This is quite a prototype setup; I think if we'd want to introduce something like this, we'd need adapt some of this to be less fragile.

@pichlermarc
Copy link
Member Author

pichlermarc commented Nov 13, 2023

Unassigning as this PR is for illustration purposes only.

@pichlermarc
Copy link
Member Author

Closing as #1735 has now been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants