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

mondrake created an issue. See original summary.

mondrake’s picture

Status: Active » Needs review
StatusFileSize
new1.34 KB

Patch

mondrake’s picture

Component: entity system » field system
Issue summary: View changes
amateescu’s picture

Status: Needs review » Reviewed & tested by the community

Wow, 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.

mondrake’s picture

I guess we don't have a way to test this with the db drivers provided by core, do we?

I 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.

xjm’s picture

Status: Reviewed & tested by the community » Needs work

Nice 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?

mondrake’s picture

Status: Needs work » Needs review
StatusFileSize
new1.66 KB
new1.26 KB

Sure, here we go

amateescu’s picture

After 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.

mondrake’s picture

StatusFileSize
new1.56 KB
new1.94 KB

Oh I didn't know assertEquals on arrays would not be looking at the sequence of the keys. Sure then it makes sense.

amateescu’s picture

Status: Needs review » Reviewed & tested by the community

Awesome, thank you!

mondrake’s picture

Status: Reviewed & tested by the community » Needs work

It will certainly pass in core, but I haven't checked with contrib... let me do that and I'll re-RTBC if all fine.

mondrake’s picture

Status: Needs work » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 9: 2925750-9.patch, failed testing. View results

mondrake’s picture

Status: Needs work » Reviewed & tested by the community

#13 was a DrupalCI abort

  • xjm committed 5bba892 on 8.5.x
    Issue #2925750 by mondrake, amateescu, xjm: EntityDefinitionUpdateTest...
xjm’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.5.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.