Fix date picker being blank and jumpy when appearing #16650
Merged
Conversation
This file contains 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
|
Edit: it's ok actually. I forgot one important line in my previous commit. I amended it. |
cbliard
force-pushed
the
fix/fix-flaky-non-working-spec
branch
from
b6a3464
to
4bf5b57
Compare
It was in order to fix the flaky spec `spec/features/admin/working_days_spec.rb:189`. The flakiness can be forced by adding a `sleep 2` at the beginning of the test, just before the `datepicker.open_modal!` call. Not absolutely sure why so many forced change detection calls are needed, but this way it seems to work. It seems to be related to having a lot of nested templates, outlets and `if`s in templates which each time need a new change detection to be reevaluated. But I'm not an expert in this. As a bonus there is no delay anymore in the drop modal initialization, which should lead to more stable tests. For instance execution time of `spec/features/admin/working_days_spec.rb:189` dropped from 17 secs to 7 secs.
cbliard
force-pushed
the
fix/fix-flaky-non-working-spec
branch
from
4bf5b57
to
3594502
Compare
The modal behavior being more reliable now, the `retry_block` is not needed anymore. Also the `#select_day` method was similar in the parent class. Better keep only the one from the parent class.
It was stupid to make an assertion on the selected day for the datepicker in `#select_day` as the date picker is dismissed half of the time, so the selected element is not there anymore to make assertions on it. Replaced with a direct assertion where it matters and reverted the "safeguard".
modules/team_planner/spec/features/query_handling_spec.rb sometimes misses the dropdown, because it does not wait long enough after adding a filter, and once the filter is applied, the page state is reset and the dropdown is closed, which leads to the error.
oliverguenther
approved these changes
Sep 9, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
It was in order to fix the flaky spec
spec/features/admin/working_days_spec.rb:189. The flakiness can be forced by adding asleep 2at the beginning of the test, just before thedatepicker.open_modal!call.Not absolutely sure why so many forced change detection calls are needed, but this way it seems to work. It seems to be related to having a lot of nested templates, outlets and
ifs in templates which each time need a new change detection to be reevaluated. But I'm not an expert in this.As a bonus there is no delay anymore in the drop modal initialization, which should lead to more stable tests. For instance execution time of
spec/features/admin/working_days_spec.rb:189dropped from 17 secs to 7 secs.FWIW, here is how the error looked on the CI
It was failing at two places: twice when trying to open the modal (it was opened "blank" without the flatpickr initialized on first try, then closed by clicking "Cancel" button on second try, then 3 times when trying to set the date (because it's closed).