More \Drupal\Tests\outside_in\FunctionalJavascript\OutsideInBlockFormTest random fails

Drupal\Tests\outside_in\FunctionalJavascript\OutsideInJavascriptTestBase:

/**
 * Waits for Off-canvas tray to open.
 */
protected function waitForOffCanvasToOpen() {
  $web_assert = $this->assertSession();
  $web_assert->assertWaitOnAjaxRequest();
  $this->waitForElement('#drupal-offcanvas');
}

//...

/**
 * Waits for an element to appear on the page.
 *
 * @param string $selector
 *   CSS selector.
 * @param int $timeout
 *   (optional) Timeout in milliseconds, defaults to 10000.
 */
protected function waitForElement($selector, $timeout = 10000) {
  $condition = "(jQuery('$selector').length > 0)";
  $this->assertJsCondition($condition, $timeout);
}

:(

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).

Random test failures are critical in general regardless of the release cycle phase. Thanks!

@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()

Sorry, 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..

Looked 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.

Question/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?

@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

Patch 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.

Reupload for #6 for RTBC.

I'll create the follow-up later on.

Committed/pushed to 8.4.x and cherry-picked to 8.3.x. Thanks!

@catch Thanks for committing!

Changing to new settings_tray.module component. @drpal thanks for script help! :)