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

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

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#16 interdiff-2809499-14-16.txt737 bytesApacheEx
#16 2809499-16.patch14.38 KBApacheEx
#14 2809499-14.patch14.39 KBApacheEx
#10 2809499-10.patch5.03 KBApacheEx
#9 interdiff-2809499-5-9.txt1.76 KBApacheEx
#9 2809499-9.patch11.56 KBApacheEx
#5 2809499-5.patch11.44 KBApacheEx

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 commented
Component: phpunit » field system
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 commented
Status: Active » Needs review
StatusFileSize
new 2809499-5.patch11.44 KB

Here is a patch. Only testNumberFormatter has ajax part.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Needs work

Thank you for starting to work on those issues! Highly appreciated

+++ b/core/modules/field/tests/src/FunctionalJavascript/Number/NumberFieldTest.php
@@ -0,0 +1,187 @@
+    $page->pressButton("${float_field}_settings_edit");
+    $assert_session->assertWaitOnAjaxRequest();
...
+    $assert_session->assertWaitOnAjaxRequest();
+    $this->drupalPostForm(NULL, [], t('Save'));
...
+    $page->pressButton("${integer_field}_settings_edit");
+    $assert_session->assertWaitOnAjaxRequest();

As in the other issue, let's not use assertWaitOnAjaxRequest but rather wait till a certain dom element appears.

ApacheEx’s picture

ApacheEx commented

I didn't know, but now I know, will update all my patches.

p.s thank you for reviewing.

dawehner’s picture

dawehner
Primary language German
commented

@ApacheEx
I'm happy to review every JS test patch you ever post :)

ApacheEx’s picture

ApacheEx commented
Status: Needs work » Needs review
StatusFileSize
new 2809499-9.patch11.56 KB
new interdiff-2809499-5-9.txt1.76 KB

Here is updated patch :)

ApacheEx’s picture

ApacheEx commented
StatusFileSize
new 2809499-10.patch5.03 KB

Whole NumberFieldTest can be converted to BrowserTestBase with minimal changes.
So, why not to prefer this patch.

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.

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
commented
Status: Needs review » Needs work

Not convinced just converting to BrowserTestBase is the best option here, it's really nice to have the widget covered by a real javascript test I think. So for me, I'd say lets update #9 to use WebDriverTestBase and go with that, but open to be persuaded otherwise :)

ApacheEx’s picture

ApacheEx commented
Status: Needs work » Needs review
StatusFileSize
new 2809499-14.patch14.39 KB

yes, I agree 100%.

So, I moved NumberFieldTest::testNumberFormatter to WebDriverTestBase and rest of NumberFieldTest to BTB.

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented

@ApacheEx nice! looking great. Just some really really minor nits:

  1. +++ b/core/modules/field/tests/src/FunctionalJavascript/Number/NumberFieldTest.php
@@ -0,0 +1,183 @@
+use Drupal\field\Entity\FieldConfig;
+use Drupal\FunctionalJavascriptTests\WebDriverTestBase;
+use Drupal\node\Entity\Node;
+use Drupal\field\Entity\FieldStorageConfig;

    nittiest of nit picks: these need to be alphabetical 

  2. +++ b/core/modules/field/tests/src/FunctionalJavascript/Number/NumberFieldTest.php
@@ -0,0 +1,183 @@
+ * Tests the creation of numeric fields.

    Comment needs an update for the new test I think: 'Tests the numeric field widget.' Or something like that?

ApacheEx’s picture

ApacheEx commented
StatusFileSize
new 2809499-16.patch14.38 KB
new interdiff-2809499-14-16.txt737 bytes

Thanks for your feedback. All things make sense for me :)

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Status: Needs review » Reviewed & tested by the community

@ApacheEx thanks!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed a15fd23c6c to 8.7.x and 51402a7051 to 8.6.x. Thanks!

  • alexpott committed a15fd23 on 8.7.x 
    Issue #2809499 by ApacheEx, dawehner, Lendude: Convert AJAX part of \...

  • alexpott committed 51402a7 on 8.6.x 
    Issue #2809499 by ApacheEx, dawehner, Lendude: Convert AJAX part of \...

Status: Fixed » Closed (fixed)

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