Problem/Motivation
Since the commit of #2282119: Make the Entity Field API handle field purging, EntityDefinitionUpdateTest fails with contrib db driver.
1) Drupal\KernelTests\Core\Entity\EntityDefinitionUpdateTest::testBaseFieldDeleteWithExistingData
Failed asserting that Array &0 (
'deleted' => '1'
'entity_id' => '2'
'langcode' => 'en'
'delta' => '0'
'bundle' => 'entity_test_update'
'revision_id' => '2'
'new_base_field_value' => 'foo'
) is identical to Array &0 (
'bundle' => 'entity_test_update'
'deleted' => '1'
'entity_id' => '2'
'revision_id' => '2'
'langcode' => 'en'
'delta' => '0'
'new_base_field_value' => 'foo'
).
/home/travis/drupal8/core/tests/Drupal/KernelTests/Core/Entity/EntityDefinitionUpdateTest.php:444
2) Drupal\KernelTests\Core\Entity\EntityDefinitionUpdateTest::testBundleFieldDeleteWithExistingData
Failed asserting that Array &0 (
'deleted' => '1'
'entity_id' => '1'
'langcode' => 'en'
'delta' => '0'
'bundle' => 'test_bundle'
'revision_id' => '1'
'new_bundle_field_value' => 'foo'
) is identical to Array &0 (
'bundle' => 'test_bundle'
'deleted' => '1'
'entity_id' => '1'
'revision_id' => '1'
'langcode' => 'en'
'delta' => '0'
'new_bundle_field_value' => 'foo'
).
/home/travis/drupal8/core/tests/Drupal/KernelTests/Core/Entity/EntityDefinitionUpdateTest.php:511
This is because the test checks for the identicality of an expected array to the actual results. The assert relies on the fact that not only array values are the same, but also about the sequence of the keys in the array.
Proposed resolution
Sort both the expected and the actual array by key before asserting.
Remaining tasks
review
User interface changes
na
API changes
na
Data model changes
na
Comments
Comment #2
mondrakePatch
Comment #3
mondrakeComment #4
amateescu commentedWow, I guess we don't have a way to test this with the db drivers provided by core, do we?
I have to say I'm bit surprised though, until now I thought that PostgreSQL is the most nitpicky database on the matter of result sorting.
Comment #5
mondrakeI also think no, we don't.
I have an experimental project to use Doctrine DBAL as a db driver, https://github.com/mondrake/drudbal
This runs selected core tests on TravisCI, and every now and then I find failures that I am trying to report. That's it, I hope it can help the community.
Comment #6
xjmNice find.
Since it looks like we can't test it on the core test suite, can we add an inline comment to both spots explaining why we're making the change, so we don't helpfully refactor the fix away later in some cleanup?
Comment #7
mondrakeSure, here we go
Comment #8
amateescu commentedAfter sleeping on it for a day, maybe it would be simpler if we just change the assertion to
assertEquals()instead of manually re-ordering the keys. What do you think?Of course, adding a comment about it makes sense regardless of that.
Comment #9
mondrakeOh I didn't know assertEquals on arrays would not be looking at the sequence of the keys. Sure then it makes sense.
Comment #10
amateescu commentedAwesome, thank you!
Comment #11
mondrakeIt will certainly pass in core, but I haven't checked with contrib... let me do that and I'll re-RTBC if all fine.
Comment #12
mondrakeYep, all good.
(if interested, here's the pass https://travis-ci.org/mondrake/drudbal/builds/306744286 and here's the fail https://travis-ci.org/mondrake/drudbal/builds/305137646)
Comment #14
mondrake#13 was a DrupalCI abort
Comment #16
xjmCommitted and pushed to 8.5.x. Thanks!
Comment #18
mondrakeAgain, now because of #2923567: Select from correct entity tables for additional entity/field definitions when moving data to dedicated table storage from shared table storage for the purposes of purging.
Comment #19
mondrakeFiled #2930197: EntityDefinitionUpdateTest fails with contrib db driver (again)