[policy, no patch] How to ensure migrations continue to work when modules are moved to contrib

Thanks @quietone!

Moving back a step to the deprecation stage, because the contrib module should include whatever correct solution for the migrations in order to be marked stable, which is generally needed before we deprecate the module in D9.

Rewording the title a bit to make sure it's only talking about migrations and not hook_update_N().

What changes should be made to the test fixtures; leave as is, remove the module and all the related data, or change to uninstalled? For example, right now the fixture has not changed and forum nodes and fields are migrated and tested.

I think it would be OK to leave the migration test fixtures as-is in core for the 'new' contrib migrations to use for test coverage. We include migrations in core for Drupal 6/7 contrib modules for things that moved into core, and we know we're eventually going to remove the Drupal 6 and 7 migrations from core (probably in Drupal 11), at which point the fixtures will go too. Extricating the fixtures would be a lot of work for something we'll eventually delete anyway.

What to do when the removal changes migration yml files outside of the module namespace? For example, to move forum to contrib forum related processes are being removed from the block migrations.

Is there a standard way for contrib modules to migrate block settings? If so forum module would need to do that. If not, then if contrib modules aren't migrating block config, it seems OK for forum not to either - it's just a setting in this case.

However, something like d7_taxonomy_term_entity_translation where it looks at forum containers is a much bigger problem. If there's no way to move that sort of logic to the contrib module, then are we able to keep the logic in place without forum module in core? I think it would be OK to remove the module but continue to support aspects of it in miscellaneous migrations where it's hard-coded - since again we know we're going to remove these from core altogether in Drupal 11.

Forum is a special snowflake too. Because it uses terms in D7 and it needs to re-use the vocabulary and fields installed by forum module in D9 (which is named differently in D9 then in D6/D7) and depends upon terms being created in that vocabulary during the migration. See forum_vocabulary/ForumVocabulary in taxonomy.module as just one example.

I guess what I'm saying is that we might make some decisions for some modules. Then for forum, we might need to make a one-off decision. And that might be OK.

We discussed this issue at #3263731: [meeting] Migrate Meeting 2022-02-17 1400Z. That issue will have a transcript of the meeting.

I think we have consensus:

• If possible, make no changes to the tests.
• Keep support for the deprecated modules in migrations, process plugins, etc.

A couple of points that I made:

If I have a D6 site that uses the Forum module, and I migrate to D10 with the contrib Forum module, then I will be happy that it is still supported.

We already have precedent for including in core some support for contrib modules. For example, the filter_id process plugin (Drupal\filter\Plugin\migrate\process\FilterID::getSourceFilterType()) handles filter formats from several contrib modules.

If it is possible, then I think we should choose the first option from the issue summary (Create a way to keep the upgrade path working and implement that).

Is there anything missing, not covered?

I do not notice anything. I will not be surprised if you come up with more questions as you continue to work on fixing the migrations.

I am making a couple of small corrections to the text.

While I was in proofreading mode, I noticed that FilterID should be changed to FilterId. I am too tired to create an issue for that.

Keep all the assertions in core tests related to modules that move to contrib.

This will be interesting to see how it works in practice. The migration requirements will fail if the contrib module doesn't exist and that migration won't execute. I think in practice, we'll need to move all tests to contrib as well.

I think I agree with everything here. I'll add a few comments:

The drupal 6 and drupal 7 test fixtures stay the same.

Yes, the test fixtures should not be trimmed down. Firstly it's a lot of work to save a few kb of file size, More importantly, the fixtures represent a 'standard' D7 installation containing data from D7 core modules, plus d7 modules which have ended up in core. It's important to leave data from say the forum module in that fixture, because we should test the core upgrades on a site that had that module enabled to make sure they are doing what we want them to do when there is forum data in the database.

Copy the test fixtures to the module so they remain independent of any core changes to the fixture.

I'm on the fence on this one. Copying them to the module would make the tests consistent and deterministic, but using the core fixtures might catch bugs if we add something from another module that causes a conflict. At the end of the day, if the tests are counting entities, those may change in core, so copying over is probably best.

Keep all the assertions in core tests related to modules that move to contrib as much as possible.

If we are counting entities, the core tests should be counting what gets migrated when the contrib module is not there, whatever it is. Forum is a special snowflake. I haven't looked but off the top of my head we might end up with nodes and comments migrated but no forum data migrated, or did we go out of the way to not migrate the nodes from forum unless the contrib module is installed? Either way, keeping the forum data in the fixture ensures that whatever we intend to happen when migrating a d7 site with forum enabled into a d8 site without it.

Migration yml and processes that are in the module move with the module to contrib.

Yes, the contrib module should be responsible for its own migrations, again, forum is a special snowflake.

Migration yml and processes that are outside the module but support the upgrade path for that module stays in core. There is precedent for this in the \Drupal\filter\Plugin\migrate\process\FilterID process plugin, which supports contrib modules. (Thanks to benjifisher for pointing that out).

No real reason to remove process plugins that live outside the module unless there is some other motivation for deprecating them.

The functional tests in migrate_drupal_ui will need to change.

Yes, and this will be caught at least by the PR that removes the module.