From fdd88e0c8475ded68d4a9d0db60a575ca6d84cd2 Mon Sep 17 00:00:00 2001
From: catch <catch@35733.no-reply.drupal.org>
Date: Mon, 26 Oct 2020 08:52:48 +0000
Subject: [PATCH] Issue #3037436 by alexpott, jonathan1055, Wim Leers, catch,
tedbow, longwave: [random test failure] Make QuickEditIntegrationTest more
robust and fail proof
(cherry picked from commit 3d7dd76cdfff2fe11d43a97f637b0dc21de054d6)
---
.../QuickEditImageTest.php | 6 ++-
.../LayoutBuilderQuickEditTest.php | 8 +++-
.../QuickEditIntegrationTest.php | 23 ++++++-----
.../QuickEditJavascriptTestBase.php | 40 +++++++------------
.../tests/modules/hold_test/hold_test.module | 12 ++++++
.../EventSubscriber/HoldTestSubscriber.php | 9 ++++-
.../WebDriverCurlService.php | 3 ++
7 files changed, 59 insertions(+), 42 deletions(-)
diff --git a/core/modules/image/tests/src/FunctionalJavascript/QuickEditImageTest.php b/core/modules/image/tests/src/FunctionalJavascript/QuickEditImageTest.php
index b9cf6196561d..66b2516b4de2 100644
--- a/core/modules/image/tests/src/FunctionalJavascript/QuickEditImageTest.php
+++ b/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'");
diff --git a/core/modules/layout_builder/tests/src/FunctionalJavascript/LayoutBuilderQuickEditTest.php b/core/modules/layout_builder/tests/src/FunctionalJavascript/LayoutBuilderQuickEditTest.php
index 3288f9ba87f0..54569d229bc5 100644
--- a/core/modules/layout_builder/tests/src/FunctionalJavascript/LayoutBuilderQuickEditTest.php
+++ b/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));
}
/**
diff --git a/core/modules/quickedit/tests/src/FunctionalJavascript/QuickEditIntegrationTest.php b/core/modules/quickedit/tests/src/FunctionalJavascript/QuickEditIntegrationTest.php
index 07e88b89c2aa..2328880a8d93 100644
--- a/core/modules/quickedit/tests/src/FunctionalJavascript/QuickEditIntegrationTest.php
+++ b/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"]');
diff --git a/core/modules/quickedit/tests/src/FunctionalJavascript/QuickEditJavascriptTestBase.php b/core/modules/quickedit/tests/src/FunctionalJavascript/QuickEditJavascriptTestBase.php
index acfe3fb2960e..22ddc63ddaac 100644
--- a/core/modules/quickedit/tests/src/FunctionalJavascript/QuickEditJavascriptTestBase.php
+++ b/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 . ')');
}
}
diff --git a/core/modules/system/tests/modules/hold_test/hold_test.module b/core/modules/system/tests/modules/hold_test/hold_test.module
index e08141edf97e..4444f96dcc11 100644
--- a/core/modules/system/tests/modules/hold_test/hold_test.module
+++ b/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);
+ }
}
diff --git a/core/modules/system/tests/modules/hold_test/src/EventSubscriber/HoldTestSubscriber.php b/core/modules/system/tests/modules/hold_test/src/EventSubscriber/HoldTestSubscriber.php
index a9160a46146a..c5264ba1d6f5 100644
--- a/core/modules/system/tests/modules/hold_test/src/EventSubscriber/HoldTestSubscriber.php
+++ b/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)));
}
/**
diff --git a/core/tests/Drupal/FunctionalJavascriptTests/WebDriverCurlService.php b/core/tests/Drupal/FunctionalJavascriptTests/WebDriverCurlService.php
index eb2eb301f36e..a5df8c610819 100644
--- a/core/tests/Drupal/FunctionalJavascriptTests/WebDriverCurlService.php
+++ b/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));
}
--
GitLab