UpdatePostUpdateTest is extremely fragile to change and does not test batches in post updates [#2581459]

See #2520526: Calculate configuration entity dependencies on install:

Every new post update has to be added an array - making this test certain fail if you an a post update to core
Post updates support batches but it is not explicitly tested here.

Release candidate phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category	Task because only improving test coverage.
Issue priority	Normal because only making testing easier and improving explicit coverage.
Unfrozen changes	Unfrozen because it only changes tests.

Comment	File	Size	Author
#11	revert.patch	4.27 KB	catch
#11	8.0.x: PHP 5.5 & MySQL 5.5 14,223 pass PHP 5.6 & MySQL 5.5 14,223 pass PHP 7 & MySQL 5.5 14,097 pass, 42 fail
#3	2581459-3.patch	4.27 KB	alexpott
#3	8.0.x: PHP 5.5 & MySQL 5.5 14,214 pass

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

7 October 2015 at 01:44

alexpott created an issue. See original summary.

Comment #2

dawehner

German

CreditAttribution: dawehner as a volunteer commented 7 October 2015 at 07:02

For this particular issue I think it is enough to switch form assert Equal to a checking that at least these 5 update functions are in the state.

Comment #3

alexpott

he/they

English

🇪🇺🌍

CreditAttribution: alexpott at Chapter Three commented 7 October 2015 at 09:07

Issue summary:	View changes
Status:	Active	» Needs review

File	Size
2581459-3.patch	4.27 KB
8.0.x: PHP 5.5 & MySQL 5.5 14,214 pass

Comment #4

dawehner

German

CreditAttribution: dawehner as a volunteer commented 7 October 2015 at 09:44

Status:

Needs review

» Reviewed & tested by the community

Perfect!

Comment #5

yched CreditAttribution: yched commented 7 October 2015 at 10:07

THANK YOU

Comment #6

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers at Acquia commented 7 October 2015 at 13:06

+++ b/core/modules/system/src/Tests/Update/UpdatePostUpdateTest.php
@@ -49,24 +51,34 @@ public function testPostUpdate() {
-    $updates = array_merge([
-      'block_post_update_disable_blocks_with_missing_contexts',
-      'field_post_update_save_custom_storage_property',
-      'field_post_update_entity_reference_handler_setting',
-      'system_post_update_recalculate_configuration_entity_dependencies',
-      'views_post_update_update_cacheability_metadata',
-    ], $updates);

Hallelujah! I'd use emojis, but Drupal 7 doesn't support that, so using ASCII instead: kill it with fire!

Comment #7

effulgentsia CreditAttribution: effulgentsia at Acquia commented 9 October 2015 at 18:28

Issue tags:

+rc eligible

This only changes a test, so tagging "rc eligible" per https://www.drupal.org/core/d8-allowed-changes#rc.

Comment #8

9 October 2015 at 18:33

effulgentsia committed d5bd2ec on

Issue #2581459 by alexpott: UpdatePostUpdateTest is extremely fragile to...

Comment #9

effulgentsia CreditAttribution: effulgentsia at Acquia commented 9 October 2015 at 18:34

Status:

Reviewed & tested by the community

» Fixed

Pushed to 8.0.x.

Comment #10

catch

he/him

English

CreditAttribution: catch commented 19 October 2015 at 11:32

Status:

Fixed

» Active

Looks like this broke PHP 7:

https://www.drupal.org/pift-ci-job/56619

Comment #11

catch

he/him

English

CreditAttribution: catch commented 19 October 2015 at 11:35

Status:

Active

» Needs review

File	Size
revert.patch	4.27 KB
8.0.x: PHP 5.5 & MySQL 5.5 14,223 pass PHP 5.6 & MySQL 5.5 14,223 pass PHP 7 & MySQL 5.5 14,097 pass, 42 fail

Sending revert for a test run.

Comment #12

dawehner

German

CreditAttribution: dawehner as a volunteer commented 19 October 2015 at 15:23

There are certainly tests failing which are unrelated with updates in general, it could be just "random" PHP7 failures.

Comment #13

catch

he/him

English

CreditAttribution: catch commented 19 October 2015 at 15:59

Status:

Needs review

» Fixed

Yeah it's not a regression introduced here anyway: https://www.drupal.org/pift-ci-job/57019 Apologies for the false alarm.

fail: [Other] Line 59 of core/modules/system/src/Tests/Update/UpdatePostUpdateTest.php:
Value array (
  0 => 'update_test_postupdate_post_update_first',
  1 => 'update_test_postupdate_post_update_second',
  2 => 'update_test_postupdate_post_update_test1',
  3 => 'update_test_postupdate_post_update_test0',
) is identical to value array (
  0 => 'update_test_postupdate_post_update_test0',
  1 => 'update_test_postupdate_post_update_test1',
  2 => 'update_test_postupdate_post_update_second',
  3 => 'update_test_postupdate_post_update_first',
).

fail: [Other] Line 69 of core/modules/system/src/Tests/Update/UpdatePostUpdateTest.php:
Value array (
  0 => 'block_post_update_disable_blocks_with_missing_contexts',
  1 => 'field_post_update_save_custom_storage_property',
  2 => 'field_post_update_entity_reference_handler_setting',
  3 => 'system_post_update_recalculate_configuration_entity_dependencies',
  4 => 'views_post_update_update_cacheability_metadata',
  5 => 'update_test_postupdate_post_update_first',
  6 => 'update_test_postupdate_post_update_second',
  7 => 'update_test_postupdate_post_update_test1',
  8 => 'update_test_postupdate_post_update_test0',
) is equal to value array (
  0 => 'block_post_update_disable_blocks_with_missing_contexts',
  1 => 'field_post_update_entity_reference_handler_setting',
  2 => 'field_post_update_save_custom_storage_property',
  3 => 'system_post_update_recalculate_configuration_entity_dependencies',
  4 => 'views_post_update_update_cacheability_metadata',
  5 => 'update_test_postupdate_post_update_test0',
  6 => 'update_test_postupdate_post_update_test1',
  7 => 'update_test_postupdate_post_update_second',
  8 => 'update_test_postupdate_post_update_first',
).