Convert web tests to browser tests for field_ui module

This is ready again to be worked on.

Re-rolled.

No longer applies.

Re-rolled.

I expect this to fail some tests because it seems BrowserTestBase cannot handle hidden and image input elements, e.g.

Form field with id|name|label|value "refresh_rows"

EDIT: connected comment to test error.

This fixes a few failures, but the bug about BrowserTestBase failing to handle hidden and image input elements still remains.

There are a couple of (recurrent) issues in the tests, which my latest patch tries to resolve based on Jo Fitzgeralds' patch.

1. Simpletests asserts non-visible, javascript based, input fields

The Field UI adds a input[type=”hidden”] field for refreshing fields that might have been changed.
It also adds a hidden submit button with label ‘Refresh’, that is clicked through javascript (field_ui.js)

In several cases the clicking of the hidden ‘refresh’ button is asserted by the legacy test. When clicking the 'Refresh' button, values are passed to the hidden refresh_fields fields.

The test fails because Mink doesn’t allow clicking of hidden buttons: the FieldExists() method returns FALSE, because for Mink, input[type="hidden"] is not an existing field.

On the other hand, the functionality that is tested concerns a 100% JS test case. The BrowserTestBase is about testing non JS, visual user interaction, so IMO the only solution is to remove these specific assertions of the test.

About the refresh_rows and the refresh button in EntityDisplayFormBase.php:224

In overviews involving nested rows from contributed modules (i.e field_group), the 'plugin type' selects can trigger a series of changes in child rows. The #ajax behavior is therefore not attached directly to the selects, but triggered by the client-side script through a hidden #ajax 'Refresh' button.

2. The ::assertFieldByID method fails asserting ‘buttons’.

There is a patch for that. I’ve included the patch in this path to make the tests work.

https://www.drupal.org/node/2862947

3. FieldDefinitionInterFace::getDefaultLiteral() returns an array().

ManageFieldTests::testDefaultValue() asserts that the getDefaultsLiteral() should return NULL (no default configuration). According to the interface it always returns an array. If there is no default value, the array is empty.

4. testLockedField() assumes that xpath() returns an array, but it returns an array of NodeElement when using Mink.

Therefore the the assertion has to be changed to use the NodeElement that the xpath() method returns and check it for the presence of the ‘Locked’ string.

My patch included some comments, and differences in 8.4.x that I hadn't pulled.

These ones are clean:

- One including patch of https://www.drupal.org/node/2862947 - which should pass.
- One excluding patch of https://www.drupal.org/node/2862947 - which will not pass.

@nuez it is really helpful if you also add interdiffs with your patches - it makes them much easier to review.

Thanks for the tip @Jo Fitzgerald. Interdiff added.

There was still a failing test.

In testFormatterUI() there is an ajax interaction involving clicking the hidden 'refresh' button that I just simply removed. I'll try and revise this tomorrow, because I don't think it's the right solution.

For now I think the test passes when applying with the 2862947-8.patch.

@nuez++ for the interdiff :D

Keep an eye out for using the short array syntax: [] instead of array().

Assuming this passes the tests then it's probably worth postponing it until #2862947: Incorrect field assertions in AssertLegacyTrait has been accepted into core (it's not the only ticket waiting on that).

