Convert ParagraphsTestBase and the easy-to-convert tests extending it to BrowserTestBase

This conversion will mean changing it from a WebTestBase to a BrowserTestBase. That means all the classes that extend it change too. They will need to be done in the same patch.

This includes:

• ParagraphsAddModesTest
• ParagraphsConfigTest
• ParagraphsContactTest
• ParagraphsInlineEntityFormTest
• ParagraphsTypesTest
• ParagraphsWidgetButtonsTest

Ok maybe I can do a better job of explaining this...

Once the testbase gets converted to BrowserTestBase all the tests that are extending it will stop working and no longer pass. That's why such a patch can not be committed on its own.

Come to think of it, it may be possible if ParagraphsTestBase can be changed into ParagraphsTestTrait for this issue so the classes using it can extend WebTestBase directly until they get worked on in their own issues.

Here is a patch that will allow the tests using ParagraphsTestBase to be worked on separately. I tried to move the new ParagraphsTestTrait to tests/src/Functional but the autoloader only finds stuff descending from src. It could be moved after all of the classes using it are converted and moved. That would be the last step in converting ParagraphsTestBase to PHPUnit.

I see that moving to a trait works, but adds lots of duplication in module dependency declaration.

Is the previous test base also valuable for other tests that are currently not built on top of ParagraphsTestBase?
Otherwise i don't see why switching away from a base class is an improvement?

@miro_dietiker I am not sure how I can explain the reasoning behind the change any better than I already did. Is there anything particular in my posts that doesn't seem to make sense? I may be able to break it down more.

Not sure.

If we'd have dozens of tests, then maybe doing child issues might be better, but you already end up changing all subclasses now too, so what's the benefit? Core also doesn't do it per class, but is discussing about what the best way to group them is. And I think one of the suggestions is to do it per base class, when there is one.

I also think we already have an issue for this, where someone started to convert to javascript test base. We discused there that it's not needed for all classes but actually, I can see drupalPostAjaxForm() calls in 9 of the 11 test web tests. Either we find a way to not use ajax for them or we have to make them JS tests.

Given that, not exactly sure. Open to make it a trait, especially since we seem to need at least two different base classes. But maybe we could keep the base classses, exactly to avoid changing the subclasses and duplicate the modules everywhere? We could do that + convert everything we can to BTB and then do the remaining ones to javascript tests.

So the best case scenario would be if all the tests using this baseclass could be converted to BrowserTestBase and the ajax removed from them and it was all done at once as per #2. It may be possible, the interface is all supposed to work without javascript in theory.

This passes for me and includes all the tests listed in #2. Setting to postponed because it depends on the patches in the core issues I'm relating.

Moving this to "Needs Review". The second of the two issues was just committed to 8.6.x and 8.7.x (the first being fixed in 8.3.x).

Continuing the work from #2752659: [META] Convert SimpleTest Tests to PHPUnit here.

This removes the call to assertNoErrorsLogged() which has no replacement, and reverts ParagraphsExperimentalTranslationTest back to simpletest for now.

+++ b/tests/src/Functional/Experimental/ParagraphsExperimentalBehaviorsTest.php
@@ -328,7 +328,6 @@
     $this->drupalPostForm(NULL, $edit, t('Save'));
-    $this->assertNoErrorsLogged();
     $this->clickLink('Edit');
     $this->assertFieldByName('field_paragraphs[0][behavior_plugins][test_text_color][text_color]', 'red');

Hm, is that really not required anymore? I'm wondering why we added those calls. phpunit fails if it detects an error I think, but so did simpletest.

We have 3 of these calls in total, this one was added by #2851402: Error on subform state when removing/adding container . I tried to undo the fix to see if it still fails, but it the code has changed and I didn't manage to produce an error. So, fine with removing this.

We already have some actual JS tests we can always add more, but I'm happy if as much as possible don't need to be. Much better for test performance as long as we can't run then in parallel.

FieldUiTestTrait is an 8.7 deprecation, so I plan to commit this after the next release (should happen this week?) and update our dependency definition.

Since we already touch some of the existing browser tests to update the use statements, wondering if we should go one step further and also use the new base class there now, should save us a few lines duplicating the traits in all of them?

Much better for test performance as long as we can't run then in parallel.

Hmm? Javascript tests run with concurrency 15 (PhantomJs couldn't, Chrome/Webdriver can)

This changes the existing browsertestbase test to use the new Base classes. Also moved them to the classic/experimental dirs where possible. There are 3 tests that already exist inside the new dirs (name wise), so left them for now, they probably need to be merged or something, but that might make this harder to review, so left them for now.

Didn't run the tests locally, so this might fail spectacularly :)

This should clear up the fails

....

Thanks, committed. Also updated the core dependency to 8.7.