Issue #3037436 by alexpott, jonathan1055, Wim Leers, catch, tedbow, longwave:...

Issue #3037436 by alexpott, jonathan1055, Wim Leers, catch, tedbow, longwave: [random test failure] Make QuickEditIntegrationTest more robust and fail proof

(cherry picked from commit 3d7dd76c)

core/modules/image/tests/src/FunctionalJavascript/QuickEditImageTest.php

@@ -20,7 +20,7 @@ class QuickEditImageTest extends QuickEditJavascriptTestBase {
   /**
    * {@inheritdoc}
    */
-  protected static $modules = ['node', 'image', 'field_ui'];
+  protected static $modules = ['node', 'image', 'field_ui', 'hold_test'];
 
   /**
    * {@inheritdoc}
@@ -178,6 +178,7 @@ public function testImageInPlaceEditor() {
     $this->prepareRequest();
 
     // Click 'Save'.
+    hold_test_response(TRUE);
     $this->saveQuickEdit();
     $this->assertEntityInstanceStates([
       'node/1[0]' => 'committing',
@@ -189,9 +190,10 @@ public function testImageInPlaceEditor() {
       'node/1/body/en/full'                => 'candidate',
       'node/1/' . $field_name . '/en/full' => 'saving',
     ]);
-    $this->assertEntityInstanceFieldMarkup('node', 1, 0, [
+    $this->assertEntityInstanceFieldMarkup([
       'node/1/' . $field_name . '/en/full' => '.quickedit-changed',
     ]);
+    hold_test_response(FALSE);
 
     // Wait for the saving of the image field to complete.
     $this->assertJsCondition("Drupal.quickedit.collections.entities.get('node/1[0]').get('state') === 'closed'");

core/modules/layout_builder/tests/src/FunctionalJavascript/LayoutBuilderQuickEditTest.php

@@ -202,8 +202,12 @@ protected function assertEntityInstanceFieldStates($entity_type_id, $entity_id,
   /**
    * {@inheritdoc}
    */
-  protected function assertEntityInstanceFieldMarkup($entity_type_id, $entity_id, $entity_instance_id, array $expected_field_attributes) {
-    parent::assertEntityInstanceFieldMarkup($entity_type_id, $entity_id, $entity_instance_id, $this->replaceLayoutBuilderFieldIdKeys($expected_field_attributes));
+  protected function assertEntityInstanceFieldMarkup($expected_field_attributes) {
+    if (func_num_args() === 4) {
+      $expected_field_attributes = func_get_arg(3);
+      @trigger_error('Calling ' . __METHOD__ . '() with 4 arguments is deprecated in drupal:9.1.0 and will throw an error in drupal:10.0.0. See https://www.drupal.org/project/drupal/issues/3037436', E_USER_DEPRECATED);
+    }
+    parent::assertEntityInstanceFieldMarkup($this->replaceLayoutBuilderFieldIdKeys($expected_field_attributes));
   }
 
   /**

core/modules/quickedit/tests/src/FunctionalJavascript/QuickEditIntegrationTest.php

@@ -179,7 +179,7 @@ public function testArticleNode() {
       'node/1/body/en/full'       => 'candidate',
       'node/1/field_tags/en/full' => 'candidate',
     ]);
-    $this->assertEntityInstanceFieldMarkup('node', 1, 0, [
+    $this->assertEntityInstanceFieldMarkup([
       'node/1/title/en/full' => '[contenteditable="true"]',
     ]);
 
@@ -208,9 +208,7 @@ public function testArticleNode() {
     ]);
     hold_test_response(FALSE);
 
-    // Wait for CKEditor to load, then verify it has.
-    $this->assertJsCondition('CKEDITOR.status === "loaded"');
-    $this->assertEntityInstanceFieldMarkup('node', 1, 0, [
+    $this->assertEntityInstanceFieldMarkup([
       'node/1/body/en/full'       => '.cke_editable_inline',
       'node/1/field_tags/en/full' => ':not(.quickedit-editor-is-popup)',
     ]);
@@ -231,7 +229,7 @@ public function testArticleNode() {
       'node/1/field_tags/en/full' => 'activating',
       'node/1/title/en/full'      => 'candidate',
     ]);
-    $this->assertEntityInstanceFieldMarkup('node', 1, 0, [
+    $this->assertEntityInstanceFieldMarkup([
       'node/1/title/en/full'      => '.quickedit-changed',
       'node/1/field_tags/en/full' => '.quickedit-editor-is-popup',
     ]);
@@ -272,17 +270,23 @@ public function testArticleNode() {
       'node/1/field_tags/en/full' => 'saving',
       'node/1/title/en/full'      => 'candidate',
     ]);
-    hold_test_response(FALSE);
-    $this->assertEntityInstanceFieldMarkup('node', 1, 0, [
+    $this->assertEntityInstanceFieldMarkup([
       'node/1/title/en/full'      => '.quickedit-changed',
       'node/1/field_tags/en/full' => '.quickedit-changed',
     ]);
+    hold_test_response(FALSE);
 
     // Wait for the saving of the tags field to complete.
     $this->assertJsCondition("Drupal.quickedit.collections.entities.get('node/1[0]').get('state') === 'closed'");
     $this->assertEntityInstanceStates([
       'node/1[0]' => 'closed',
     ]);
+
+    // Get the load again and ensure the values are the expected values.
+    $this->drupalGet('node/' . $node->id());
+    $this->assertSession()->pageTextContains(' Llamas are awesome!');
+    $this->assertSession()->linkExists('foo');
+    $this->assertSession()->linkExists('bar');
   }
 
   /**
@@ -336,10 +340,7 @@ public function testCustomBlock() {
     $this->assertEntityInstanceFieldStates('block_content', 1, 0, [
       'block_content/1/body/en/full' => 'active',
     ]);
-
-    // Wait for CKEditor to load, then verify it has.
-    $this->assertJsCondition('CKEDITOR.status === "loaded"');
-    $this->assertEntityInstanceFieldMarkup('block_content', 1, 0, [
+    $this->assertEntityInstanceFieldMarkup([
       'block_content/1/body/en/full' => '.cke_editable_inline',
     ]);
     $this->assertSession()->elementExists('css', '#quickedit-entity-toolbar .quickedit-toolgroup.wysiwyg-main > .cke_chrome .cke_top[role="presentation"] .cke_toolbar[role="toolbar"] .cke_toolgroup[role="presentation"] > .cke_button[title~="Bold"][role="button"]');

core/modules/quickedit/tests/src/FunctionalJavascript/QuickEditJavascriptTestBase.php

@@ -238,42 +238,30 @@ function () {
     foreach ($expected_field_states as $quickedit_field_id => $expected_field_state) {
       $expected_field_attributes[$quickedit_field_id] = static::$expectedFieldStateAttributes[$expected_field_state];
     }
-    $this->assertEntityInstanceFieldMarkup($entity_type_id, $entity_id, $entity_instance_id, $expected_field_attributes);
+    $this->assertEntityInstanceFieldMarkup($expected_field_attributes);
   }
 
   /**
    * Asserts all in-place editable fields with markup expectations.
    *
-   * @param string $entity_type_id
-   *   The entity type ID.
-   * @param int $entity_id
-   *   The entity ID.
-   * @param int $entity_instance_id
-   *   The entity instance ID. (Instance on the page.)
    * @param array $expected_field_attributes
    *   Must describe the expected markup attributes for all given in-place
    *   editable fields.
+   *
+   * @todo https://www.drupal.org/project/drupal/issues/3178758 Remove
+   *   deprecation layer and add array typehint.
    */
-  protected function assertEntityInstanceFieldMarkup($entity_type_id, $entity_id, $entity_instance_id, array $expected_field_attributes) {
-    $entity_page_id = $entity_type_id . '/' . $entity_id . '[' . $entity_instance_id . ']';
-    $expected_field_attributes_json = json_encode($expected_field_attributes);
-    $js_match_field_element_attributes = <<<JS
-function () {
-  var expectations = $expected_field_attributes_json;
-  var entityCollection = Drupal.quickedit.collections.entities;
-  var entityModel = entityCollection.get('$entity_page_id');
-  return entityModel.get('fields').reduce(function (result, fieldModel) {
-    var fieldID = fieldModel.get('fieldID');
-    var element = fieldModel.get('el');
-    var matches = element.webkitMatchesSelector(expectations[fieldID]);
-    result[fieldID] = matches ? matches : element.outerHTML;
-    return result;
-  }, {});
-}()
-JS;
-    $result = $this->getSession()->evaluateScript($js_match_field_element_attributes);
+  protected function assertEntityInstanceFieldMarkup($expected_field_attributes) {
+    if (func_num_args() === 4) {
+      $expected_field_attributes = func_get_arg(3);
+      @trigger_error('Calling ' . __METHOD__ . '() with 4 arguments is deprecated in drupal:9.1.0 and will throw an error in drupal:10.0.0. See https://www.drupal.org/project/drupal/issues/3037436', E_USER_DEPRECATED);
+    }
+    if (!is_array($expected_field_attributes)) {
+      throw new \InvalidArgumentException('The $expected_field_attributes argument must be an array.');
+    }
     foreach ($expected_field_attributes as $quickedit_field_id => $expectation) {
-      $this->assertTrue($result[$quickedit_field_id], 'Field ' . $quickedit_field_id . ' did not match its expectation selector (' . $expectation . '), actual HTML: ' . $result[$quickedit_field_id]);
+      $element = $this->assertSession()->waitForElementVisible('css', '[data-quickedit-field-id="' . $quickedit_field_id . '"]' . $expectation);
+      $this->assertNotEmpty($element, 'Field ' . $quickedit_field_id . ' did not match its expectation selector (' . $expectation . ')');
     }
   }
 

core/modules/system/tests/modules/hold_test/hold_test.module

@@ -5,6 +5,8 @@
  * Contains functions for testing hold request/response.
  */
 
+use Drupal\hold_test\EventSubscriber\HoldTestSubscriber;
+
 /**
  * Request hold.
  *
@@ -14,6 +16,11 @@
 function hold_test_request($status) {
   $site_path = \Drupal::getContainer()->getParameter('site.path');
   file_put_contents($site_path . '/hold_test_request.txt', $status);
+  // If we're releasing the hold wait for a bit to allow the subscriber to read
+  // the file.
+  if (!$status) {
+    usleep(HoldTestSubscriber::WAIT * 2);
+  }
 }
 
 /**
@@ -25,4 +32,9 @@ function hold_test_request($status) {
 function hold_test_response($status) {
   $site_path = \Drupal::getContainer()->getParameter('site.path');
   file_put_contents($site_path . '/hold_test_response.txt', $status);
+  // If we're releasing the hold wait for a bit to allow the subscriber to read
+  // the file.
+  if (!$status) {
+    usleep(HoldTestSubscriber::WAIT * 2);
+  }
 }

core/modules/system/tests/modules/hold_test/src/EventSubscriber/HoldTestSubscriber.php

@@ -13,6 +13,13 @@ class HoldTestSubscriber implements EventSubscriberInterface {
   const HOLD_REQUEST = 'request';
   const HOLD_RESPONSE = 'response';
 
+  /**
+   * Time in microseconds to wait for before checking if the file is updated.
+   *
+   * @var int
+   */
+  const WAIT = 100000;
+
   /**
    * The site path.
    *
@@ -54,7 +61,7 @@ protected function hold($type) {
     $path = "{$this->sitePath}/hold_test_$type.txt";
     do {
       $status = (bool) file_get_contents($path);
-    } while ($status && (NULL === usleep(100000)));
+    } while ($status && (NULL === usleep(static::WAIT)));
   }
 
   /**

core/tests/Drupal/FunctionalJavascriptTests/WebDriverCurlService.php

@@ -114,6 +114,9 @@ public function execute($requestMethod, $url, $parameters = NULL, $extraOptions
         $retries++;
       }
     }
+    if (empty($error)) {
+      $error = "Retries: $retries and last result:\n" . ($rawResult ?? '');
+    }
     throw WebDriverException::factory(WebDriverException::CURL_EXEC, sprintf("Curl error thrown for http %s to %s%s\n\n%s", $requestMethod, $url, $parameters && is_array($parameters) ? ' with params: ' . json_encode($parameters) : '', $error));
   }