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.

Sign up for GitHub

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

Jump to bottom

Remove Rollup pre-proccesor from Karma runner #8935

Merged
merged 1 commit into from
Apr 27, 2023
Merged

Remove Rollup pre-proccesor from Karma runner #8935

merged 1 commit into from
Apr 27, 2023

Conversation

jonkoops
Copy link
Collaborator

This PR refactors the Karma configuration so that the Rollup pre-proccesor is no longer included. This is needed for a subsequent that is in the works that converts the tests to load Leaflet using import maps (and fully ESM).

This PR also:

  • Removes the spec/index.html file in favor of using Karma's server.
  • Runs Karma in watch mode by default so tests are ran when changes are made to the tests or source.
  • Adds a type annotation so that IDEs can provide documentation and auto completion of the Karma config.
  • Aligns the values of the Karma config as much as possible with the defaults.

@jonkoops jonkoops added the workflow Anything related to Leaflet development process and tools label Apr 27, 2023
@jonkoops jonkoops added this to the 2.0 milestone 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
mourner
mourner approved these changes Apr 27, 2023
View reviewed changes
Copy link
Member

@mourner mourner left a comment

Choose a reason for hiding this comment

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

Very cool to see so much code removed! 🤩

IvanSanchez
IvanSanchez reviewed Apr 27, 2023
View reviewed changes
CONTRIBUTING.md
in a graphical browser. This requires regenerating the bundled files quickly.

In order to do so, run `npm run watch` or `yarn run watch`. This will keep
on rebuilding the bundles whenever any source file changes.

## Running the Tests

To run the tests from the command line, install [Google Chrome](https://www.google.com/chrome/) then run:
Before running the tests make sure that the source code has been built (as mentioned above). If you want to run the tests in the background while working on Leaflet it is recommended you run the build in `watch` mode. This way the tests will automatically re-run when changes to the source code are made.
Copy link
Member

Choose a reason for hiding this comment

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

TODO: run the tests based solely on the unbundled ES modules

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I believe we can get there yeah, not sure if Karma can track the changes between modules. We might need to use a different solution in that case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Created an issue (#8937).

@IvanSanchez IvanSanchez merged commit 058c3de into Leaflet:main Apr 27, 2023
17 checks passed
ES Modules automation moved this from In progress to Done Apr 27, 2023
@jonkoops jonkoops deleted the no-karma-rollup branch April 27, 2023 22:41
@jonkoops jonkoops mentioned this pull request Apr 27, 2023
Open
@Falke-Design
Copy link
Member

Falke-Design commented May 16, 2023 •
edited
Loading

Removes the spec/index.html file in favor of using Karma's server.

I just saw it and I really don't like it. For me it was a great way to quickly create and test my tests. How do you debug now a test?
@jonkoops If there is a way to bring this (in a modern way) back, I would really appreciate it.

@jonkoops
Copy link
Collaborator Author

@Falke-Design I expect this to change when we move from Karma to Web Test Runner. The debugging experience there should be a lot more integrated and intuitive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IvanSanchez IvanSanchez IvanSanchez left review comments

@mourner mourner mourner approved these changes

@Falke-Design Falke-Design Awaiting requested review from Falke-Design

Assignees

@jonkoops jonkoops

Labels
refactoring workflow Anything related to Leaflet development process and tools
Projects
ES Modules
  
Done
Milestone
2.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jonkoops @Falke-Design @mourner @IvanSanchez