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

Import Leaflet in tests using JavaScript modules #8936

Closed
wants to merge 1 commit into from

Conversation

jonkoops
Copy link
Collaborator

Changes the unit tests so they use the named exports from Leaflet loaded trough an import map.

@jonkoops jonkoops added refactoring workflow Anything related to Leaflet development process and tools labels Apr 27, 2023
@jonkoops jonkoops self-assigned this Apr 27, 2023
@jonkoops jonkoops added this to In progress in ES Modules via automation Apr 27, 2023
@jonkoops jonkoops added this to the 2.0 milestone Apr 27, 2023
@jonkoops
Copy link
Collaborator Author

Interesting, tests sometimes fail. I have no idea why yet, will look at it another time (gotta sleep).

@mourner
Copy link
Member

mourner commented Apr 28, 2023

Interesting, tests sometimes fail. I have no idea why yet, will look at it another time (gotta sleep).

Looks like all the failures are timeouts — I guess browsers take to long to hop through all the imports and fetch everything.

Also noting that the total test time jumps from <3m to 6–8m. We should look into addressing that before shipping — perhaps this alone would be enough to keep running against the bundle. It may also be beneficial because some issues may only manifest after bundling, which is how users end up using the code anyway, so tests closer to real world.

@jonkoops
Copy link
Collaborator Author

Perhaps this alone would be enough to keep running against the bundle.

How do you mean? We are currently still running the tests against the bundled code. Or are you referring to #8937?

I am not quite sure what is causing the performance to regress so much, I'll have to take a look and investigate deeper.

@mourner
Copy link
Member

mourner commented Apr 28, 2023

@jonkoops ah right, I missed that it's not yet implementing individual file import. But the regression is indeed suspicious — let's make sure it's not slower before we can land this.

@jonkoops jonkoops mentioned this pull request Apr 28, 2023
@jonkoops
Copy link
Collaborator Author

I'm closing this PR so that I can merge this work into a larger effort to start using Web Test Runner. See #8939 for more information.

@jonkoops jonkoops closed this May 14, 2023
ES Modules automation moved this from In progress to Done May 14, 2023
@jonkoops jonkoops deleted the esm-leaflet-tests branch May 14, 2023 13:27
@jonkoops jonkoops removed this from the 2.0 milestone May 14, 2023
@jonkoops jonkoops removed this from Done in ES Modules May 14, 2023
@jonkoops jonkoops restored the esm-leaflet-tests branch May 30, 2023 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring workflow Anything related to Leaflet development process and tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants