Convert web tests to browser tests for filter module

• Moved all 4 files.
• Corrected namespace.
• Updated references.

Sorry @nlisgo, I only just noticed you had assigned this to yourself after doing the work.

About changes in core/modules/filter/tests/src/Functional/FilterAdminTest.php

assertFieldByName() now deprecated and was changed, so third paramater was removed, also test assert was changed because we need to check that checkboxes, which was checked on add filter form, is checked on filter edit form.

Also $this->xpath() now return NodeElement[] array, instead of \SimpleXMLElement[]|bool, so all function call also changed.

Hi @zviryatko, thank you for your contribution!

First tip: Try to use the following git configuration:

[diff]
renames = copies
This detects renames, see https://www.drupal.org/documentation/git/configure for more details. This will make the patch size significantly smaller.

Sorry, forgot about this ((

@zviryatko Thanks for the patch! Great to see this moving forward!

Some review comments:

1. +++ b/core/modules/filter/tests/src/Functional/FilterAdminTest.php
   @@ -142,7 +142,7 @@ public function testFormatAdmin() {
   +    $this->assertTrue(!empty($edit_link), format_string('Link href %href found.',
   
   @@ -297,8 +297,8 @@ public function testFilterAdmin() {
   +    $this->assertTrue(!empty($view_link), 'The message area contains a link to a node');
   
   +++ b/core/modules/filter/tests/src/Functional/FilterFormTest.php
   @@ -199,14 +196,13 @@ protected function assertNoSelect($id) {
   +    $this->assertTrue(!empty($select), SafeMarkup::format('Field @id exists.', [
   
   @@ -253,14 +247,13 @@ protected function assertRequiredSelectAndOptions($id, array $options) {
   +    $this->assertTrue(!empty($select), SafeMarkup::format('Required field @id exists.', [
   
   @@ -276,8 +269,7 @@ protected function assertEnabledTextarea($id) {
   +    $this->assertTrue(!empty($textarea), SafeMarkup::format('Enabled field @id exists.', [
   
   @@ -295,17 +287,17 @@ protected function assertDisabledTextarea($id) {
   +    $this->assertTrue(!empty($textarea), SafeMarkup::format('Disabled field @id exists.', [
   

   You can use assertNotEmpty here.

2. +++ b/core/modules/filter/tests/src/Functional/FilterFormTest.php
   @@ -173,13 +173,10 @@ protected function doFilterFormTestAsNonAdmin() {
   +    $this->assertTrue(empty($select), SafeMarkup::format('Field @id does not exist.', [
   
   @@ -253,14 +247,13 @@ protected function assertRequiredSelectAndOptions($id, array $options) {
   +    $this->assertTrue(!empty($select), SafeMarkup::format('Required field @id exists.', [
   

   You can use assertEmpty() here.

3. +++ b/core/modules/filter/tests/src/Functional/FilterFormatAccessTest.php
   @@ -151,7 +151,7 @@ public function testFormatPermissions() {
   +      $options[(string) $element->getValue()] = $element;
   

   (string) cast is not required anymore

4. +++ b/core/modules/filter/tests/src/Functional/FilterHtmlImageSecureTest.php
   @@ -141,14 +143,14 @@ public function testImageSource() {
   +          $this->assertEqual((string) $element->getAttribute('src'), $red_x_image);
   +          $this->assertEqual((string) $element->getAttribute('alt'), $alt_text);
   +          $this->assertEqual((string) $element->getAttribute('title'), $title_text);
   +          $this->assertEqual((string) $element->getAttribute('height'), '16');
   +          $this->assertEqual((string) $element->getAttribute('width'), '16');
   ...
   +          $this->assertEqual((string) $element->getAttribute('src'), $converted);
   

   (string) cast is not required anymore

1. +++ b/core/modules/filter/tests/src/Functional/FilterAdminTest.php
   @@ -142,7 +142,7 @@ public function testFormatAdmin() {
   -    $this->assertTrue($edit_link, format_string('Link href %href found.',
   +    $this->assertNotEmpty($edit_link, format_string('Link href %href found.',
   

   I really like these changes.

2. +++ b/core/modules/filter/tests/src/Functional/FilterFormTest.php
   @@ -199,14 +196,13 @@ protected function assertNoSelect($id) {
   -    $select = reset($select);
   -    $passed = $this->assertTrue($select instanceof \SimpleXMLElement, SafeMarkup::format('Field @id exists.', [
   +    $this->assertNotEmpty($select, SafeMarkup::format('Field @id exists.', [
   

   +1 also for this change. IMHO we don't loose any level of test coverage here.

Committed and pushed 26ad8f8 to 8.4.x and 589eede to 8.3.x. Thanks!

Committed to 8.3.x since this is tests only.

This clashed with #2868019: AssertLegacyTrait field assertions not compatible with Simpletest assertions and resulted in HEAD fails.

Sorry about that @alexpott, didn't catch that one at review.

Updated the patch to conform to the new AssertLegacyTrait, this should pass.

Let's get it in

Committed and pushed facaf22 to 8.4.x and d002aa9 to 8.3.x. Thanks!

I'm really sorry - additional problems with #2868019: AssertLegacyTrait field assertions not compatible with Simpletest assertions came to light so we had to revert it - which means we have to revert this too.

Re-uploading #12 as that worked before and will commit if it is green.

Discussed with @alexpott on IRC. We'll have to fix the checkbox assertion that currently is failing to a correct test using the non-deprecated methods of Mink.

$this->assertFieldByName('roles[' . RoleInterface::AUTHENTICATED_ID . ']', RoleInterface::AUTHENTICATED_ID);

This assertion is incorrect because it checks if the value is equal to a role ID. Reading the test what was attempted to assert was if the checkbox was checked or not, this assertion does not test this at all. At most it tests if the checkbox is rendered properly with a value matching the name.

Without using the deprecated methods you could assert the state of a checkbox like this:

$this->assertSession()->checkboxChecked($id);

or

$field = $this->getSession()->getPage()->findField('name');
$this->assertTrue($field->isChecked();

As we have checkbox names available. So we could use $this->assertSession()->checkboxChecked

//Naveen

@naveenvalecha thanks for the patch!

Looks good, setting to RTBC.

Maybe related: #2363815: FilterHtmlImageSecureTest fails on MAMP, not on testbot.

back to RTBC per #31

Committed/pushed to 8.5.x and cherry-picked to 8.4.x. Thanks!