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
When any update path test fails we just check for TRUE/FALSE whether we need some additional updates. This doesn't help people to figure out for example which updates didn't got executed in the test.
Proposed resolution
Do something along
+++ b/core/modules/system/src/Tests/Update/UpdatePathTestBase.php
@@ -265,7 +265,15 @@ protected function runUpdates() {
- $this->assertFalse(\Drupal::service('entity.definition_update_manager')->needsUpdates(), 'After all updates ran, entity schema is up to date.');
+ $needs_updates = \Drupal::entityDefinitionUpdateManager()->needsUpdates();
+ $this->assertFalse($needs_updates, 'After all updates ran, entity schema is up to date.');
+ if ($needs_updates) {
+ foreach (\Drupal::entityDefinitionUpdateManager()->getChangeSummary() as $entity_type_id => $summary) {
+ foreach ($summary as $message) {
+ $this->fail($message);
+ }
+ }
+ }
Remaining tasks
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#7 | 2747789-07.patch | 1.13 KB | jhedstrom |
#2 | improve_UpdatePathTestBase_debuggablity-2747789-2.patch | 1006 bytes | pashupathi nath gajawada |
Comments
Comment #2
pashupathi nath gajawada CreditAttribution: pashupathi nath gajawada as a volunteer and at Melity commentedHi @dawehner,
As proposed I have Impriovised the debuggability of the UpdatePathTestBase.
Please find the attached patch.
Comment #6
jhedstromThis went in along with #2308745: Remove rest.settings.yml, use rest_resource config entities, so closing as a duplicate.
However, now that this base class is phpunit-based, any code after a failed assertion is not executed, so we are back to it just displaying
with no further details.
Comment #7
jhedstromComment #9
jhedstromJust ran into this again over in #2716891: DateTimeIso8601::setDateTime() vs. DateTimeItem::schema(): (at least) one of them is broken.
Comment #10
jhedstromComment #11
jhedstromHonestly though, this sort of message isn't too much more helpful in terms of determining what actually needs to be updated:
Comment #12
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedRe #11, at least it quickly shows you what needs to be investigated/debugged further. The current code in HEAD forces you to debug the whole update process and extract the relevant problem (e.g. which field needs to be updated) manually.
So #7 is already a nice improvement, IMO :)
Comment #13
catchCommitted/pushed to 8.6.x and cherry-picked to 8.5.x. Thanks!