1. +++ b/core/modules/field_ui/tests/src/Functional/FieldUiTestTrait.php
   @@ -84,9 +84,9 @@ public function fieldUIAddExistingField($bundle_path, $existing_storage_name, $l
   +    $content = reset($main_content)->getHtml();
   +    $this->assertTrue(FALSE !== strpos($content, 'field-config-edit-form'), 'The field config edit form is present.');
   +    $this->assertTrue(FALSE === strpos($content, '<'), 'The page does not have double escaped HTML tags.');
   

   instead of reset() you could use $main_content[0].

   instead of assertTrue() you should use assertContains() for checking strings.

2. +++ b/core/modules/field_ui/tests/src/Functional/ManageDisplayTest.php
   @@ -85,7 +85,7 @@ public function testFormatterUI() {
   -      return (string) $item->attributes()->value[0];
   +      return (string) $item->getValue();
   

   the (string) cast can now be removed.

3. +++ b/core/modules/field_ui/tests/src/Functional/ManageDisplayTest.php
   @@ -200,13 +194,15 @@ public function testFormatterUI() {
   -    $this->assertFieldByName('field_test_settings_edit');
   +    $this->assertNotEmpty($this->xpath($this->cssSelectToXpath('input[name="field_test_settings_edit"]')));
   

   why do we need to change this? shouldn't this just work? Something missing in assertFieldByName()

4. +++ b/core/modules/field_ui/tests/src/Functional/ManageDisplayTest.php
   @@ -247,7 +243,7 @@ public function testWidgetUI() {
   -      return (string) $item->attributes()->value[0];
   +      return (string) $item->getValue();
        }, $result);
   

   remove (string) cast.

5. +++ b/core/modules/field_ui/tests/src/Functional/ManageDisplayTest.php
   @@ -508,7 +503,7 @@ public function assertNodeViewTextHelper(EntityInterface $node, $view_mode, $tex
   -    $old_content = $this->getRawContent();
   +    $old_content = $this->getSession()->getPage()->getContent();
   

   getRawContent() exists now, so this change should be removed.

6. +++ b/core/modules/field_ui/tests/src/Functional/ManageDisplayTest.php
   @@ -516,15 +511,9 @@ public function assertNodeViewTextHelper(EntityInterface $node, $view_mode, $tex
   +    $exists = (FALSE !== strpos($output, $text));
   +    $this->assertTrue(($not_exists !== $exists), $message);
   

   assertContains()

7. +++ b/core/modules/field_ui/tests/src/Functional/ManageDisplayTest.php
   @@ -558,23 +545,16 @@ protected function assertFieldSelectOptions($name, array $expected_options) {
   +   * @param array $elements
   +   *   The options from the select, found with a call to $this->xpath().
   

   should be \Behat\Mink\Element\NodeElement[] as type

8. +++ b/core/modules/field_ui/tests/src/Functional/ManageDisplayTest.php
   @@ -558,23 +545,16 @@ protected function assertFieldSelectOptions($name, array $expected_options) {
   +      return (string) $item->getValue();
   

   remove (string)

9. +++ b/core/modules/field_ui/tests/src/Functional/ManageFieldsTest.php
   @@ -153,17 +154,17 @@ public function manageFieldsPage($type = '') {
   -          $this->assertIdentical($url, (string) $link['href']);
   +          $this->assertIdentical($url, (string) $link->getAttribute('href'));
   

   same here, remove (string). Also elsewhere.

10. +++ b/core/modules/field_ui/tests/src/Functional/ManageFieldsTest.php
    @@ -449,7 +450,7 @@ public function testDefaultValue() {
    -    $this->assertEqual($field->getDefaultValueLiteral(), NULL, 'The default value was correctly saved.');
    +    $this->assertEqual($field->getDefaultValueLiteral(), array(), 'The default value was correctly saved.');
    

    use short array syntax

11. +++ b/core/modules/field_ui/tests/src/Functional/ManageFieldsTest.php
    @@ -564,7 +565,9 @@ public function testLockedField() {
    +    /** @var NodeElement $nodeElement */
    

    @var docs should be fully qualified namespaces.

12. +++ b/core/modules/field_ui/tests/src/Functional/ManageFieldsTest.php
    @@ -564,7 +565,9 @@ public function testLockedField() {
    +    $this->assertTrue('Locked' == $lockedNodeElement->getHtml(), 'Field is marked as Locked in the UI');
    

    use assertSame() instead.

13. +++ b/core/modules/field_ui/tests/src/Functional/ManageFieldsTest.php
    --- a/core/modules/file/src/Tests/FileFieldWidgetTest.php
    +++ b/core/modules/file/src/Tests/FileFieldWidgetTest.php
    

    looks like you forgot to move the file?

Postponing on #2862947: Incorrect field assertions in AssertLegacyTrait

Solved 1,2,4,5,6,8,9,10,12

3. assertFieldByName() calles FieldExists() on a input[type="image"] button (the wee cog), which is not recognised by Mink. That's why I converted it to an xpath assertion. Should we change assertFieldByName() instead?

7. I've changed the argument docs from array to \Behat\Mink\Element\NodeElement[]. PHP doesn't support strongly typed arrays of objects as arguments so that will have to remain array.

11. Done.

13. The FieldUiTestTrait trait is used by many other modules so I've left and deprecated the old trait. Field UI already uses the new trait, but other modules continue using the old trait.

Postponed on: #2809501: Convert AJAX part of \Drupal\field_ui\Tests\ManageDisplayTest to JavascriptTestBase.

There are currently only a few tests left, and only ManageDisplayTest needs the functional javascript treatment.

So I'd un-postpone here so we could convert the other ones and, more importantly, move FieldUiTestTrait to field_ui/tests/src/Traits/. However: ManageFieldsTest uses Drupal\field\Tests\EntityReference\EntityReferenceTestTrait which we should move out of simpletest-land.

So it's fair to say we're now postponed on #2996789: Deprecate Drupal\field\Tests\EntityReference\EntityReferenceTestTrait

blocker has landed.

This does it for the remaining tests, except ManageDisplayTest for which we have a follow up #2809501: Convert AJAX part of \Drupal\field_ui\Tests\ManageDisplayTest to JavascriptTestBase

Deprecated the old trait and added a CR for that and removed usage in PHPUnit test using it, left it in tests still using WebTestBase.

No interdiff because too much has changed since the last patch. I've tried to reuse as much as possible for earlier patch but a lot has changed, so much I just redid.

Missed one.

edit: interdiff is a bit weird, showing stuff that isn't true

+++ b/core/modules/field_ui/tests/src/Functional/ManageFieldsFunctionalTest.php
@@ -564,11 +565,7 @@ public function testLockedField() {
-    $edit_link = $this->xpath('//tr[@id=:field_name]/td[4]', [':field_name' => $field_name]);
-    $this->assertFalse(in_array('edit', $edit_link), 'Edit option for locked field is not present the UI');
-    $delete_link = $this->xpath('//tr[@id=:field_name]/td[4]', [':field_name' => $field_name]);
-    $this->assertFalse(in_array('delete', $delete_link), 'Delete option for locked field is not present the UI');

What about these asserts?

@jibran, well spotted, I knew I should have put that in a comment here :-)

+++ b/core/modules/field_ui/tests/src/Functional/ManageFieldsFunctionalTest.php
@@ -564,11 +565,7 @@ public function testLockedField() {
     $locked = $this->xpath('//tr[@id=:field_name]/td[4]', [':field_name' => $field_name]);
...
+    $this->assertSame('Locked', $locked[0]->getHtml(), 'Field is marked as Locked in the UI');

With the rewrite we now check the full HTML of that xpath (they all use the same xpath query), so if this is true the others must be false.
negative assertions--
So, I think, they serve no purpose, so they can just go.

Thanks, for explaining it.

@jibran thanks for asking that - I was wondering the same.

Committed 982c174 and pushed to 8.7.x. Thanks!
Committed 58746b5 and pushed to 8.6.x. Thanks! Backported without the deprecation of FieldUiTestTrait to keep the tests in-sync between the branches.

diff --git a/core/modules/field_ui/src/Tests/FieldUiTestTrait.php b/core/modules/field_ui/src/Tests/FieldUiTestTrait.php
index 935821c1f8..1c6d5cb0a2 100644
--- a/core/modules/field_ui/src/Tests/FieldUiTestTrait.php
+++ b/core/modules/field_ui/src/Tests/FieldUiTestTrait.php
@@ -2,12 +2,12 @@
 
 namespace Drupal\field_ui\Tests;
 
-@trigger_error(__NAMESPACE__ . '\FieldUiTestTrait is deprecated in Drupal 8.6.x and will be removed before Drupal 9.0.0. Instead, use \Drupal\Tests\field_ui\Traits\FieldUiTestTrait. See https://www.drupal.org/node/3001664', E_USER_DEPRECATED);
+@trigger_error(__NAMESPACE__ . '\FieldUiTestTrait is deprecated in Drupal 8.7.x and will be removed before Drupal 9.0.0. Instead, use \Drupal\Tests\field_ui\Traits\FieldUiTestTrait. See https://www.drupal.org/node/3001664', E_USER_DEPRECATED);
 
 /**
  * Provides common functionality for the Field UI test classes.
  *
- * @deprecated in Drupal 8.6.x and will be removed before Drupal 9.0.0.
+ * @deprecated in Drupal 8.7.x and will be removed before Drupal 9.0.0.
  *   Use \Drupal\Tests\field_ui\Traits\FieldUiTestTrait. See
  *   https://www.drupal.org/node/3001664
  */

After recent discussions only doing deprecations in the next minor for the time being.

publish the change record