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

chore: update karma-webpack to v5.0.1, adapt tests #4648

Conversation

pichlermarc
Copy link
Member

@pichlermarc pichlermarc commented Apr 19, 2024

Which problem is this PR solving?

We've been using karma-webpack@4 which is not compatible with webpack@5 that we were using since #4340
Since webpack 5, node polyfills are not automatically included anymore, so we have to add polyfills manually.

Using an old verison of karma-webpack with a new version of webpack may be is what's causing test failures in #4622, as one can't access any other entrypoint than @opentelemetry/api

Notable changes:

  • we use assert in our tests, so I included the assert polyfill
    • the assert polyfill needs the util polyfill, so I included that
      • the util polyfill needs a process polyfill, so I included that too

Requires #4649
Supersedes #4647, tests fail as no polyfills are added.
Enables #4622, where we can't import @opentelemetry/api/experimental in browser tests unless we update karma-webpack

Type of change

  • internal for the test changes

@pichlermarc
Copy link
Member Author

Well that's no fun. Looks like karma-webpack is not working on macOS. #2234

@pichlermarc
Copy link
Member Author

Well that's no fun. Looks like karma-webpack is not working on macOS. #2234

Actually it looks like this is working, I asked @dyladan to run it on his mac and it works there.

I'll wait for #4649 until marking this as ready for review 🙂
If any other macOS users stumble upon this please try to run the browser tests locally and let me know if you experience any problems.

@pichlermarc pichlermarc marked this pull request as ready for review April 26, 2024 15:09
@pichlermarc pichlermarc requested a review from a team April 26, 2024 15:09
@@ -32,7 +32,7 @@ describe('OTLPLogExporter', () => {
sinon.restore();
});

if (typeof process === 'undefined') {
if (global.process?.versions?.node === undefined) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needed so that we mock the right thing for the right platform, process is now defined due to the required polyfill, so we're using process?.versions?.node instead to determine if we're running in Node.js or the browser.

@trentm
Copy link
Contributor

trentm commented May 1, 2024

If any other macOS users stumble upon this please try to run the browser tests locally and let me know if you experience any problems.

Tests passed for me locally on macOS using Node.js v20.12.2.

Zirak pushed a commit to Zirak/opentelemetry-js that referenced this pull request Sep 14, 2024
* chore(deps): update dependency karma-webpack to v5

* feat: add polyfills missing from webpack 5, adapt tests, split up raw env parsing into browser/node specific code

---------

Co-authored-by: Mend Renovate <[email protected]>
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