Issue #3211164 by alexpott, catch: Random errors in Javascript Testing

core/modules/system/tests/modules/js_webassert_test/js_webassert_test.routing.yml

@@ -5,3 +5,11 @@ js_webassert_test.js_webassert_test_form:
     _title: 'JsWebAssertForm'
   requirements:
     _access: 'TRUE'
+
+js_webassert_test.js_webassert_test_page:
+  path: '/js_webassert_test_page'
+  defaults:
+    _controller: '\Drupal\js_webassert_test\Controller\TestController::page'
+    _title: 'JsWebAssertPage'
+  requirements:
+    _access: 'TRUE'

core/modules/system/tests/modules/js_webassert_test/src/Controller/TestController.php

+<?php
+
+namespace Drupal\js_webassert_test\Controller;
+
+use Drupal\Core\Render\Markup;
+
+class TestController {
+
+  public function page() {
+    $markup = <<<JS
+<script>
+var timesRun = 0;
+var interval = setInterval(function() {
+  timesRun += 1;
+  // Clear the interval after 1.1 seconds as this is longer than the time
+  // WebDriverCurlService would retry for if retries are enabled but shorter
+  // than the 10 seconds JSWebAssert::waitForElementVisible() waits for.
+  if (timesRun === 1100) {
+    clearInterval(interval);
+  }
+  var p = document.createElement("p");
+  var txt = document.createTextNode("New Text!! ".concat(timesRun));
+  p.setAttribute("id", "test_text");
+  p.appendChild(txt);
+
+  var div = document.getElementById("test_container");
+  div.replaceChild(p, div.childNodes[0]);
+}, 1);
+</script>
+<div id="test_container">
+  <p id="test_text"></p>
+</div>
+JS;
+    return [
+      // Javascript should not be injected into a page this way unless in test
+      // code.
+      '#markup' => Markup::create($markup),
+    ];
+  }
+
+}

core/tests/Drupal/FunctionalJavascriptTests/JSWebAssert.php

@@ -2,11 +2,13 @@
 
 namespace Drupal\FunctionalJavascriptTests;
 
+use Behat\Mink\Element\Element;
 use Behat\Mink\Element\NodeElement;
 use Behat\Mink\Exception\ElementHtmlException;
 use Behat\Mink\Exception\ElementNotFoundException;
 use Behat\Mink\Exception\UnsupportedDriverActionException;
 use Drupal\Tests\WebAssert;
