Problem/Motivation

drupalPostAjaxForm() is simulating the behaviour of ajax.js, so using it, doesn't really provide fundamental guarantees.
#2809161: Convert Javascript/AJAX testing to use JavascriptTestBase suggests to convert them to JavascriptTestBase

The rest of the test has to be converted to a BrowserTestBase test.

Proposed resolution

  1. Figure out which part of the test is testing PHP code and which part ajax behaviour
  2. Extract the ajax behaviour into a test that extends JavascriptTestBase
  3. Move the rest of the test to a BrowserTestBase test

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#15 2809513-15.patch11.24 KBLendude
#15 interdiff-2809513-14-15.txt5.2 KBLendude
#14 2809513-convert-responsiveimagefielduitest-14.patch11.58 KBbrentg
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner created an issue. See original summary.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

michielnugter’s picture

michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commented
Title: Convert AJAX part of \Drupal\responsive_image\Tests\ResponsiveImageFieldUiTest to JavascriptTestBase » Convert AJAX part of \Drupal\responsive_image\Tests\ResponsiveImageFieldUiTest to JavascriptTestBase and the rest to BrowserTestBase
Component: phpunit » responsive_image.module
Issue summary: View changes

Updated the title and IS to reflect the increased scope for the issue, the BrowserTestBase conversion is now part of this issue.

michielnugter’s picture

michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commented
Issue tags: +phpunit initiative

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

ApacheEx’s picture

ApacheEx CreditAttribution: ApacheEx commented
Issue tags: +LutskGSW18

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

ApacheEx’s picture

ApacheEx CreditAttribution: ApacheEx as a volunteer and at Drupal Ukraine Community commented
Assigned: Unassigned » ApacheEx
Status: Active » Needs work

I will try to prepare patch today on Drupal Global CodeSprint Jan 2018

ApacheEx’s picture

ApacheEx CreditAttribution: ApacheEx as a volunteer and at Drupal Ukraine Community commented
Assigned: ApacheEx » Unassigned
Status: Needs work » Active

seems form has hidden fields (e.g refresh_rows), that's why we need to wait when #2917885: Add drupalPostFormWithInvalidOptions() to BrowserTestBase lands.

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
CreditAttribution: voleger as a volunteer and at GOLEMS GABB, Drupal Ukraine Community for Drupal Ukraine Community, GOLEMS GABB commented
Title: Convert AJAX part of \Drupal\responsive_image\Tests\ResponsiveImageFieldUiTest to JavascriptTestBase and the rest to BrowserTestBase » [PP-1] Convert AJAX part of \Drupal\responsive_image\Tests\ResponsiveImageFieldUiTest to JavascriptTestBase and the rest to BrowserTestBase
Status: Active » Postponed

Set proper status.
Postponed on #2917885: Add drupalPostFormWithInvalidOptions() to BrowserTestBase

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude as a volunteer and at Dx Experts commented
Status: Postponed » Active

Can we find a way to work around the hidden fields?

brentg’s picture

brentg
Primary language Dutch
Location Ghent
CreditAttribution: brentg as a volunteer commented
Assigned: Unassigned » brentg
Issue tags: -LutskGSW18
brentg’s picture

brentg
Primary language Dutch
Location Ghent
CreditAttribution: brentg as a volunteer commented
FileSize
2809513-convert-responsiveimagefielduitest-14.patch11.58 KB

Still working on this issue, converted a first part of the old test, still have to convert the last part, but uploading my current progress already.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude as a volunteer and at Dx Experts commented
Title: [PP-1] Convert AJAX part of \Drupal\responsive_image\Tests\ResponsiveImageFieldUiTest to JavascriptTestBase and the rest to BrowserTestBase » Convert AJAX part of \Drupal\responsive_image\Tests\ResponsiveImageFieldUiTest to JavascriptTestBase and the rest to BrowserTestBase
Status: Active » Needs review
FileSize
interdiff-2809513-14-15.txt5.2 KB
2809513-15.patch11.24 KB

@brentgees really nice work on this! Since this is one of the last remaining conversions, I'm giving this a little push.

brentg’s picture

brentg
Primary language Dutch
Location Ghent
CreditAttribution: brentg as a volunteer commented

Thanks for the push, glad to see most of my code was already decent.

In my opinion your test looks good, but I would prefer someone else having a look as well before putting this to reviewed since my knowledge is a bit limited on WebDriverTestBase.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran at PreviousNext commented
Status: Needs review » Reviewed & tested by the community

Thanks, patch looks great RTBC.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed 56658f00e7 to 8.7.x and 6531ee4b1c to 8.6.x. Thanks!

I tested locally on 8.6.x and everything passed.

+++ b/core/modules/responsive_image/src/FunctionalJavascript/ResponsiveImageFieldUiTest.php
@@ -0,0 +1,164 @@
+use Drupal\Tests\field_ui\Traits\FieldUiTestTrait;
...
+
+
+  use FieldUiTestTrait;

I don't think we need this trait at all.

I ran the tests locally without it and everything was fine.

diff --git a/core/modules/responsive_image/src/FunctionalJavascript/ResponsiveImageFieldUiTest.php b/core/modules/responsive_image/src/FunctionalJavascript/ResponsiveImageFieldUiTest.php
index a726b75b95..e7e3f753a0 100644
--- a/core/modules/responsive_image/src/FunctionalJavascript/ResponsiveImageFieldUiTest.php
+++ b/core/modules/responsive_image/src/FunctionalJavascript/ResponsiveImageFieldUiTest.php
@@ -4,7 +4,6 @@
 
 use Drupal\FunctionalJavascriptTests\WebDriverTestBase;
 use Drupal\responsive_image\Entity\ResponsiveImageStyle;
-use Drupal\Tests\field_ui\Traits\FieldUiTestTrait;
 
 /**
  * Class ResponsiveImageFieldUiTest.
@@ -13,9 +12,6 @@
  */
 class ResponsiveImageFieldUiTest extends WebDriverTestBase {
 
-
-  use FieldUiTestTrait;
-
   /**
    * Modules to install.
    *

Did this on commit.

  • alexpott committed 56658f0 on 8.7.x 
    Issue #2809513 by Lendude, brentgees: Convert AJAX part of \Drupal\...

  • alexpott committed 6531ee4 on 8.6.x 
    Issue #2809513 by Lendude, brentgees: Convert AJAX part of \Drupal\...

Status: Fixed » Closed (fixed)

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