Skip to content

CommonJS & ESM for tests #3391

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

Closed
david-luna opened this issue May 29, 2023 · 2 comments
Closed

CommonJS & ESM for tests #3391

david-luna opened this issue May 29, 2023 · 2 comments
Labels
agent-nodejs Make available for APM Agents project planning.

Comments

@david-luna
Copy link
Member

Some of the agent dependencies are moving to ESM exclusively and this presents a new challenge for testing their version using TAV. In this dependabot PR which updates got we faced 2 problems

  • exports property of the package does not have package.json in it so we are not allowed to require it
  • we need to use dynamic imports

So we should find an alternative to require to read package.json files and also a way for the test to know when to use require or dynamic imports (if there is no single way of doing it)

Note: better to wait until #1952 is completed

@github-actions github-actions bot added the agent-nodejs Make available for APM Agents project planning. label May 29, 2023
@trentm
Copy link
Member

trentm commented May 29, 2023

Specifically for got note

# Testing 'got' was initially added to test an issue in its usage of
# [email protected]. However, this test case serves to sanity test 'got'
# usage with the agent. Got@12 is pure ESM, so cannot currently be tested with
# the current test script.


For the "get the version from package.json" error (if I understand it correctly), we have safeGetPackageVersion in _utils.js:

// "Safely" get the version of the given package, if possible. Otherwise return
// null.
//
// Here "safely" means avoiding `require("$packageName/package.json")` because
// that can fail if the package uses an old form of "exports"
// (e.g. https://github.com/elastic/apm-agent-nodejs/issues/2350).
function safeGetPackageVersion (packageName) {

@david-luna
Copy link
Member Author

So we should find an alternative to require to read package.json files and also a way for the test to know when to use require or dynamic imports (if there is no single way of doing it)

#1952 is done so now:

  • we can use runTestFixtures function to tests for CJS and ESM packages
  • we also have safeGetPackageVersion to get the package info

we can close this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-nodejs Make available for APM Agents project planning.
Projects
None yet
Development

No branches or pull requests

2 participants