+use WebDriver\Exception\CurlExec;
 
 /**
  * Defines a class with methods for asserting presence of elements during tests.
@@ -64,13 +66,9 @@ function isAjaxing(instance) {
    * @see \Behat\Mink\Element\ElementInterface::findAll()
    */
   public function waitForElement($selector, $locator, $timeout = 10000) {
-    $page = $this->session->getPage();
-
-    $result = $page->waitFor($timeout / 1000, function () use ($page, $selector, $locator) {
+    return $this->waitForHelper($timeout, function (Element $page) use ($selector, $locator) {
       return $page->find($selector, $locator);
     });
-
-    return $result;
   }
 
   /**
@@ -90,13 +88,9 @@ public function waitForElement($selector, $locator, $timeout = 10000) {
    * @see \Behat\Mink\Element\ElementInterface::findAll()
    */
   public function waitForElementRemoved($selector, $locator, $timeout = 10000) {
-    $page = $this->session->getPage();
-
-    $result = $page->waitFor($timeout / 1000, function () use ($page, $selector, $locator) {
+    return (bool) $this->waitForHelper($timeout, function (Element $page) use ($selector, $locator) {
       return !$page->find($selector, $locator);
     });
-
-    return $result;
   }
 
   /**
@@ -116,17 +110,13 @@ public function waitForElementRemoved($selector, $locator, $timeout = 10000) {
    * @see \Behat\Mink\Element\ElementInterface::findAll()
    */
   public function waitForElementVisible($selector, $locator, $timeout = 10000) {
-    $page = $this->session->getPage();
-
-    $result = $page->waitFor($timeout / 1000, function () use ($page, $selector, $locator) {
+    return $this->waitForHelper($timeout, function (Element $page) use ($selector, $locator) {
       $element = $page->find($selector, $locator);
       if (!empty($element) && $element->isVisible()) {
         return $element;
       }
       return NULL;
     });
-
-    return $result;
   }
 
   /**
@@ -137,18 +127,43 @@ public function waitForElementVisible($selector, $locator, $timeout = 10000) {
    * @param int $timeout
    *   (Optional) Timeout in milliseconds, defaults to 10000.
    *
-   * @return \Behat\Mink\Element\NodeElement|null
-   *   The page element node if found and visible, NULL if not.
+   * @return bool
+   *   TRUE if not found, FALSE if found.
    */
   public function waitForText($text, $timeout = 10000) {
-    $page = $this->session->getPage();
-    return $page->waitFor($timeout / 1000, function () use ($page, $text) {
+    return (bool) $this->waitForHelper($timeout, function (Element $page) use ($text) {
       $actual = preg_replace('/\s+/u', ' ', $page->getText());
       $regex = '/' . preg_quote($text, '/') . '/ui';
       return (bool) preg_match($regex, $actual);
     });
   }
 
+  /**
+   * Wraps waits in a function to catch curl exceptions to continue waiting.
+   *
+   * @param int $timeout
+   *   Timeout in milliseconds.
+   * @param callable $callback
+   *   Callback, which result is both used as waiting condition and returned.
+   *
+   * @return mixed
+   *   The result of $callback.
+   */
+  private function waitForHelper(int $timeout, callable $callback) {
+    WebDriverCurlService::disableRetry();
+    $wrapper = function (Element $element) use ($callback) {
+      try {
+        return call_user_func($callback, $element);
+      }
+      catch (CurlExec $e) {
+        return NULL;
+      }
+    };
+    $result = $this->session->getPage()->waitFor($timeout / 1000, $wrapper);
+    WebDriverCurlService::enableRetry();
+    return $result;
+  }
+
   /**
    * Waits for a button (input[type=submit|image|button|reset], button) with
    * specified locator and returns it.

core/tests/Drupal/FunctionalJavascriptTests/Tests/JSWebAssertTest.php

@@ -106,6 +106,12 @@ public function testJsWebAssert() {
     $this->assertNotEmpty($result);
     $this->assertInstanceOf(NodeElement::class, $result);
     $this->assertEquals(TRUE, $result->isVisible());
+
+    $this->drupalGet('js_webassert_test_page');
+    $result = $assert_session->waitForElementVisible('named', ['id', 'test_text']);
+    $this->assertSame('test_text', $result->getAttribute('id'));
+    // Ensure that the javascript has replaced the element 1100 times.
+    $assert_session->pageTextContains('New Text!! 1100');
   }
 
 }

core/tests/Drupal/FunctionalJavascriptTests/WebDriverCurlService.php

@@ -14,6 +14,31 @@
  */
 class WebDriverCurlService extends CurlService {
 
+  /**
+   * Flag that indicates if retries are enabled.
+   *
+   * @var bool
+   */
+  private static $retry = TRUE;
+
+  /**
+   * Enables retries.
+   *
+   * This is useful if the caller is implementing it's own waiting process.
+   */
+  public static function enableRetry() {
+    static::$retry = TRUE;
+  }
+
+  /**
+   * Disables retries.
+   *
+   * This is useful if the caller is implementing it's own waiting process.
+   */
+  public static function disableRetry() {
+    static::$retry = FALSE;
+  }
+
   /**
    * {@inheritdoc}
    */
@@ -22,7 +47,8 @@ public function execute($requestMethod, $url, $parameters = NULL, $extraOptions
       CURLOPT_FAILONERROR => TRUE,
     ];
     $retries = 0;
-    while ($retries < 10) {
+    $max_retries = static::$retry ? 10 : 1;
+    while ($retries < $max_retries) {
       try {
         $customHeaders = [
           'Content-Type: application/json;charset=UTF-8',