Convert WebTestBase to BrowserTestBase

Attached patch converts ImageWidgetCropTest from Simpletest to a FunctionalJavascript test. Since some assertions test JavaScript interactions, it is not possible to use BrowserTestBase directly.

This looks very good to me. There are some relatively minor points I would suggest changing before commit, but none of these are essential.

1. +++ b/tests/src/FunctionalJavascript/ImageWidgetCropTest.php
   @@ -60,9 +65,10 @@ class ImageWidgetCropTest extends WebTestBase {
   +    $field = $this->getSession()->getPage()->findField('files[field_image_crop_test_0]');
   +    $field->attachFile($this->container->get('file_system')->realpath('public://image-test.jpg'));
   

   Nit: There's no need for $field to be its own variable. I would suggest, for extra strength, that we use $this->getSession()->getPage()->attachFileToField() instead.

2. +++ b/tests/src/FunctionalJavascript/ImageWidgetCropTest.php
   @@ -94,21 +99,22 @@ class ImageWidgetCropTest extends WebTestBase {
   +    $assert_session->elementNotExists('css', '#edit-field-image-crop-test-wrapper summary');
   

   This assertion is not a direct replacement for the assertion it its replacing. Why not just use $assert_session->responseNotContains()?

3. +++ b/tests/src/FunctionalJavascript/ImageWidgetCropTest.php
   @@ -94,21 +99,22 @@ class ImageWidgetCropTest extends WebTestBase {
   +    $field = $this->getSession()->getPage()->findField('files[field_image_crop_test_0]');
   +    $field->attachFile($this->container->get('file_system')->realpath('public://image-test.jpg'));
   

   Nit: There is no need for $field to be its own variable. We should probably use $this->getSession()->getPage()->attachFileToField() here instead, to prevent potentially calling methods on NULL.

4. +++ b/tests/src/FunctionalJavascript/ImageWidgetCropTest.php
   @@ -94,21 +99,22 @@ class ImageWidgetCropTest extends WebTestBase {
   +    $assert_session->elementExists('css', '#edit-field-image-crop-test-wrapper summary');
   

   Again, maybe $assert_session->responseContains($raw) would be more in keeping with how the test was previously?

5. +++ b/tests/src/FunctionalJavascript/ImageWidgetCropTest.php
   @@ -94,21 +99,22 @@ class ImageWidgetCropTest extends WebTestBase {
   +    $remove_button = $this->xpath('//input[@type="submit" and @value="' . t('Remove') . '"]');
   +    $this->assertTrue(!empty($remove_button));
   

   This seems a little odd; why not just $assert_session->buttonExists('Remove')?

6. +++ b/tests/src/FunctionalJavascript/ImageWidgetCropTest.php
   @@ -125,22 +131,23 @@ class ImageWidgetCropTest extends WebTestBase {
   +    $button = $this->xpath('//input[@type="submit" and @value="' . t('Remove') . '"]');
   +    $button[0]->click();
   

   To prevent calling click() on NULL, and for clarity, I would suggest using $this->getSession()->getPage()->pressButton('Remove') here.

7. +++ b/tests/src/FunctionalJavascript/ImageWidgetCropTest.php
   @@ -125,22 +131,23 @@ class ImageWidgetCropTest extends WebTestBase {
   +    $field = $this->getSession()->getPage()->findField('files[field_image_crop_test_0]');
   +    $field->attachFile($this->container->get('file_system')->realpath('public://image-test.jpg'));
   

   As before, I see no benefit to using this pattern instead of $this->getSession()->getPage()->attachFileToField().

this should address the feedback in #4.

trying that again

After doing a bit of debugging with @katherined, I realized that my request in #4.2 was mistaken. My apologies.

Using a CSS selector is smarter than looking for raw response text, because the AJAX system will randomize the ID attribute. This would not have happened in a SimpleTest test, since that used a fake AJAX simulation, rather than actually doing a real AJAX request in a real browser. That explains why the test might have passed in SimpleTest, but not PHPUnit.

So, we can either go back to what we had in #3 for that assertion, or switch to a CSS selector like summary:contains(Crop image). Both work for me. Sorry for the disruption!

third time's the charm?

Looks green to me :)

Adjusting credit for commit.

Re-ran tests to confirm everything still looks good, then committed and pushed to 8.x-2.x. Thanks!