Test Drupal.behaviors.copyFieldValue javascript

I tried writing Javascript test case for Drupal.behaviours.copyFieldValue Here is in progress code in gist https://gist.github.com/msankhala/e6bed178e64c4369b755eb4ef7a76088

I took core/tests/Drupal/FunctionalJavascriptTests/Core/Installer/Form/SelectProfileFormTest.php test case as starting point for this. I simply copied core/tests/Drupal/FunctionalJavascriptTests/Core/Installer/Form/SelectProfileFormTest.php
to core/tests/Drupal/FunctionalJavascriptTests/Core/Installer/Form/ConfigureSiteFormTest.php and created new method called testSiteEmailCopiedToAccountEmail() and started writing test scenario in this method.

I am not sure how to wait till Drupal installation is complete and redirected to Configure Site screen, where actually Drupal.behaviours.copyFieldValue is in action. Can someone guide me in this?

Or this should go into System module. Should i create new module inside core/modules/system/tests/modules which will provide form with two email fields and load this Javascript behaviour on that page and test it there? Because Drupal.behaviours.copyFieldValue is part of system module.

@alexpott Thanks for your guide. I'll move this test to system module.

Here is my first Drupal Functional Javascript test. I created a controller which simply return \Drupal\Core\Installer\Form\SiteConfigureFormand then tested Drupal.behaviors.copyFieldValuefunctionality on this form.

Thanks @alexpott for feedback. Here is new tests only patch. Now this does not uses \Drupal\Core\Installer\Form\SiteConfigureFormform anymore.

Hi @alexpott yes this test can completely be decoupled from site configure form. Here is new patch as per your suggestion.

@alexpott Isn't it advised to avoid waiting for a specific number of seconds?

I tried with waitFor as you suggested. This caused two things.

1. Number of assertions increased from 4 to 103.
2. If all the assertions run successfully then there is no issue but if i explicitly fail the second assertion by changing assertNotEquals to assertEquals then it breaks the test.

    // Test Drupal.behaviors.copyFieldValue. Check if source field is filled,
    // target field is also filled with same value.
    $page->fillField('edit-source-field', 'test@example.com');
    // After filling the field the JS will run async so add wait of 10s.
    $page->waitFor(10, function () use ($source_field, $target_field) {
      $this->assertEquals($source_field->getValue(), $target_field->getValue());
    });

    // Now target field is already filled. Now changing source field should not
    // change target field.
    $page->fillField('edit-source-field', '');
    // Adding explicit wait of 1s to allow js to update field.
    $page->waitFor(1, function () use ($source_field, $target_field) {
      $this->assertNotEquals($source_field->getValue(), $target_field->getValue());
    });
  }

This returns:

Testing Drupal\Tests\system\FunctionalJavascript\CopyFieldValueTest


Time: 19.51 seconds, Memory: 6.00MB

OK (1 test, 103 assertions)

Process finished with exit code 0

As you can see it is making 103 assertions.

In second assertion if i change assertNotEquals to assertEquals like:

    // Test Drupal.behaviors.copyFieldValue. Check if source field is filled,
    // target field is also filled with same value.
    $page->fillField('edit-source-field', 'test@example.com');
    // After filling the field the JS will run async so add wait of 10s.
    $page->waitFor(10, function () use ($source_field, $target_field) {
      $this->assertEquals($source_field->getValue(), $target_field->getValue());
    });

    // Now target field is already filled. Now changing source field should not
    // change target field.
    $page->fillField('edit-source-field', '');
    // Adding explicit wait of 1s to allow js to update field.
    $page->waitFor(1, function () use ($source_field, $target_field) {
      $this->assertEquals($source_field->getValue(), $target_field->getValue());
    });

This returns:

Testing Drupal\Tests\system\FunctionalJavascript\CopyFieldValueTest

PHP Fatal error:  Uncaught Exception: Serialization of 'Closure' is not allowed in -:68
Stack trace:
#0 -(68): serialize(Array)
#1 -(100): __phpunit_run_isolated_test()
#2 {main}
  thrown in - on line 68


Time: 18.37 seconds, Memory: 6.00MB

Thanks @alexpott for detailed feedback. I missed the whole point of waitFor and added assertion inside the callback. Here is updated patch. Please review.

nits: #1 removed extra lines
#2 docs added
#3 removed
#4 comment removed
#5 ...
#6 and #7 fields modified

+++ b/core/modules/system/tests/src/FunctionalJavascript/CopyFieldValueTest.php
@@ -0,0 +1,52 @@
+//      $random_string = '';

Remove comment.

