Convert AJAX part of \Drupal\field\Tests\FormTest::testFieldFormJSAddMore to JavascriptTestBase

I recreated the behavior as far as I understand the current test. As I see it the test only tests ajax behavior and no php behavior, therefore the test is fully converted to a JavascriptTestBase.

I tried to assume nothing and check everything one step at a time.

I'd love some feedback on it!

1 thing I'm not sure about:
- The original test used entity_get_form_display which is deprecated. Couldn't figure out the correct way yet, should it be converted as well?

I will review this face-to-face with @michielnugter.

I updated the patch after lendude's feedback for some code cleanup, thanks!

interdiff is from before the filename change.

Nice clean up, thanks for that!

+++ b/core/modules/field/src/Tests/FormTest.php
@@ -389,62 +389,6 @@ function testFieldFormMultivalueWithRequiredRadio() {
-    $this->assertNoField("{$field_name}[" . ($delta + 1) . '][value]', 'No extraneous widget is displayed');

This is the only assertion I'm missing in the new test.

Thanks for the review and spotting that, I added it to the new test.

This looks good now, and covers everything in the original test.

Unrelated fail, see #2828045: Tests failing with unrelated „Translation: The French translation is not available”, waiting for that to go away before RTBCing again

Back to RTBC

Back to RTBC after a random fail.

JavascriptTestBase has at least 3 random test fails right now, so we need to stop converting tests to JavascriptTestBase until #2830485: \Drupal\Tests\outside_in\FunctionalJavascript\OutsideInBlockFormTest fails randomly is resolved. Sorry!

Moving back as the mentioned issue is not critical and not causing random failures anymore.

@klausi, @michielnugter - what's the current state of random fails in javascript test base tests. The resolution to #2830485: \Drupal\Tests\outside_in\FunctionalJavascript\OutsideInBlockFormTest fails randomly was to just skip the test for now - so #17 still seems relevant.

@alexpott
As I understood it, the critical part was removed by disabling the test and as it's not a critical issue anymore work on other JavascriptTestBase tests could continue.

There is another critical issue for JTB though: #2831603

So postponed untill both are fixed or both are at least not critical anymore?

Small update to conform to the new API where possible.

Also, the mentioned random fails are fixed, this is good to go again?

Needed a reroll (so no interdiff), addressed #25, did array() => [] conversion

Thanks for the reroll, did spot a small error in the test though.

Did a self review:

+++ b/core/modules/field/tests/src/FunctionalJavascript/FormJSAddMoreTest.php
@@ -0,0 +1,122 @@
+
...
+    $assert_session->assertWaitOnAjaxRequest();

This can be changed to assertSession()->waitForField instead.

Thanks for those and they're fixed!

The more I work on the JavascriptTestBase tests the more I dislike the assertWaitOnAjaxRequest method. It promotes incorrect testing and is one of the major reasons for random fails. I think we should always check for what we expect in the DOM. This works around all the problems we've had so far (debounce, ajax handling by drupal, browser based events, etc). I've been thinking about this a lot and will update the documentation soon if I have a good way of putting my thoughts on paper.

Fail was a random fail btw which has an RTBC issue: #2870146: Even more random fails in \Drupal\Tests\outside_in\FunctionalJavascript\OutsideInBlockFormTest.

With a little (lot ;)) more experience now as opposed to then I do think that it's solvable with xpath :) Back to needs work!

I've added a bit xpath stuff based on #30, is it good now?

Thanks @ApacheEx

Still loving how this tests turned out. Feedback has been addressed, random fails in JavascriptTestBase are under control, back to RTBC after 11 months :)

I read over the old method and the new test side by side. In addition to converting it to a JS test, we're also getting rid of some totally unnecessary random weighting in favor of testing it directly with specific fixtures, and replacing a deprecated function call. That all seems reasonable since we have to rewrite the main portion of the test anyway.

Committed and pushed to 8.5.x. I also backported it to 8.4.x since this doesn't affect public test API to keep the branches in sync for bugfixes. Thanks!