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

While converting testWizardForm tests by adding the assertions to the existing tests in \Drupal\Tests\views_ui\FunctionalJavascript\ViewsWizardTest::testCreateViewWizard a problem with the Block rowStyleOptions was raised.

Currently the rowStyleOptions for Block do not get checked for valid options, so when changing wizard type the Block rowStyleOptions can contain an invalid option which results in a 500 error on the Ajax call trying to update the wizard form.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#6 2809549-6.patch5.33 KBLendude
#6 interdiff-2809549-3-6.txt3.41 KBLendude
#3 2809549-3.patch3.31 KBLendude
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner created an issue. See original summary.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at Noctilaris commented

The assertions this test does can probably be added to \Drupal\Tests\views_ui\FunctionalJavascript\ViewsWizardTest::testCreateViewWizard

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at Noctilaris commented
Status: Active » Needs review
FileSize
2809549-3.patch3.31 KB

So something like this.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at Noctilaris commented 
+++ b/core/modules/views_ui/tests/src/FunctionalJavascript/ViewsWizardTest.php
@@ -56,6 +56,22 @@ public function testCreateViewWizard() {
+    $page->findField('block[create]')->click();

This is needed the second time to avoid getting:

An AJAX HTTP error occurred.
HTTP Result Code: 500
Debugging information follows.
Path: /admin/structure/views/add?ajax_form=1
StatusText: Internal Server Error
ResponseText: {"message":"A fatal error occurred: The \u0022titles_linked\u0022 plugin does not exist."}

Something goes wrong in \Drupal\views\Plugin\views\wizard\WizardPluginBase::buildFormStyle I guess? But don't know if that is a testing artefact or something real. A little clicking around the wizard outside the test didn't show anything, but didn't really dig too deep.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at Noctilaris commented

Oh I lie, some more clicking and I killed it. Changing the entity type doesn't change the block display format, it does update the page display format.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at Noctilaris commented
FileSize
interdiff-2809549-3-6.txt3.41 KB
2809549-6.patch5.33 KB

The power of proper javascript tests vs faking them!

Hijack this issue and turn it into a bug or open a new issue?

Anyway, here is the conversion with the bug fix to make it work and some additional tests for the row style change that happens when changing the wizard type.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at Noctilaris commented
Title: Convert AJAX part of \Drupal\views\Tests\Wizard\BasicTest::testWizardForm to JavascriptTestBase » Convert AJAX part of \Drupal\views\Tests\Wizard\BasicTest::testWizardForm to JavascriptTestBase and fix resulting Block row display options
Category: Task » Bug report
Issue summary: View changes
Issue tags: +VDC

Discussed with @dawehner on IRC. Since making a separate issue with tests for this bug would completely overlap with the conversion, we will just upgrade this to a bug

Updated IS to reflect the change.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Status: Needs review » Reviewed & tested by the community

nice bugfix!

  • catch committed c271cea on 8.3.x 
    Issue #2809549 by Lendude: Convert AJAX part of \Drupal\views\Tests\...

  • catch committed eda7aeb on 8.2.x 
    Issue #2809549 by Lendude: Convert AJAX part of \Drupal\views\Tests\...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 8.3.x-dev » 8.2.x-dev
Status: Reviewed & tested by the community » Fixed

Looks great. Since it's a bug fix now rather than just a conversion issue, putting it in 8.2.x as well.

Committed/pushed to 8.3.x and cherry-picked to 8.2.x. Thanks!

Status: Fixed » Closed (fixed)

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