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

Status: Active » Needs review
FileSize
11.87 KB

Here is a patch :)

ApacheEx’s picture

FileSize
11.95 KB
1.65 KB

Based on @dawehner review in previous issues here is updated patch.

Lendude’s picture

Status: Needs review » Needs work

Already looking nice, but couple of things:

  1. +++ b/core/modules/field/tests/src/FunctionalJavascript/Boolean/BooleanFieldTest.php
    @@ -0,0 +1,193 @@
    +    // Verify that boolean value is displayed.
    +    $entity = EntityTest::load($id);
    +    $display = entity_get_display($entity->getEntityTypeId(), $entity->bundle(), 'full');
    +    $content = $display->build($entity);
    +
    +    $output = \Drupal::service('renderer')->renderRoot($content);
    +    $text = '<div class="field__item">' . $on . '</div>';
    +    $regex = '/' . preg_quote($text, '/') . '/ui';
    +    $message = sprintf('The string "%s" was not found anywhere in the HTML response of the current page.', $text);
    +    $this->assert((bool) preg_match($regex, $output), $message);
    

    This is a replacement for setRawContent() which is something we only do in Kernel tests now. So, either that has to be taken out and moved to a kernel test, or we should try to find a way to do that render on a URL we can call. The latter would be better since then we have an actual functional test. Doesn't entity_test have a detail view we can render this field on?

  2. +++ b/core/modules/field/tests/src/FunctionalJavascript/Boolean/BooleanFieldTest.php
    @@ -0,0 +1,193 @@
    +    $assert_session->waitForElement('css', '.field-plugin-summary-cell > .ajax-new-content');
    

    All 'waitfor' methods should be followed by an assertion that the element is actually on the page, otherwise we might just be waiting 10 seconds for nothing.
    I think all the other waits already have this, just missed this one :)

ApacheEx’s picture

Status: Needs work » Needs review
FileSize
4.93 KB

I'm surprized that whole BooleanFieldTest can be converted to BrowserTestBase :) So, no Javascript dependency.
Here is a patch.

p.s. Based on @vaplas comment here #2809493: Convert AJAX part of \Drupal\field\Tests\Boolean\BooleanFormatterSettingsTest::testBooleanFormatterSettings to JavascriptTestBase

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.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

@ApacheEx
I guess this really shows that our no JS fallback work, I think that's enough. Boolean fields don't actually add anything custom ajax wise to those forms.

Status: Reviewed & tested by the community » Needs work

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

ApacheEx’s picture

Assigned: Unassigned » ApacheEx
Status: Needs work » Needs review
Issue tags: +LutskGSW18
FileSize
4.5 KB

Agree, thanks for review. Here is re-rolled patch with 8.6.x.

ApacheEx’s picture

Assigned: ApacheEx » Unassigned

Status: Needs review » Needs work

The last submitted patch, 12: 2809491-12.patch, failed testing. View results

ApacheEx’s picture

Status: Needs work » Reviewed & tested by the community

Re-rolled patch was failed first time because of some another unrelated issue, now it's passed. Moved again to RTBC based on #10

  • catch committed f2c5cfd on 8.6.x
    Issue #2809491 by ApacheEx, dawehner, Lendude: Convert AJAX part of \...

  • catch committed f006764 on 8.5.x
    Issue #2809491 by ApacheEx, dawehner, Lendude: Convert AJAX part of \...
catch’s picture

Version: 8.6.x-dev » 8.5.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.6.x and cherry-picked to 8.5.x. Thanks!

Status: Fixed » Closed (fixed)

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