Convert AJAX part of \Drupal\field\Tests\EntityReference\EntityReferenceAdminTest to JavascriptTestBase

Here we go.

Split out the part that does actual ajax stuff. The smaller tests I just used drupalPostForm since those steps get Javascript coverage in the other test, no need to duplicate that. Same goes for the part that makes a view etc, just kept that simple.

A couple of unavoidable assertWaitOnAjaxRequest in there for changes that don't change anything in the DOM, so there is nothing else to wait for.

The patch looks great here are some suggestions:

1. +++ b/core/modules/field/tests/src/FunctionalJavascript/EntityReference/EntityReferenceAdminTest.php
   @@ -0,0 +1,321 @@
   +    $this->getSession()
   +      ->getPage()
   +      ->findField('new_storage_type')
   +      ->setValue('entity_reference');
   +    $this->assertSession()->waitForField('label');
   +    $this->getSession()->getPage()->findField('label')->setValue('Test');
   +    $this->assertSession()
   +      ->waitForElement('xpath', '//*[@id="edit-label-machine-name-suffix"]/span[2]/span[contains(text(), "field_test")]');
   

   I think we can simplfiy this.
   This is what DER is doing, HT:@dpi

       // Add a new dynamic entity reference field.
       $this->drupalGet('entity_test/structure/entity_test/fields/add-field');
       $select = $assert_session->selectExists('new_storage_type');
       $select->selectOption('dynamic_entity_reference');
       $label = $assert_session->fieldExists('label');
       $label->setValue('Foobar');
       // Wait for the machine name.
       $assert_session->waitForElementVisible('css', '[name="label"] + * .machine-name-value');

2. +++ b/core/modules/field/tests/src/FunctionalJavascript/EntityReference/EntityReferenceAdminTest.php
   @@ -0,0 +1,321 @@
   +    $this->getSession()
   +      ->getPage()
   ...
   +    $this->getSession()
   +      ->getPage()
   ...
   +    $this->getSession()
   +      ->getPage()
   

   Let's create $page.

3. +++ b/core/modules/field/tests/src/FunctionalJavascript/EntityReference/EntityReferenceAdminTest.php
   @@ -0,0 +1,321 @@
   +    $this->assertSession()->pageTextContains('Saved Test configuration.');
   

   Let's create $assert_session instead of $this->assertSession() everywhere.

@jibran thanks for the review!

#8.1 I cleaned it up a bit, but I kinda like the wait using the xpath containing the text() because the rest of the test depends on the machine name being that exact string. Otherwise we could have a situation where the machine name is set, but not yet set to that string (if the input has a slight delay midway through for example, which prematurely triggers setting the machine name), which would lead to a random fail. Fairly hypothetical to be sure, but why risk it.
#8.2 fixed
#8.3 fixed

Cleaned up some more. Also went to work on assertFieldSelectOptions which was going into a loop for some reason in the JS test, something not right in the optgroup handling. So rewrote that a bit and uncommented the assert that was blocking me before.

RE: #9.1 Personally, I don't like using xpath because we have cssSelect now, but I do understand your point :). The core is setting an example here so I disagree with your approach because it is overly complicated but I don't want to hold the patch for this anymore so let's committer decide what is a better DX.

$assert_session->waitForElement('xpath', '//*[@id="edit-label-machine-name-suffix"]/span[2]/span[contains(text(), "field_test")]');
OR
$assert_session->waitForElementVisible('css', '[name="label"] + * .machine-name-value');

Edit: FWIW, waitForElementVisible returns the result element and we can assert its value as well.

This adds some new PHPCS fails

Checking changed files...

FILE: ...ests/src/Functional/EntityReference/EntityReferenceAdminTest.php
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------
 380 | ERROR | [ ] If there is no return value for a function, there
     |       |     must not be a @return tag.
 388 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Time: 111ms; Memory: 8Mb


FILE: ...unctionalJavascript/EntityReference/EntityReferenceAdminTest.php
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------
 220 | ERROR | [ ] If there is no return value for a function, there
     |       |     must not be a @return tag.
 232 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Feedback addressed.

Back to RTBC. @larowlan any opinion on #10?

Committed and pushed 825a340641 to 8.7.x and edb0e47428 to 8.6.x. Thanks!

Backported to 8.6.x to keep the tests aligned.