Test Drupal.behaviors.copyFieldValue javascript

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.

The last submitted patch, 51: 2944089-51.patch, failed testing. View results →

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.

The last submitted patch, 59: 2944089-59.patch, failed testing. View results →

Random failure.

Committed f88d58b and pushed to 10.1.x. Thanks!

Automatically closed - issue fixed for 2 weeks with no activity.