Issue #2868019 by michielnugter, jonathan1055, Lendude, dawehner, alexpott: AssertLegacyTrait field assertions not compatible with Simpletest assertions

core/modules/system/tests/modules/test_page_test/src/Form/TestForm.php

@@ -64,6 +64,18 @@ class TestForm extends FormBase {
       '#default_value' => 2,
     ];
 
+    $form['duplicate_button'] = [
+      '#type' => 'submit',
+      '#name' => 'duplicate_button',
+      '#value' => 'Duplicate button 1',
+    ];
+
+    $form['duplicate_button_2'] = [
+      '#type' => 'submit',
+      '#name' => 'duplicate_button',
+      '#value' => 'Duplicate button 2',
+    ];
+
     $form['save'] = [
       '#type' => 'submit',
       '#value' => $this->t('Save'),

core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php

@@ -2,7 +2,6 @@
 
 namespace Drupal\FunctionalTests;
 
-use Behat\Mink\Exception\ElementNotFoundException;
 use Behat\Mink\Exception\ExpectationException;
 use Behat\Mink\Selector\Xpath\Escaper;
 use Drupal\Component\Render\FormattableMarkup;
@@ -220,13 +219,11 @@ trait AssertLegacyTrait {
    *
    * @deprecated Scheduled for removal in Drupal 9.0.0.
    *   Use $this->assertSession()->fieldExists() or
+   *   $this->assertSession()->buttonExists() or
    *   $this->assertSession()->fieldValueEquals() instead.
    */
   protected function assertFieldByName($name, $value = NULL) {
-    $this->assertSession()->fieldExists($name);
+    $this->assertFieldByXPath($this->constructFieldXpath('name', $name), $value);
-    if ($value !== NULL) {
-      $this->assertSession()->fieldValueEquals($name, (string) $value);
-    }
   }
 
   /**
@@ -242,15 +239,11 @@ trait AssertLegacyTrait {
    *
    * @deprecated Scheduled for removal in Drupal 9.0.0.
    *   Use $this->assertSession()->fieldNotExists() or
+   *   $this->assertSession()->buttonNotExists() or
    *   $this->assertSession()->fieldValueNotEquals() instead.
    */
   protected function assertNoFieldByName($name, $value = '') {
-    if ($this->getSession()->getPage()->findField($name) && isset($value)) {
+    $this->assertNoFieldByXPath($this->constructFieldXpath('name', $name), $value);
-      $this->assertSession()->fieldValueNotEquals($name, (string) $value);
-    }
-    else {
-      $this->assertSession()->fieldNotExists($name);
-    }
   }
 
   /**
@@ -268,19 +261,11 @@ trait AssertLegacyTrait {
    *
    * @deprecated Scheduled for removal in Drupal 9.0.0.
    *   Use $this->assertSession()->fieldExists() or
+   *   $this->assertSession()->buttonExists() or
    *   $this->assertSession()->fieldValueEquals() instead.
    */
   protected function assertFieldById($id, $value = '') {
-    $xpath = $this->assertSession()->buildXPathQuery('//textarea[@id=:value]|//input[@id=:value]|//select[@id=:value]', [':value' => $id]);
+    $this->assertFieldByXPath($this->constructFieldXpath('id', $id), $value);
-    $field = $this->getSession()->getPage()->find('xpath', $xpath);
-
-    if (empty($field)) {
-      throw new ElementNotFoundException($this->getSession()->getDriver(), 'form field', 'id', $field);
-    }
-
-    if ($value !== NULL) {
-      $this->assertEquals($value, $field->getValue());
-    }
   }
 
   /**
@@ -290,23 +275,25 @@ trait AssertLegacyTrait {
    *   Name or ID of field to assert.
    *
    * @deprecated Scheduled for removal in Drupal 9.0.0.
-   *   Use $this->assertSession()->fieldExists() instead.
+   *   Use $this->assertSession()->fieldExists() or
+   *   $this->assertSession()->buttonExists() instead.
    */
   protected function assertField($field) {
-    $this->assertSession()->fieldExists($field);
+    $this->assertFieldByXPath($this->constructFieldXpath('name', $field) . '|' . $this->constructFieldXpath('id', $field));
   }
 
   /**
-   * Asserts that a field exists with the given name or ID does NOT exist.
+   * Asserts that a field does NOT exist with the given name or ID.
    *
    * @param string $field
    *   Name or ID of field to assert.
    *
    * @deprecated Scheduled for removal in Drupal 9.0.0.
-   *   Use $this->assertSession()->fieldNotExists() instead.
+   *   Use $this->assertSession()->fieldNotExists() or
+   *   $this->assertSession()->buttonNotExists() instead.
    */
   protected function assertNoField($field) {
-    $this->assertSession()->fieldNotExists($field);
+    $this->assertNoFieldByXPath($this->constructFieldXpath('name', $field) . '|' . $this->constructFieldXpath('id', $field));
   }
 
   /**
@@ -427,23 +414,11 @@ trait AssertLegacyTrait {
    *
    * @deprecated Scheduled for removal in Drupal 9.0.0.
    *   Use $this->assertSession()->fieldNotExists() or
+   *   $this->assertSession()->buttonNotExists() or
    *   $this->assertSession()->fieldValueNotEquals() instead.
    */
   protected function assertNoFieldById($id, $value = '') {
-    $xpath = $this->assertSession()->buildXPathQuery('//textarea[@id=:value]|//input[@id=:value]|//select[@id=:value]', [':value' => $id]);
+    $this->assertNoFieldByXPath($this->constructFieldXpath('id', $id), $value);
-    $field = $this->getSession()->getPage()->find('xpath', $xpath);
-
-    // Return early if the field could not be found as expected.
-    if ($field === NULL) {
-      return;
-    }
-
-    if (!isset($value)) {
-      throw new ExpectationException(sprintf('Id "%s" appears on this page, but it should not.', $id), $this->getSession()->getDriver());
-    }
-    elseif ($value === $field->getValue()) {
-      throw new ExpectationException(sprintf('Failed asserting that %s is not equal to %s', $field->getValue(), $value), $this->getSession()->getDriver());
-    }
   }
 
   /**
@@ -585,25 +560,32 @@ trait AssertLegacyTrait {
    *   (optional) A message to display with the assertion. Do not translate
    *   messages with t().
    *
+   * @throws \Behat\Mink\Exception\ExpectationException
+   *
    * @deprecated Scheduled for removal in Drupal 9.0.0.
    *   Use $this->xpath() instead and assert that the result is empty.
    */
   protected function assertNoFieldByXPath($xpath, $value = NULL, $message = '') {
     $fields = $this->xpath($xpath);
 
-    // If value specified then check array for match.
+    if (!empty($fields)) {
-    $found = TRUE;
+      if (isset($value)) {
-    if (isset($value)) {
+        $found = FALSE;
-      $found = FALSE;
+        try {
-      if ($fields) {
+          $this->assertFieldsByValue($fields, $value);
-        foreach ($fields as $field) {
+          $found = TRUE;
-          if ($field->getAttribute('value') == $value) {
+        }
-            $found = TRUE;
+        catch (\Exception $e) {
-          }
+        }
+
+        if ($found) {
+          throw new ExpectationException(sprintf('The field resulting from %s was found with the provided value %s.', $xpath, $value), $this->getSession()->getDriver());
         }
       }
+      else {
+        throw new ExpectationException(sprintf('The field resulting from %s was found.', $xpath), $this->getSession()->getDriver());
+      }
     }
-    return $this->assertFalse($fields && $found, $message);
   }
 
   /**
@@ -629,7 +611,15 @@ trait AssertLegacyTrait {
       $found = FALSE;
       if ($fields) {
         foreach ($fields as $field) {
-          if ($field->getAttribute('value') == $value) {
+          if ($field->getAttribute('type') == 'checkbox') {
+            if (is_bool($value)) {
+              $found = $field->isChecked() == $value;
+            }
+            else {
+              $found = TRUE;
+            }
+          }
+          elseif ($field->getAttribute('value') == $value) {
             // Input element with correct value.
             $found = TRUE;
           }
@@ -637,7 +627,7 @@ trait AssertLegacyTrait {
             // Select element with an option.
             $found = TRUE;
           }
-          elseif ($field->getText() == $value) {
+          elseif ($field->getTagName() !== 'input' && $field->getText() == $value) {
             // Text area with correct text.
             $found = TRUE;
           }
@@ -788,6 +778,25 @@ trait AssertLegacyTrait {
     return $this->assertSession()->buildXPathQuery($xpath, $args);
   }
 
+  /**
+   * Helper: Constructs an XPath for the given set of attributes and value.
+   *
+   * @param string $attribute
+   *   Field attributes.
+   * @param string $value
+   *   Value of field.
+   *
+   * @return string
+   *   XPath for specified values.
+   *
+   * @deprecated Scheduled for removal in Drupal 9.0.0.
+   *   Use $this->getSession()->getPage()->findField() instead.
+   */
+  protected function constructFieldXpath($attribute, $value) {
+    $xpath = '//textarea[@' . $attribute . '=:value]|//input[@' . $attribute . '=:value]|//select[@' . $attribute . '=:value]';
+    return $this->buildXPathQuery($xpath, [':value' => $value]);
+  }
+
   /**
    * Gets the current raw content.
    *

core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php

@@ -223,7 +223,7 @@ class BrowserTestBaseTest extends BrowserTestBase {
       $this->assertNoFieldByXPath("//input[@id = 'edit-name']");
       $this->fail('The "edit-name" field was not found.');
     }
-    catch (\PHPUnit_Framework_ExpectationFailedException $e) {
+    catch (ExpectationException $e) {
       $this->pass('assertNoFieldByXPath correctly failed. The "edit-name" field was found.');
     }
 
@@ -239,7 +239,7 @@ class BrowserTestBaseTest extends BrowserTestBase {
   /**
    * Tests legacy field asserts using textfields.
    */
-  public function testLegacyFieldAssertsWithTextfields() {
+  public function testLegacyFieldAssertsForTextfields() {
     $this->drupalGet('test-field-xpath');
 
     // *** 1. assertNoField().
@@ -272,7 +272,7 @@ class BrowserTestBaseTest extends BrowserTestBase {
       $this->assertField('invalid_name_and_id');
       $this->fail('The "invalid_name_and_id" field was found.');
     }
-    catch (ExpectationException $e) {
+    catch (\PHPUnit_Framework_ExpectationFailedException $e) {
       $this->pass('assertField correctly failed. The "invalid_name_and_id" field was not found.');
     }
 
@@ -361,7 +361,7 @@ class BrowserTestBaseTest extends BrowserTestBase {
       $this->assertFieldByName('non-existing-name');
       $this->fail('The "non-existing-name" field was found.');
     }
-    catch (ExpectationException $e) {
+    catch (\PHPUnit_Framework_ExpectationFailedException $e) {
       $this->pass('The "non-existing-name" field was not found');
     }
 
@@ -370,15 +370,15 @@ class BrowserTestBaseTest extends BrowserTestBase {
       $this->assertFieldByName('name', 'not the value');
       $this->fail('The "name" field with incorrect value was found.');
     }
-    catch (ExpectationException $e) {
+    catch (\PHPUnit_Framework_ExpectationFailedException $e) {
       $this->pass('assertFieldByName correctly failed. The "name" field with incorrect value was not found.');
     }
   }
 
   /**
-   * Tests legacy field asserts on other types of field.
+   * Tests legacy field asserts for options field type.
    */
-  public function testLegacyFieldAssertsWithNonTextfields() {
+  public function testLegacyFieldAssertsForOptions() {
     $this->drupalGet('test-field-xpath');
 
     // Option field type.
@@ -425,8 +425,14 @@ class BrowserTestBaseTest extends BrowserTestBase {
     catch (\PHPUnit_Framework_ExpectationFailedException $e) {
       $this->pass($e->getMessage());
     }
+  }
+
+  /**
+   * Tests legacy field asserts for button field type.
+   */
+  public function testLegacyFieldAssertsForButton() {
+    $this->drupalGet('test-field-xpath');
 
-    // Button field type.
     $this->assertFieldById('edit-save', NULL);
     // Test that the assertion fails correctly if the field value is passed in
     // rather than the id.
@@ -434,7 +440,7 @@ class BrowserTestBaseTest extends BrowserTestBase {
       $this->assertFieldById('Save', NULL);
       $this->fail('The field with id of "Save" was found.');
     }
-    catch (ExpectationException $e) {
+    catch (\PHPUnit_Framework_ExpectationFailedException $e) {
       $this->pass($e->getMessage());
     }
 
@@ -449,7 +455,27 @@ class BrowserTestBaseTest extends BrowserTestBase {
       $this->pass($e->getMessage());
     }
 
-    // Checkbox field type.
+    // Test that multiple fields with the same name are validated correctly.
+    $this->assertFieldByName('duplicate_button', 'Duplicate button 1');
+    $this->assertFieldByName('duplicate_button', 'Duplicate button 2');
+    $this->assertNoFieldByName('duplicate_button', 'Rabbit');
+
+    try {
+      $this->assertNoFieldByName('duplicate_button', 'Duplicate button 2');
+      $this->fail('The "duplicate_button" field with the value Duplicate button 2 was not found.');
+    }
+    catch (ExpectationException $e) {
+      $this->pass('assertNoFieldByName correctly failed. The "duplicate_button" field with the value Duplicate button 2 was found.');
+    }
+  }
+
+  /**
+   * Tests legacy field asserts for checkbox field type.
+   */
+  public function testLegacyFieldAssertsForCheckbox() {
+    $this->drupalGet('test-field-xpath');
+
+    // Part 1 - Test by name.
     // Test that checkboxes are found/not found correctly by name, when using
     // TRUE or FALSE to match their 'checked' state.
     $this->assertFieldByName('checkbox_enabled', TRUE);
@@ -462,17 +488,13 @@ class BrowserTestBaseTest extends BrowserTestBase {
     $this->assertFieldByName('checkbox_enabled', NULL);
     $this->assertFieldByName('checkbox_disabled', NULL);
 
-    // Test that checkboxes are found/not found correctly by ID, when using
+    // Test that checkboxes are found by name when passing no second parameter.
-    // TRUE or FALSE to match their 'checked' state.
+    $this->assertFieldByName('checkbox_enabled');
-    $this->assertFieldById('edit-checkbox-enabled', TRUE);
+    $this->assertFieldByName('checkbox_disabled');
-    $this->assertFieldById('edit-checkbox-disabled', FALSE);
-    $this->assertNoFieldById('edit-checkbox-enabled', FALSE);
-    $this->assertNoFieldById('edit-checkbox-disabled', TRUE);
 
-    // Test that checkboxes are found by by ID, when using NULL to ignore the
+    // Test that we have legacy support.
-    // 'checked' state.
+    $this->assertFieldByName('checkbox_enabled', '1');
-    $this->assertFieldById('edit-checkbox-enabled', NULL);
+    $this->assertFieldByName('checkbox_disabled', '');
-    $this->assertFieldById('edit-checkbox-disabled', NULL);
 
     // Test that the assertion fails correctly when using NULL to ignore state.
     try {
@@ -483,6 +505,27 @@ class BrowserTestBaseTest extends BrowserTestBase {
       $this->pass('assertNoFieldByName failed correctly. The "checkbox_enabled" field was found using NULL value.');
     }
 
+    // Part 2 - Test by ID.
+    // Test that checkboxes are found/not found correctly by ID, when using
+    // TRUE or FALSE to match their 'checked' state.
+    $this->assertFieldById('edit-checkbox-enabled', TRUE);
+    $this->assertFieldById('edit-checkbox-disabled', FALSE);
+    $this->assertNoFieldById('edit-checkbox-enabled', FALSE);
+    $this->assertNoFieldById('edit-checkbox-disabled', TRUE);
+
+    // Test that checkboxes are found by ID, when using NULL to ignore the
+    // 'checked' state.
+    $this->assertFieldById('edit-checkbox-enabled', NULL);
+    $this->assertFieldById('edit-checkbox-disabled', NULL);
+
+    // Test that checkboxes are found by ID when passing no second parameter.
+    $this->assertFieldById('edit-checkbox-enabled');
+    $this->assertFieldById('edit-checkbox-disabled');
+
+    // Test that we have legacy support.
+    $this->assertFieldById('edit-checkbox-enabled', '1');
+    $this->assertFieldById('edit-checkbox-disabled', '');
+
     // Test that the assertion fails correctly when using NULL to ignore state.
     try {
       $this->assertNoFieldById('edit-checkbox-disabled', NULL);
@@ -492,7 +535,7 @@ class BrowserTestBaseTest extends BrowserTestBase {
       $this->pass('assertNoFieldById failed correctly. The "edit-checkbox-disabled" field was found by ID using NULL value.');
     }
 
-    // Test the specific 'checked' assertions.
+    // Part 3 - Test the specific 'checked' assertions.
     $this->assertFieldChecked('edit-checkbox-enabled');
     $this->assertNoFieldChecked('edit-checkbox-disabled');