Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
More random fails in outside_in that are happening:
https://www.drupal.org/pift-ci-job/585941
https://www.drupal.org/pift-ci-job/581480
Setting critical because it is causing RTBC issue bouncing during 8.3.x-almost-alpha
Comments
Comment #2
tacituseu CreditAttribution: tacituseu commentedComment #3
tacituseu CreditAttribution: tacituseu commentedDrupal\Tests\outside_in\FunctionalJavascript\OutsideInJavascriptTestBase
:Comment #4
michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commented:(
Maybe we should try to use the new available waitForElementVisible() and drop the custom implementations. Will work on that later today (in a few hours).
Comment #5
xjmRandom test failures are critical in general regardless of the release cycle phase. Thanks!
Comment #6
tedbow@michielnugter that sounds like good idea.
Here is patch that removes OutsideInJavascriptTestBase::waitForElement() and replaces all the calls with calls to \Drupal\FunctionalJavascriptTests\JSWebAssert::waitForElementVisible()
Comment #7
michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commentedSorry, couldn't get to it anymore yesterday.
Let's try a 50x for that patch. This should somehow be standard for each patch containing anything JTB related..
Comment #8
michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commentedComment #10
michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commentedLooked at the dispatcher, all tests passed. 100x the same test is somewhere near the limit for JTB, because of the 4 tests in test it took too long.
Adding a 15x variant and will add the complete range and reuploaded the original patch for RTBC consideration.
EDIT: Wow, sorry, very stupid mistake. Only the comment was updated to 15, not the actual array fill. Will checkt the dispatcher later on when the tests finish.. Sorry about that.
Comment #11
michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commentedQuestion/consideration: The waitForElementVisible method only checks if an element is visible. It does not check if there are animations running on the element. Could this be a problem and cause for random fails?
Comment #13
tedbow@michielnugter another good idea! I think we should a cautious as we can with this test since there are have been so many random failures.
This patch creates a new method waitForElementVisibleAndAjaxRequest()
which calls waitForElementVisible() and assertWaitOnAjaxRequest()
All calls now go to this wrapper. This allows waitForOffCanvasToOpen() be simplified.
I also noticed there was 1 remaining call that was sending a value for $timeout. But it is 10000 which is the default anyways. So removing that so that if the global default ever changes.
UPDATE: chatted with @michielnugter he was actually talking about adding an "animation" check directly to \Drupal\FunctionalJavascriptTests\JSWebAssert::waitForElement.
That makes more sense
Comment #14
michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commentedPatch based on -7 with the addition of the animation check.
EDIT: checked dispatcher on the original -7 patch, everything passes untill the test-bot gave up.
Discussed with @tedbow: The animation addition might still be valuable but should not be part of this issue. Follow-up issue should be opened.
Comment #15
michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commentedReupload for #6 for RTBC.
I'll create the follow-up later on.
Comment #16
catchCommitted/pushed to 8.4.x and cherry-picked to 8.3.x. Thanks!
Comment #19
tedbow@catch Thanks for committing!
Comment #21
xjmComment #22
Anonymous (not verified) CreditAttribution: Anonymous commentedСontinued in #2901626: CSS animations cause \Drupal\Tests\outside_in\FunctionalJavascript\OutsideInBlockFormTest random fails.
Comment #23
Anonymous (not verified) CreditAttribution: Anonymous commented#14:
done.
Comment #24
tedbowChanging to new settings_tray.module component. @drpal thanks for script help! :)