1. Removed comment.
2. Removed $random = new Random(); object.
3. created $source_field_selector variable.

Can anyone review this?

Patch for 8.7.x. Even though nothing changed between 8.6.x to 8.7.x.

Here is the updated patch which fixes the failing test.

Re-rolled patch against 10.1.x. Please review

Fixing CCF in #39

Thanks NivethaSubramaniyan unfortunately you're missing many files from the previous patch.

1. +++ b/core/modules/system/tests/src/FunctionalJavascript/CopyFieldValueTest.php
   @@ -0,0 +1,55 @@
   +    // After filling the field the JS will run async so wait up to 10s.
   +    $this->assertTrue($page->waitFor(10, function () use ($target_field, $random_string) {
   +      return $target_field->getValue() == $random_string;
   +    }));
   

   Going with $this->assertTrue($target_field->getValue() == $ random_string); works.

   it needs to wait for the blur event on the source field, by the time $target_field->focus() is done, the blur event has already been executed (because of the order in which those event are spec'ed to occur).

2. +++ b/core/modules/system/tests/src/FunctionalJavascript/CopyFieldValueTest.php
   @@ -0,0 +1,55 @@
   +    // After filling the field the JS will run async so add wait of 1s. Just to
   +    // ensure the JS doesn't unexpected fill the field.
   +    $this->assertFalse($page->waitFor(1, function () use ($target_field) {
   +      // This should never be TRUE.
   +      return $target_field->getValue() == '';
   

   Here we should probably check that the target field has the same value as before : $random_string that would allow us to remove the waitFor as well.

Addressed points in #43

looks good to me

So should it not wait?

reviewed the patch on comment #48, patch applied successfully on D10 and there is no functionality break in the website, can we merge now

+++ b/core/modules/system/tests/src/FunctionalJavascript/CopyFieldValueTest.php
@@ -0,0 +1,54 @@
+    $assert_session->assertWaitOnAjaxRequest();
...
+    $assert_session->assertWaitOnAjaxRequest();

There's no AJAX going on here so waiting for AJAX is pointless.

Updated patch #48 by addressing #50, please review it.

Thanks!

Even though I worked on the ticket, this was previously marked RTBC and the update was removing 2 lines so think I can still move it back to RTBC.

Random failure

1. +++ b/core/modules/system/tests/modules/system_test/src/Form/CopyFieldValueTestForm.php
   @@ -0,0 +1,53 @@
   +    // We are only testing the javascript part of form. We are not submitting
   
   +++ b/core/modules/system/tests/src/FunctionalJavascript/CopyFieldValueTest.php
   @@ -0,0 +1,52 @@
   + * Tests copy field value javascript functionality works fine.
   ...
   +   * Tests copy field value javascript functionality.
   

   Nit: javascript should be replaced with JavaScript.

2. +++ b/core/modules/system/tests/modules/system_test/system_test.routing.yml
   --- /dev/null
   +++ b/core/modules/system/tests/src/FunctionalJavascript/CopyFieldValueTest.php
   
   +++ b/core/modules/system/tests/src/FunctionalJavascript/CopyFieldValueTest.php
   @@ -0,0 +1,52 @@
   +    $assert_session = $this->assertSession();
   

   Unused variable.

3. +++ b/core/modules/system/tests/src/FunctionalJavascript/CopyFieldValueTest.php
   @@ -0,0 +1,52 @@
   +    // Now focus target field.
   ...
   +    // Now focus target field.
   

   This comment is redundant because that's exactly how the following line would read..

4. +++ b/core/modules/system/tests/src/FunctionalJavascript/CopyFieldValueTest.php
   @@ -0,0 +1,52 @@
   +    $this->assertTrue($target_field->getValue() == $random_string);
   ...
   +    $this->assertTrue($target_field->getValue() == $random_string);
   

   You can use $this->assertEquals() here 😇

5. +++ b/core/modules/system/tests/src/FunctionalJavascript/CopyFieldValueTest.php
   @@ -0,0 +1,52 @@
   +    // After filling the field the JS will run async. Just to ensure the JS
   

   I'm not sure I understand this comment. Isn't the expectation that nothing happens to the target at this point? Why is this comment explaining that the JavaScript will run async?

Uploading updated #51 patch with suggested 1,2 and 3 changes in comment #55.

#56 doesn't fix issues 1,4,5 mentioned in #55, re-uploading patch to fix all the mentioned issues, please review.

#57 appears to address the points from #55

Minor adjustments to code comments.

Random failure.

Committed f88d58b and pushed to 10.1.x. Thanks!