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

Karma is deprecated #8939

Open
jonkoops opened this issue Apr 28, 2023 · 17 comments · May be fixed by #9242 or #9243
Open

Karma is deprecated #8939

jonkoops opened this issue Apr 28, 2023 · 17 comments · May be fixed by #9242 or #9243
Labels
tests workflow Anything related to Leaflet development process and tools

Comments

@jonkoops
Copy link
Collaborator

jonkoops commented Apr 28, 2023

Karma has been deprecated today, so we'll likely have to migrate away from it at some point. We might want to consider Web Test Runner as the blog post from the Angular team suggests.

@jonkoops jonkoops added tests workflow Anything related to Leaflet development process and tools labels Apr 28, 2023
@mourner
Copy link
Member

mourner commented Apr 28, 2023

I thought we would migrate away from Karma sooner or later anyway — it's a very old project and a pretty stale one. Web Test Runner looks pretty nice.

@jonkoops
Copy link
Collaborator Author

@mourner I wonder if the performance issues that we're experiencing under #8936 might also be resolved migrating away from Karma. I think I will take a look at how much effort it might be to move away from Karma first.

@jonkoops
Copy link
Collaborator Author

@Falke-Design as part of our migration to Web Test Runner we need all of our dependencies used during the testing process to be ESM compatible. Since you are maintaining ui-event-simulator could you review the PR implementing the aforementioned there?

@Falke-Design
Copy link
Member

@jonkoops done

@jonkoops
Copy link
Collaborator Author

@Falke-Design Awesome thanks! Could you also publish a 2.0 for that package?

@Falke-Design
Copy link
Member

Should be already there 🤔

@jonkoops
Copy link
Collaborator Author

Strange, it is not showing up on NPM.

@Falke-Design
Copy link
Member

I forgot to publish ... 😄 I will do it at the evening (in 6h)

@jonkoops
Copy link
Collaborator Author

jonkoops commented May 16, 2023

I also had to refactor prosthetic-hand to a pure ESM package so it can be consumed by Web Test Runner. @IvanSanchez @mourner @Falke-Design can I ask you to review this PR? Leaflet/prosthetic-hand#25

@Falke-Design
Copy link
Member

I forgot to publish ... 😄 I will do it at the evening (in 6h)

@jonkoops done

@mourner
Copy link
Member

mourner commented Feb 3, 2024

Tried porting a small part of the tests to Web Test Runner to get a feel for it — looks like it works pretty well overall. Here's the commit: e9a3cf3

A few hiccups I've encountered:

  • We'll probably have to mock package.json because browsers don't support importing JSON like it was JS (or maybe with / assert keywords would help, not sure about browser support though).
  • GeneralSpec.js can't test global variables because the test will import Leaflet as a module, so it won't get the global L.

I guess we could address both by setting the built Leaflet version as the entry point in the import maps rather than src/Leaflet.js, but not sure — didn't we want the source to be a viable module entry point for v2? In that case it would defeat the purpose, right? @jonkoops

@jonkoops
Copy link
Collaborator Author

jonkoops commented Feb 3, 2024

We'll probably have to mock package.json because browsers don't support importing JSON like it was JS (or maybe with / assert keywords would help, not sure about browser support though).

Yeah, basically only Chrome supports this at the moment, and it only supports the now defunct assert keyword. We could work around this for now by handling this at compile-time, or perhaps by fetching the JSON.

GeneralSpec.js can't test global variables because the test will import Leaflet as a module, so it won't get the global L.

Ah yes, I hadn't considered that. We'll have to find a way to get that working.

I guess we could address both by setting the built Leaflet version as the entry point in the import maps rather than src/Leaflet.js, but not sure — didn't we want the source to be a viable module entry point for v2? In that case it would defeat the purpose, right? @jonkoops

Yeah, ideally we'd be able to load it directly from source, but there are likely still benefits to having a bundled version. We'll have to evaluate this based on user demand I think.

I think right now the most critical thing is that we want to move the test suite away from Karma, we can worry about details for the distribution later. We might also consider using Vitest, which has the advantage that we can rely on Vite being the bundler, of which my experience has been pretty good. It also has a built-in browser mode.

@mourner perhaps you could raise the Web Test Runner code as a draft PR? We can see if we can do some collaboration to get it landed.

@mourner mourner linked a pull request Feb 3, 2024 that will close this issue
@mourner
Copy link
Member

mourner commented Feb 3, 2024

@jonkoops started a draft PR here: #9242

I also tried Vitest but somehow found it much clunkier and more difficult to get working, at least for this codebase; perhaps this is due to its browser mode still being experimental and not mature enough.

@jonkoops
Copy link
Collaborator Author

jonkoops commented Feb 3, 2024

Yeah, just wanted to put it out there as an option. I think Web Test Runner might make more sense for us now, also because it will allow us to keep the test suite pretty much unmodified otherwise.

jonkoops added a commit that referenced this issue Feb 5, 2024
@jonkoops jonkoops linked a pull request Feb 5, 2024 that will close this issue
@jonkoops
Copy link
Collaborator Author

jonkoops commented Feb 5, 2024

Alternatively we might also consider uvu, but it seems extremely minimal.

@mourner
Copy link
Member

mourner commented Feb 5, 2024

@jonkoops love uvu, would be really awesome if we could get it to work with multiple browsers on CI and enough flexibility. The minimalistic approach fits the one from Leafet really well, and the new bloated frameworks are really tiring (very hard to debug and maintain).

@jonkoops
Copy link
Collaborator Author

jonkoops commented Feb 5, 2024

Yeah, totally agree with that assessment. Unfortunately it's a bit too minimal, since the docs reference browser support, but fail to actually provide details aside from a very bare-bones example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests workflow Anything related to Leaflet development process and tools
Projects
None yet
3 participants