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 » user.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.

Tess Bakker’s picture

Assigned: Unassigned » Tess Bakker
Issue tags: +DevDaysLisbon
Tess Bakker’s picture

Assigned: Tess Bakker » Unassigned
Status: Active » Needs review
FileSize
7 KB

Patch with:
- conversion of the AJAX part to Webdriver test
- other tests to Browser test (with some speed improvement, by using the testing profile)

Tess Bakker’s picture

Title: Convert AJAX part of \Drupal\user\Tests\UserPasswordResetTest to JavascriptTestBase » Convert AJAX part of \Drupal\user\Tests\UserPasswordResetTest to WebDriverTestBase
Lendude’s picture

Status: Needs review » Needs work

This looks great, just some minor clean up as far as I can see:

  1. +++ b/core/modules/user/src/Tests/Functional/UserPasswordResetTest.php
    @@ -14,16 +15,9 @@
    -  protected $profile = 'standard';
    

    So nice that the browser test doesn't need this anymore!

  2. +++ b/core/modules/user/src/Tests/Functional/UserPasswordResetTest.php
    @@ -239,6 +225,8 @@ public function testUserPasswordResetLoggedIn() {
    +    $this->drupalLogout();
    +    $this->drupalGet('<front>');
    

    Still weird that this is needed, but I saw it fail otherwise....so lets keep it

  3. +++ b/core/modules/user/src/Tests/FunctionalJavascript/UserPasswordResetTest.php
    @@ -0,0 +1,137 @@
    +  /**
    +   * Modules to enable.
    +   *
    +   * @var array
    +   */
    

    This can just be {@inheritdoc} in the new test.

  4. +++ b/core/modules/user/src/Tests/FunctionalJavascript/UserPasswordResetTest.php
    @@ -0,0 +1,137 @@
    +    // Enable page caching.
    +    $config = $this->config('system.performance');
    +    $config->set('cache.page.max_age', 3600);
    +    $config->save();
    +
    +    $this->drupalPlaceBlock('system_menu_block:account');
    

    Do we need the block in the javascript test? Don't think so, right? And the page caching?

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 work » Needs review
FileSize
1.44 KB
6.38 KB

Addressed my own feedback

#9.2 works on my machine when I take it out, lets see what the Bot thinks

borisson_’s picture

Status: Needs review » Reviewed & tested by the community

The Bot, it agrees! As do I, this looks very solid and #11 resolves #9.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed e4146aa44f to 8.7.x and 7a3a01f34f to 8.6.x. Thanks!

  • alexpott committed e4146aa on 8.7.x
    Issue #2809543 by Lendude, Tessa Bakker: Convert AJAX part of \Drupal\...

  • alexpott committed 7a3a01f on 8.6.x
    Issue #2809543 by Lendude, Tessa Bakker: Convert AJAX part of \Drupal\...
Lendude’s picture

Status: Fixed » Needs review
FileSize
1.4 KB

Whoops, this actually forgot to move the new tests to the right spot and failed to set the right namespace!

navneet0693’s picture

Status: Needs review » Reviewed & tested by the community

The last submitted patch, 7: 2809543-7.patch, failed testing. View results

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Unfortunately the patch does not apply.

Lendude’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
1.42 KB

Rerolled for 8.7.x

borisson_’s picture

Status: Needs review » Reviewed & tested by the community

The patch applies again.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed 57dfd4ba8c to 8.7.x and a42db5a7ee to 8.6.x. Thanks!

  • alexpott committed 57dfd4b on 8.7.x
    Issue #2809543 by Lendude, Tessa Bakker: Convert AJAX part of \Drupal\...

  • alexpott committed a42db5a on 8.6.x
    Issue #2809543 by Lendude, Tessa Bakker: Convert AJAX part of \Drupal\...

Status: Fixed » Closed (fixed)

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