chore: replace karma with wpt runner in user-interaction
#2005
Conversation
|
Hmm, ok, fails with the same error here 🤔 Any ideas what it might be? Immediate suspect from my side is that rollup/eslint bundles differently than webpack when it comes to browser/non-browser and we somehow get the wrong version? |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2005 +/- ##
==========================================
- Coverage 90.97% 90.29% -0.69%
==========================================
Files 146 139 -7
Lines 7492 6891 -601
Branches 1502 1379 -123
==========================================
- Hits 6816 6222 -594
+ Misses 676 669 -7 |
There was a problem hiding this comment.
I get the same ❌ TypeError: this._wrap is not a function when attempting npm run test:browser locally.
https://gist.github.com/trentm/0ac028d3d343ffb83a7398540c3a05ae
| @@ -192,7 +192,7 @@ | |||
| you may not use this file except in compliance with the License. | |||
| You may obtain a copy of the License at | |||
|
|
|||
| http://www.apache.org/licenses/LICENSE-2.0 | |||
| https://www.apache.org/licenses/LICENSE-2.0 | |||
There was a problem hiding this comment.
This change should be reverted. The LICENSE files are meant to be a verbatim copy of https://www.apache.org/licenses/LICENSE-2.0.txt
I spent a little time getting further on this, but I don't have the time to get to a conclusion. Adding some debug prints (marked with "XXX" in the output) we see that the imported
In the browser tests, IIUC, we are running as an ES Module (modulo the other issue that the "build/esm/..." outputs are not really valid ES modules; but good enough for bundlers), so:
https://nodejs.org/api/esm.html#interoperability-with-commonjs
The current module.exports pattern in shimmer is one that Node's "static analysis" doesn't work on for getting named exports:
--- node_modules/shimmer/index.js.orig 2024-05-30 11:47:35
+++ node_modules/shimmer/index.js 2024-05-30 12:01:15
@@ -113,10 +113,9 @@
})
}
-shimmer.wrap = wrap
-shimmer.massWrap = massWrap
-shimmer.unwrap = unwrap
-shimmer.massUnwrap = massUnwrap
-
module.exports = shimmer
+module.exports.wrap = wrap
+module.exports.massWrap = massWrap
+module.exports.unwrap = unwrap
+module.exports.massUnwrap = massUnwrapthen I get I further in tests. After this local change the test run after my local changesnpm run test:browserSo I think a next step would be a PR on shimmer to change that I'm not entirely confident in my analysis here, because I haven't followed the code to understand what |
I think this analysis is valid and that is the next step. Looks like there is an open issue for this already too, so other folks have come across it. I'm not sure how active the author is, as there have not been any changes here in several years. But I think it's worth opening the PR with a test and going from there. |
I saw that. Same error, but I wasn't quickly sure if it was exactly the same thing. I think that is a case of the same issue, but on the failed import of names from the module that user was attempting to use shimmer on -- but I'm not sure. |
Which problem is this PR solving?
Following up #1816 for
@opentelemetry/instrumentation-user-interactionShort description of the changes
Migrating another module.
I'm getting the following error locally - it looks like something with my local env, so I thought to open a PR before digging too much more.