Remove core updates added prior to 9.3.0 and adjust test coverage

it will affect removal of views helpers in #3261245: Remove deprecated views module functions

Looking at related in #2 I bet we need to clean-up post updates as well

So the first testrun, starting at 12:45 CET (https://dispatcher.drupalci.org/job/drupal_patches/111933/) is _still_ on the PHPStan check. In fact, all testruns are busy PHPStanning.

Unsure what's going on there, but that seems a tad on the long side...

There are a couple of omissions that I can see. The remaining post updates are:

core/modules/media/media.post_update.php:function media_post_update_modify_base_field_author_override() {
core/modules/node/node.post_update.php:function node_post_update_modify_base_field_author_override() {
core/modules/system/system.post_update.php:function system_post_update_enable_provider_database_driver() {
core/modules/views/views.post_update.php:function views_post_update_provide_revision_table_relationship() {
core/modules/workspaces/workspaces.post_update.php:function workspaces_post_update_modify_base_field_author_override() {
core/modules/workspaces/workspaces.post_update.php:function workspaces_post_update_remove_association_schema_data() {

However workspaces_post_update_remove_association_schema_data() was added way back in #3098427: Manipulating the revision metadata keys before running the BC layer breaks the BC layer so it is time for it to be removed.

Also, the remaining update hooks are:

core/modules/jsonapi/jsonapi.install:function jsonapi_update_9401() {
core/modules/migrate_drupal/migrate_drupal.install:function migrate_drupal_update_8901(&$sandbox) {

migrate_drupal_update_8901() should be removed here.

Otherwise the updated tests including database dumps look OK to me.

Sorry that's what I meant in #11 - I was listing all the remaining updates and only meant that the two I highlighted should be removed.

I just did a very close review on the (giant) MR diff. The only thing I spotted was where this is going to conflict with #3261629: Database dumps are no longer driver-agnostic and need a quick reroll once that lands. Otherwise, this looks great and is IMHO RTBC. I'm not sure if I should move this to Postponed (since that's accurate) or leave it "Needs review" (since more eyes are always good). However, since both @longwave and I have done a thorough on this, probably best to spend available review efforts elsewhere. So I'm going for postponed for now. It'll have a brief period of needs work for the trivial reroll, then it should be RTBC.

Thanks!
-Derek

I've reread the entire patch and confirmed that:

- all hook_update_N() are correctly removed and hook_update_last_removed() updated to match.
- all post_update hooks are correctly removed and hook_removed_post_updates() updated to match.
- all relevant tests have been removed along with their fixtures.

One thing I did spot is that migrate_drupal_multilingual is hidden and deprecated, but isn't flagged in info.yml with the new method of deprecating modules; if we don't have an issue to fix this and remove in 10.0.x we should add one.

edit: could not find an issue, so opened #3261957: Properly deprecate migrate_drupal_multilingual for future removal

Reroll looks good. Agreed, RTBC++.

Thanks!
-Derek

Committed 4263042 and pushed to 10.0.x. Thanks!

Does it make sense to add drupal-9.3.0.bare.standard.php.gz and drupal-9.3.0.filled.standard.php.gz to Drupal 9.3 and 9.4?

contrib modules with update tests that try to add Drupal 10 support can reference those files, instead of adding a check if the current Drupal version is Drupal 10, usedrupal-9.3.0.bare.standard.php.gz. Else, use drupal-9.0.0.bare.standard.php.gz

@JeroenT that does make sense - do you want to file an issue to do that?

Created #3263886: Copy drupal-9.3.0.bare.standard.php.gz and drupal-9.3.0.filled.standard.php.gz from the Drupal 10 branch

No change notice for removing the fixtures?

My contributed module uses drupal-8.8.0.bare.standard.php.gz to test hook_update_N() for updates from D8 to D9. These tests have been running green on a weekly basis for 18 months now. But these tests started failing when this issue was committed. That's the only notice I got of the change - there is no change record.

What is the recommended course of action for contributed modules? Should we no longer test for the D8->D9 upgrade? Or, do we now need a separate branch for D10 support, because test code in the current branch won't work anymore in both D9 and D10?

This is the purpose of a change notice - if you remove something from core, there should be a change notice as well as an explanation about what contrib should do to deal with the change.

@catch thanks for https://www.drupal.org/node/3264237. I think #25 answers @tr's question about updating from D8 to D9 - ie. Drupal 8 is not supported anymore so you should not expect Drupal 8 upgrade tests to work on Drupal 10. If you want to keep the Drupal 8 to 9 test running and have a module that claim D10 compatibility then you could test for the existence of the Drupal 8 db dump in the ::setUp method and mark the test skipped if it is not there... you could probably this in the \Drupal\FunctionalTests\Update\UpdatePathTestBase::setDatabaseDumpFiles() too - as that might be even simpler. Depends how you've organised your test.

For reference, I fixed this with conditional code in setDatabaseDumpFiles(), which is the same thing I had done for this module in the D8.8->D9 transition when the old (pre-D8.8) fixtures were removed from core.

For me, this is not a matter of supporting D8, it's a matter of allowing / enabling people to use D9. That transition is still going to be happening for years so I want to keep my tests that validate the upgrade process.

Leaving aside the 8.8.x fixture, I just realized that the drupal-9.0.0.bare.standard.php.gz fixture, also removed here, lived in core for a total of 7 days. The full git history on it is just:

commit 42630422b9f2687408cd5d33c4dfb590ebb26491
Author: Alex Pott <alex.a.pott@googlemail.com>
Date:   Tue Feb 8 12:16:18 2022 +0000

    Issue #3261486 by catch, longwave, dww: Remove core updates added prior to 9.3.0 and adjust test coverage

commit ff8c31d2618ca2ee209c2ad3eb2a9af29790d45a
Author: catch <catch@35733.no-reply.drupal.org>
Date:   Mon Feb 1 15:49:03 2021 +0000

    Issue #3194562 by alexpott, clayfreeman: Add database dumps for 9.0.0

Removing this fixture makes life more difficult for bug fixes like #85494-386: Use email verification when changing user email addresses. Sadly, there's no single fixture that the upgrade path tests can use on both 9.4.x and 10.0.x branches. 😢 It means we'll need a separate branch and MR for that issue (and probably many others), or to rely on the hack of handling 9.4.x via the MR and posting patches to the issue for 10.0.x. If #3194562: Add database dumps for 9.0.0 was worth doing (which is great), we should leave that in core longer than a week. More like a couple of years. 😉

Should we revert and reopen this issue?

Should I open a new issue about restoring drupal-9.0.0.bare.standard.php.gz?

Thanks/sorry, -Derek

Hah! In Slack, @xjm pointed out I had pandemic timewarp brain and those 2 dates are 1 year 1 week apart. 😆 Tee hee...

Still, it's a pain there's no fixture in both 9.4.x and 10.0.x branches we can use right now. So here's a "backport" that only adds that fixture. I don't know if that's okay, or if we also need to backport some upgrade test changes to use it, too?

Although @xjm suggested I reopen this issue for the backport, I'm worried that "Remove core updates added prior to 9.3.0 and adjust test coverage" will be a terribly confusing git commit message for this. Should I move this patch to a new child issue so the history on it can be clear and accurate? Or will committers manually update the commit message?