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

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

Component: phpunit » system.module
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.

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.

martin107’s picture

Status: Active » Needs review
FileSize
2.02 KB

I have been nudged by @vaplas

in #2862510: Convert system/tests/src/Ajax to JavascriptTestBase I had created a monster patch which made changes to many test files.... it was unreviewable.

I have cut out the group test stuff from the last patch in that issue see

https://www.drupal.org/project/drupal/issues/2862510#comment-12225746

and created a new patch -- it should serve as a good starting point.

Status: Needs review » Needs work

The last submitted patch, 6: cutout-2809521-6.patch, failed testing. View results

Anonymous’s picture

Status: Needs work » Needs review
FileSize
2.4 KB
1.82 KB

I have been nudged by @vaplas

🤣 I would call this a venerable entreaty) Great thanks!

Added minimal AjaxTestBase class for that test + made patch with diff -M40 for more readability.

AjaxTestBase without method assertCommand() from WTB::AjaxTestBase, because it does not really check any js-behavior, and it will probably be moved to the BTB test-level in the future (or to Trait).

martin107’s picture

By my way of thinking this patch is central to making progress in other phpunit initiative tests.

So by way of trade can I review any other D8 core issues in return for a review here?

This patch is now blocking these issues.

#2809529: Convert AJAX part of \Drupal\system\Tests\Ajax\ElementValidationTest::testAjaxElementValidation to JavascriptTestBase
#2809531: Convert AJAX part of \Drupal\system\Tests\Ajax\FormValuesTest::testSimpleAjaxFormValue to JavascriptTestBase
#2809535: Convert AJAX part of \Drupal\system\Tests\Ajax\MultiFormTest to JavascriptTestBase

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

Status: Needs review » Needs work

Thanks so much for working on this!

+++ b/core/tests/Drupal/FunctionalJavascriptTests/Ajax/AjaxTestBase.php
@@ -0,0 +1,17 @@
+abstract class AjaxTestBase extends JavascriptTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  public static $modules = ['node', 'ajax_test', 'ajax_forms_test'];
+
+}

Hmm this looks like it has really minimal value. Do we really need this? Can't we just extend from WebDriverTestBase directly?

martin107’s picture

Title: Convert AJAX part of \Drupal\system\Tests\Ajax\AjaxInGroupTest to JavascriptTestBase » Convert AJAX part of \Drupal\system\Tests\Ajax\AjaxInGroupTest to WebDriverTestBase
Status: Needs work » Needs review
FileSize
3.64 KB
1.33 KB

Yep, time passes and now introducing AjaxTestBase looks like a really bad idea.

Thank you.

martin107’s picture

Sorry bad patch... my bad

Status: Needs review » Needs work

The last submitted patch, 13: 2809521-13.patch, failed testing. View results

martin107’s picture

Status: Needs work » Needs review
FileSize
3.07 KB

Wipes egg off my face and quietly removes local files.

Status: Needs review » Needs work

The last submitted patch, 15: 2809521-15.patch, failed testing. View results

martin107’s picture

Status: Needs work » Needs review
FileSize
1.94 KB
3.21 KB

Hmm that was passing locally ...on php7.23 not sure what explains that, but code related to the error report is deprecated code so....

stopped using

$this->assert()

& started using

$this->assertSession()->responseContains()

Status: Needs review » Needs work

The last submitted patch, 17: 2809521-17.patch, failed testing. View results

Lendude’s picture

Status: Needs work » Needs review
FileSize
555 bytes
3.33 KB

It was missing the module with test path.

martin107’s picture

Thank you, thank you.

Lendude++

borisson_’s picture

Status: Needs review » Reviewed & tested by the community

This looks great!

The last submitted patch, 8: 2809521-8.patch, failed testing. View results

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 7e55e67 and pushed to 8.7.x. Thanks!
Committed e4d867f and pushed to 8.6.x. Thanks!

  • alexpott committed 7e55e67 on 8.7.x
    Issue #2809521 by martin107, Lendude, vaplas: Convert AJAX part of \...

  • alexpott committed e4d867f on 8.6.x
    Issue #2809521 by martin107, Lendude, vaplas: Convert AJAX part of \...

Status: Fixed » Closed (fixed)

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