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
CKEditor test fails because it is trying to compare two arrays that are equal but have a different order of elements.
Proposed resolution
ksort both arrays.
Remaining tasks
Review patch
User interface changes
API changes
Comment | File | Size | Author |
---|---|---|---|
#9 | 2465917-9.patch | 1.36 KB | stefan.r |
#7 | 2465917-7.patch | 1.36 KB | stefan.r |
#3 | 2465917-3.patch | 1.59 KB | stefan.r |
#1 | 2465917-1.patch | 1.59 KB | stefan.r |
Comments
Comment #1
stefan.r CreditAttribution: stefan.r commentedBefore:
After:
Comment #2
aspilicious CreditAttribution: aspilicious commentedMinor point:
The order of lines is different in both blocks. Can we fix that?
In the second block the two ksorts are just after each other in the first block they are not.
Pick one approach and use it for both.
Comment #3
stefan.r CreditAttribution: stefan.r commentedComment #4
Damien Tournoud CreditAttribution: Damien Tournoud at Centarro commentedIsn't
->assertEqual()
meant precisely for that?Comment #5
aspilicious CreditAttribution: aspilicious commentedLooking good :)
Comment #6
Damien Tournoud CreditAttribution: Damien Tournoud at Centarro commentedAlso. Strict array comparisons (i.e.
->assertIdentical()
) have always been order-sensitive. So the order here changed for some reason. Can we track out why, and make sure this is not the symptom of an actual bug somewhere else?Comment #7
stefan.r CreditAttribution: stefan.r commented@Damien Tournoud well the thing that's changed is PHP7 builds its arrays differently (see https://github.com/bolt/bolt/issues/3275), I will dig a bit, isolate this issue and see if the different sorting introduces an actual bug.
Comment #9
stefan.r CreditAttribution: stefan.r commentedassertEqual makes tests pass as well -- other tests do do assertIdentical + ksort($expected), but we need to sort the array with the actual result as well in this case, so may as well do assertEqual if we're going to deviate.
The only difference between the expected and the actual array is that the 'disallowedContent' and 'allowedContent' keys are inverted in PHP7, due to this line of code:
Which works differently in PHP7:
Comment #10
stefan.r CreditAttribution: stefan.r commentedComment #11
Damien Tournoud CreditAttribution: Damien Tournoud at Centarro commentedThat's a known side effect of the introduction of the AST in PHP7.
->assertEqual()
is what the test intended to use, so let's go with this.Comment #12
webchickGreat sleuthing here, folks!
Committed and pushed to 8.0.x. Thanks!