Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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
- Figure out which part of the test is testing PHP code and which part ajax behaviour
- Extract the ajax behaviour into a test that extends JavascriptTestBase
Remaining tasks
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#12 | 2809495-12.patch | 22.19 KB | Lendude |
#12 | interdiff-2809495-9-12.txt | 2.01 KB | Lendude |
#9 | 2809495-9.patch | 22.21 KB | Lendude |
Comments
Comment #3
michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commentedComment #7
LendudeHere we go.
Split out the part that does actual ajax stuff. The smaller tests I just used drupalPostForm since those steps get Javascript coverage in the other test, no need to duplicate that. Same goes for the part that makes a view etc, just kept that simple.
A couple of unavoidable assertWaitOnAjaxRequest in there for changes that don't change anything in the DOM, so there is nothing else to wait for.
Comment #8
jibranThe patch looks great here are some suggestions:
I think we can simplfiy this.
This is what DER is doing, HT:@dpi
Let's create $page.
Let's create $assert_session instead of $this->assertSession() everywhere.
Comment #9
Lendude@jibran thanks for the review!
#8.1 I cleaned it up a bit, but I kinda like the wait using the xpath containing the text() because the rest of the test depends on the machine name being that exact string. Otherwise we could have a situation where the machine name is set, but not yet set to that string (if the input has a slight delay midway through for example, which prematurely triggers setting the machine name), which would lead to a random fail. Fairly hypothetical to be sure, but why risk it.
#8.2 fixed
#8.3 fixed
Cleaned up some more. Also went to work on assertFieldSelectOptions which was going into a loop for some reason in the JS test, something not right in the optgroup handling. So rewrote that a bit and uncommented the assert that was blocking me before.
Comment #10
jibranRE: #9.1 Personally, I don't like using xpath because we have cssSelect now, but I do understand your point :). The core is setting an example here so I disagree with your approach because it is overly complicated but I don't want to hold the patch for this anymore so let's committer decide what is a better DX.
$assert_session->waitForElement('xpath', '//*[@id="edit-label-machine-name-suffix"]/span[2]/span[contains(text(), "field_test")]');
OR
$assert_session->waitForElementVisible('css', '[name="label"] + * .machine-name-value');
Edit: FWIW,
waitForElementVisible
returns the result element and we can assert its value as well.Comment #11
larowlanThis adds some new PHPCS fails
Comment #12
LendudeFeedback addressed.
Comment #13
jibranBack to RTBC. @larowlan any opinion on #10?
Comment #14
alexpottCommitted and pushed 825a340641 to 8.7.x and edb0e47428 to 8.6.x. Thanks!
Backported to 8.6.x to keep the tests aligned.