Problem/Motivation

See https://www.drupal.org/pift-ci-job/1436032

One of the initial test failures since 9.0.x has been branched.

#slow.Drupal\Tests\system\Functional\Update\UpdatePathTestBaseFilledTest
✗	
Drupal\Tests\system\Functional\Update\UpdatePathTestBaseFilledTest
fail: [Other] Line 0 of sites/default/files/simpletest/phpunit-3.xml:
PHPunit Test failed to complete; Error: PHPUnit 6.5.14 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\system\Functional\Update\UpdatePathTestBaseFilledTest
..F...                                                              6 / 6 (100%)

Time: 8.06 minutes, Memory: 4.00MB

There was 1 failure:

1) Drupal\Tests\system\Functional\Update\UpdatePathTestBaseFilledTest::testUpdateHookN
Failed asserting that 8000 matches expected 8001.

/var/www/html/core/tests/Drupal/TestTools/PhpUnitCompatibility/PhpUnit6/TestCompatibilityTrait.php:51
/var/www/html/core/tests/Drupal/KernelTests/AssertLegacyTrait.php:37
/var/www/html/core/tests/Drupal/FunctionalTests/Update/UpdatePathTestBaseTest.php:103

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#10 3087991.patch658 bytescatch

Comments

catch created an issue. See original summary.

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
commented
Parent issue: » #3087874: [meta] Make 9.0.x branch pass testbot
jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Related issues: +#3087644: Remove Drupal 8 updates up to and including 88**

#3087644: Remove Drupal 8 updates up to and including 88** is battling with this one as well. I have added new D9 DB dump over there which is taken after adding hook_update_last_removed so every updated module has a different version now.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +Drupal 9

@jibran Does that mean this should be blocked on #3087644: Remove Drupal 8 updates up to and including 88**? Or is this a duplicate? Or is there a logical subset that we can commit here to 9.0.x to unbreak D9 automated testing?

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented

The test is failing independently to #3087644: Remove Drupal 8 updates up to and including 88**, I think fixing it here makes sense. DB dump updates can happen over there but the test should be fixed here first.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

I looked at this for 10 minutes and had no idea where to start! 🤓😊

It sounds like you understand this much better. Could you roll a patch for this maybe? 😇

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Assigned: Unassigned » jibran

On it!

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Assigned: jibran » Unassigned
Status: Active » Closed (duplicate)

On second thought let fix it over in #3087644: Remove Drupal 8 updates up to and including 88**. Schema version is outdated as is when D9 installed hook_update_last_removed fixes that over there.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Closed (duplicate) » Active

I'm not sure we should close this just yet, I'd like to understand it better first.

@alexpott and @catch also had a discussion on slack today on what exactly the expected behavior is, which update functions should run and what shouldn't, also in regards to contrib modules.

If you're an 8.x-1.x module that is compatible with 8.8+ and 9.0+, how should you number these things? There is no right moment to switch to 900x.

catch’s picture

catch
he/him
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new 3087991.patch658 bytes

This should fix it, patch is self-explanatory.

(by 'fix it' I mean bring 6 fails down to 5, we have some unrelated ones in HEAD too)

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs review » Reviewed & tested by the community

Verified locally, passes just fine. Also fixed fails in #3087644: Remove Drupal 8 updates up to and including 88**.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented 
--- a/core/modules/system/tests/modules/update_test_semver_update_n/update_test_semver_update_n.info.yml
+++ b/core/modules/system/tests/modules/update_test_semver_update_n/update_test_semver_update_n.info.yml

@@ -3,4 +3,4 @@ type: module
-core_version_requirement: ^8
+core_version_requirement: ^9

I was wondering if we could change this to '*' originally I thought "I guess not as the point is to test the core semver value" but I'm not so sure looking at the test code.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 10: 3087991.patch, failed testing. View results

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs work » Reviewed & tested by the community

We are down two fails so restoring RTBC.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

So let's do this for now and decide on #12 in a follow-up. Created #3088134: Major version dependent tests mean lots of work to get a new major branch started

Committed 3bd108b and pushed to 9.0.x. Thanks!

  • alexpott committed 3bd108b on 9.0.x 
    Issue #3087991 by catch, jibran: Drupal\Tests\system\Functional\Update\...

Status: Fixed » Closed (fixed)

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