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
1) Drupal\Tests\quickedit\FunctionalJavascript\CKEditor5IntegrationTest::testArticleNode
Field node/1/body/en/full did not match its expectation selector (.quickedit-field.quickedit-editable.quickedit-candidate.quickedit-highlighted.quickedit-editing.quickedit-changed)
Failed asserting that a NULL is not empty.
/var/www/html/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:122
/var/www/html/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:55
/var/www/html/core/modules/quickedit/tests/src/FunctionalJavascript/QuickEditJavascriptTestBase.php:267
/var/www/html/core/modules/quickedit/tests/src/FunctionalJavascript/QuickEditJavascriptTestBase.php:244
/var/www/html/core/modules/quickedit/tests/src/FunctionalJavascript/CKEditor5IntegrationTest.php:268
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
— in both https://www.drupal.org/pift-ci-job/2507614 and https://www.drupal.org/pift-ci-job/2507615
Occurred at #3313473-24: CKEditor 5 plugin definitions should be derivable.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | just-quickedit-tests.patch | 1.3 KB | bnjmnm |
| |||
| #12 | 3317515-12.patch | 839 bytes | bnjmnm |
| #5 | 3317515-5.patch | 3.97 KB | longwave |
| #4 | 3317515.patch | 739 bytes | catch |
Comments
Comment #2
Wim LeersComment #3
longwaveThis is no longer random and appears to be affecting all 9.4.x/9.5.x test runs at present: https://www.drupal.org/node/3060/qa
Comment #4
catchLet's skip it and re-enable in a follow-up once it's stable.
Comment #5
longwaveReverting #3316816: Stabilize FunctionalJavascript testing AJAX: make ::setValue() trigger both "input" and "formUpdated" events fixes this.
Comment #6
Wim LeersPer #5 … how is it possible that the exact same tests on the exact same JS codebase behave differently on 9.4 + 9.5 versus 10.0 + 10.1?
There must be some difference?
Comment #7
longwaveQuickedit no longer exists in the 10.x codebase!
Comment #8
catchI think we should go with #4 to take the pressure off, then try to fix the test.
#3316816: Stabilize FunctionalJavascript testing AJAX: make ::setValue() trigger both "input" and "formUpdated" events improves test stability overall, so we shouldn't go backwards there. Mea culpa for not doing an explicit 9.5.x test run though!
Comment #9
longwaveAgree that #4 is the right fix for now, marking RTBC.
I've played around with this test for a little while but can't see where the actual problem lies. The reason for failure is that the
.quickedit-changedclass is missing from the body field. This is only added or removed in Quick Edit's FieldDecorationView but I don't see what has changed; it is initially present (another assertion confirms this) and the events seem to fire in the same order, but if I remove the blur event handler from setValue() then the problem goes away - but setValue() appears to be called much earlier than when the problem occurs, so I'm pretty confused as to what is going on.Comment #10
longwaveAlso note that if a change is required to Quick Edit itself here, we need to port that change to the contrib module, as the tests are failing in the exact same way over there.
Comment #11
Wim Leers#8++ and #9++
Comment #12
bnjmnmThis fixes it. The test apparently wants the blur, although I didn't spot anything obvious in the backbone or quickedit regarding focus or blur events.
If there's hesitation with the approach I'm all for getting the skipped test in and dealing with this in a followup.
Comment #13
longwaveNice work! I was playing with the same method but didn't figure out the fix, to me this looks like a good enough solution and so #12 is RTBC.
Comment #15
catchCommitted/pushed #12 to 9.5.x and 9.4.x, thanks! Nice one actually fixing it so quickly.