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.I've seen this one in #1825450-3: Use lock service in lock() and #1825466-3: Allow NestedArray::mergeDeepArray() to preserve integer keys and other times as well I think.
Time to track this down. The exact error is
Entity not found in the database. Other EntityFormTest.php 66 Drupal\system\Tests\Entity\EntityFormTest->testFormCRUD()
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | d8_entity_tests.patch | 4.22 KB | fago |
| #15 | d8_entity_tests_only.patch | 3.06 KB | fago |
| #11 | 10-11-interdiff.txt | 565 bytes | alexpott |
| #11 | 1825568-11.random-fails-in-entity-form-test.patch | 2.8 KB | alexpott |
| #10 | 1825568-10.random-fails-in-entity-form-test.patch | 2.78 KB | alexpott |
Comments
Comment #1
sunLet's make sure to always tag such issues with "Random test failure". In the long run, we want to learn from previous mistakes.
Comment #2
alexpottThis fail occurs when the user_id is 0. Haven't work out exactly why yet...
Comment #3
alexpottAs so the $name1 and $name2 could will cause failures if they're the same so I've made them different lengths.
Comment #4
BerdirOk, so the actual problem here is that when the uid is 0, then it fails the form validation and we never attempt to delete.
Debug output shows "UID field is required.".
Can we add an assertion message for "entity_test 1 has been deleted."? That might tell us a bit better what exactly the problem is if we encounter something like this again.
Not sure if we should just change anything in the form handling/validation (like adding limit validation errors on the delete button) or just stick to mt_rand(1, 128) and live with it.
Comment #5
alexpottActually this issue is due to
if (!empty($values)) {in the setValue method of Drupal\Core\Entity\Field\Type\Field. If $values = 0 then this does not set the value to 0 as 0 the same as an empty array to the empty function.I think the correct fix is the patch attached. I've attached a provethisworks.patch that sets the user_id to 0 to, well, prove this works!
Comment #6
alexpottIgnore patches in #5 :( borked git diff...
Comment #7
alexpotttagging
Comment #8
BerdirMakes sense I think and is related to the previous random test failure issue at #1798382: Random Test failures in Entity translation tests.
Maybe we can extend the tests that were added there instead of using the entity form tests for that?
Comment #9
BerdirCrosspost.
Comment #10
alexpottNow with added test :)
Comment #11
alexpottNoticed that the change to setValue will break stuff in $values is an empt array. Patch attached fixes that.
Comment #12
BerdirLooks great. Has test coverage and fixes another ugly random test failure.
Comment #13
BerdirWeird double post.
Comment #14
catchThanks folks! Committed/pushed to 8.x.
Comment #15
fagoI also ran into this issue while working on #1778178: Convert comments to the new Entity Field API, however didn't notice it's causing test failures. :/
The committed fix does not account for setting NULL properly. I've attached a patch with a test that shows that.
An easy and correct fix is to check for isset($value) and !== array() instead of using !empty(). Attached patch reverts the fix to do so and adds the mentioned test for NULL. Also, I've moved over the test for setting to 0 to the entity-field test case as it's not specific to translation at all.
Patches attached, tests only should fail.
Comment #16
alexpottTested locally... and this looks good and makes sense. If #15 is green this is rtbc.
Comment #18
BerdirTests passed/failed as expected, they were just in the wrong order. Back to RTBC.
Comment #19
webchickGreat work all, thanks.
Committed and pushed to 8.x.