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 |
---|---|---|---|
#24 | 2809505-24.patch | 11.35 KB | Lendude |
#24 | interdiff-2809505-20-24.txt | 2.23 KB | Lendude |
#20 | 2809505-20.patch | 11.34 KB | Lendude |
#20 | interdiff-2809505-16-20.txt | 2.49 KB | Lendude |
#16 | 2809505-16.patch | 11.1 KB | Lendude |
Comments
Comment #3
michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commentedComment #7
Lendude'Ere we go.
Biggest change is that the javascript version of the widget doesn't have an 'upload' button.
I went with
assertWaitOnAjaxRequest()
because that seemed to fit here, might want to change that to a waitForX.Comment #9
LendudeThat file logic works fine locally and in the BrowserTestBase version
Small tweak, lets see what the bot makes of that....
Comment #11
MixologicI think the issue here is that you need to use
attachFileToField
to actually attach the file to the file upload in the form.have a look at http://cgit.drupalcode.org/drupal/tree/core/modules/system/tests/src/Fun... for an example.
Comment #12
Lendude@mixologic++
here we go
Comment #14
LendudeLooking at
\Drupal\Tests\media\Functional\MediaRevisionTest
it might be that we need to use 'temporary://' because that is shared between containers?Works locally, lets see how the old bot feels about this one....
Comment #16
Lendude.... yeah ok, so fixed #11 in both spots now .... silly me
Comment #17
dawehnerI would have expected to use
tempnam()
hereI wonder how hard it would be to actually wait for a specific element to appear instead
Comment #18
borisson_The change to tempnam() looks like it would be an easy change. The other one sounds like it would be a lot harder to figure out.
Moving this back to NW in any case.
Comment #19
borisson_Comment #20
Lendude#17.1 fixed
#17.2 wait for works when waiting for something to appear, but we still don't have anything to wait for something to disappear, so stuck with assertWaitOnAjaxRequest for the remove operations.
Comment #21
LendudeComment #22
borisson_Awesome, thank for fixing/explaining the review points. This looks great now!
Comment #23
alexpott$this->assertGreaterThan()?
Is this assert deprecated?
db_query is deprecated - let's not add a usage.
Comment #24
Lendude@alexpott thanks for the review. Feedback addressed.
Comment #25
jibran#24 addressed #23 so back to RTBC.
Comment #26
alexpottCommitted and pushed be64fa993e to 8.7.x and 0cac1a84d7 to 8.6.x. Thanks!