PB-298: Fix race condition when exporting/saving kml#649
Merged
Conversation
On the CI this E2E test is sometimes failing in the icon size check in KML payload. Although this commit should not fix this issue, it removes some bad practice of `force: true` flag and make sure that element are visible before clicking which is a good practice (avoiding clicking on element not yet fully rendered and avoiding race condition).
Remove the useless wait on icon, changing icon size don't trigger any new icon request anymore as all icon are loading from the backend using the same size. Remove a wrong closing action, at this time of the test the icon size selector is already closed.
Passing run #654 ↗︎
Details:
Review all test suite changes for PR #649 ↗︎ |
|||||||||||||||
The icon size test did failed time to time. This was due to a race condition. The icon scale in KML is set by openlayer and uses a scale factor based on the icon size. Now if the icon has not yet be loaded during the KML write, openlayer takes a default size of 64 pixel which defer from our icon size of 48 pixel. Due to this the icon scale of a large icon was 2 instead of 1.5. We solve this race condition by waiting on all icons being loaded before doing the icon size changes. NOTE this is not an issue in real life as the save drawing is always delayed at least 2 seconds which let plenty of time to load the icons. In cypress this delay was reduced to speed up testing.
To export a kml, openlayer requires the icon size in order to compute the scale. If the icon has not yet been loaded when generating the style then the icon scale would be computed with a default size of 64px. The last commit tried to solve this issue in cypress test, but unfortunately waiting on the icon to be loaded did not always solved the race condition. I could not find a better way to ensure this as settings the size when generating the icon style. This solve temporarly the issue, but in the future we should get the size from the backend with the others icon metadata.
c57e920 to
85dedaf
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
To export a kml, openlayer requires the icon size in order to compute the
scale. If the icon has not yet been loaded when generating the style then
the icon scale would be computed with a default size of 64px.
The last commit tried to solve this issue in cypress test, but unfortunately
waiting on the icon to be loaded did not always solved the race condition.
I could not find a better way to ensure this as settings the size when
generating the icon style.
This solve temporarly the issue, but in the future we should get the size
from the backend with the others icon metadata.
The icon size test did failed time to time. This was due to a race condition.
The icon scale in KML is set by openlayer and uses a scale factor based on the
icon size. Now if the icon has not yet be loaded during the KML write, openlayer
takes a default size of 64 pixel which defer from our icon size of 48 pixel.
Due to this the icon scale of a large icon was 2 instead of 1.5.
We solve this race condition by waiting on all icons being loaded before doing
the icon size changes.
Also did small test improvements:
Remove the useless wait on icon, changing icon size don't trigger any new
icon request anymore as all icon are loading from the backend using the same
size.
Remove a wrong closing action, at this time of the test the icon size selector
is already closed.
Test link