Test(TileLayer): make sure zoomOffset option is used #6011
Conversation
following issue Leaflet#6004 (fixed by PR Leaflet#6006). Added 2 dedicated tests. Unfortunately I could not add a test for detectRetina option that easily, because since the move to Rollup, Browser values became internal variables, and overriding them later on does not have any effect on Leaflet internal references… Not sure how to workaround this for the sake of testing.
There was a problem hiding this comment.
LGTM!
It's not easy to tweak L.Browser.retina :/
I found one solution, but it will set L.Browser.retina to true for all tests. They pass, but I'm still not sure it's the way to go. @ghybs what do you think about this?
|
Hum personally I would not like forcing that flag for all tests. Maybe make a 2-pass process? IIRC, you started modifying the test configuration to be able to test on many browsers. Maybe we could also give a try to Sauce Labs (https://saucelabs.com/open-source free for Open Source projects) |
|
@ghybs yes, I tried to add testing in Chrome(#5845) and Firefox(#5831) to CI, but there are some failing tests which need to be fixed first :/ Anyway, those PRs will not help to test retina. Looks like Sauce Labs is the best solution here (I think they provide testing on mobile platforms as well). |
This PR adds 2 dedicated tests to make sure that the Tile Layer's
zoomOffsetoption is correctly taken into account when building the tiles URL.As mentioned in #6010 (comment)
Follows issue #6004 (fixed by PR #6006).
The tests fail when PR #5822 is active, and pass correctly after PR #6006 is applied.
Unfortunately I could not add a similar test for
detectRetinaoption that easily, because since the move to Rollup build,Browservalues became internal variables, and overriding them later on does not have any effect on Leaflet internal references…Not sure how to workaround this for the sake of testing.