Remove usage of forum module from tests

Still need to investigate core/modules/system/tests/modules/url_alter_test/url_alter_test.install

This function,

function url_alter_test_install() {
  // Set the weight of this module to one higher than forum.module.
  module_set_weight('url_alter_test', 2);
}

was added in #320331-69: Turn custom_url_rewrite_inbound and custom_url_rewrite_outbound into hooks → . This is executed in \Drupal\Tests\path_alias\Functional\UrlAlterFunctionalTest and forum is no longer installed in the test. If the hook is deleted the test still passes. So, is this really unneeded code or is the test not doing what it should be. I don't know enough about this area of core to say.

And, because of the use of forum this test was copied to the forum module, \Drupal\Tests\forum\Functional\UrlAlterFunctionalTest with a new test module, forum_url_alter_test. In that test module the hook is absent So, perhaps this really is dead code.

I did the following:

git log -S "forum" core/modules/path_alias/tests/src/Functional/UrlAlterFunctionalTest.php

git show 8a924f778d5 core/modules/path_alias/tests/src/Functional/UrlAlterFunctionalTest.php

And the answer is - we took the forum bits of the test out in a previous patch, and this bit got missed.

If the forum tests pass without this hook_install() too, I think it's fine to remove as dead code. It was added a very, very long time ago.

Tried following the best I could

I commented out url_alter_test_install. Both UrlAlterFunctionalTest (path_alias) and UrlAlterFunctionalTest (forum) still pass.

Yep I think we can just remove it.

I removed the file as the hook was the only thing it contained.

Know I posted in slack leaving tickets in review longer. But based on the importance of this to get Forum deprecated by D11 going to go ahead and mark.

Just documenting my own results while I review.

Before

[ayrton:drupal | Sun 10:33:07] $ grep -ri "forum" * |  grep -v "node_modules" | grep -v "vendor" | grep -i "tests" | grep -vi "migrat" | grep -v "core/modules/forum" | grep -v "fixtures"
core/tests/Drupal/KernelTests/Config/Schema/MappingTest.php:      case 'field.field.node.forum.comment_forum':
core/tests/Drupal/KernelTests/Config/Schema/MappingTest.php:        $this->enableModules(['field', 'node', 'comment', 'taxonomy', 'forum']);
core/tests/Drupal/KernelTests/Config/Schema/MappingTest.php:        $this->assertNull(FieldConfig::load('node.forum.comment_forum'));
core/tests/Drupal/KernelTests/Config/Schema/MappingTest.php:        $this->installConfig(['forum']);
core/tests/Drupal/KernelTests/Config/Schema/MappingTest.php:        $this->assertNotNull(FieldConfig::load('node.forum.comment_forum'));
core/tests/Drupal/KernelTests/Config/Schema/MappingTest.php:    yield 'Dynamic type with [%parent.%parent]: field.field.node.forum.comment_forum:default_value.0' => [
core/tests/Drupal/KernelTests/Config/Schema/MappingTest.php:      'field.field.node.forum.comment_forum',
core/modules/system/tests/modules/url_alter_test/url_alter_test.install:  // Set the weight of this module to one higher than forum.module.
core/modules/system/tests/src/Functional/Update/Y2038TimestampUpdateTest.php:  protected static $modules = ['forum'];
core/modules/system/tests/src/Functional/Update/Y2038TimestampUpdateTest.php:    'forum_index' => ['created', 'last_comment_timestamp'],
core/modules/system/tests/src/Functional/Update/Y2038TimestampUpdateTest.php:      // enabled modules: forum, language, locale, statistics and tracker.

After

[ayrton:drupal | Sun 10:35:06] $ grep -ri "forum" * |  grep -v "node_modules" | grep -v "vendor" | grep -i "tests" | grep -vi "migrat" | grep -v "core/modules/forum" | grep -v "fixtures"
core/modules/system/tests/src/Functional/Update/Y2038TimestampUpdateTest.php:  protected static $modules = ['forum'];
core/modules/system/tests/src/Functional/Update/Y2038TimestampUpdateTest.php:    'forum_index' => ['created', 'last_comment_timestamp'],
core/modules/system/tests/src/Functional/Update/Y2038TimestampUpdateTest.php:      // enabled modules: forum, language, locale, statistics and tracker.

Per the IS:

core/modules/system/tests/src/Functional/Update/Y2038TimestampUpdateTest.php - no change, An update test using forum that will be removed in 11.0

1. Is there an issue for this? If not, we should file it parented to the D11 beta blocker meta or one of its children. (And add to related issues here.)
2. This is out of scope, but: Do we have other tests of 2038 upgrade paths? I guess the idea is that people must upgrade to 10.3/10.4 first regardless, which would already require their timestamps to be updated, and so it's not needed in D11. But it seems weird to have the test of this coupled only to Forum.

I diffed the Forum and new test fixure config:

[ayrton:drupal | Sun 10:49:13] $ diff -uP core/modules/forum/config/optional/field.field.node.forum.comment_forum.yml core/modules/system/tests/modules/config_mapping_test/config/optional/field.field.node.config_mapping_test.comment_config_mapping_test.yml
--- core/modules/forum/config/optional/field.field.node.forum.comment_forum.yml2023-09-25 18:03:35
+++ core/modules/system/tests/modules/config_mapping_test/config/optional/field.field.node.config_mapping_test.comment_config_mapping_test.yml	2024-01-14 10:44:22
@@ -2,14 +2,14 @@
 status: true
 dependencies:
   config:
-    - field.storage.node.comment_forum
-    - node.type.forum
+    - field.storage.node.comment_config_mapping_test
+    - node.type.config_mapping_test
   module:
     - comment
-id: node.forum.comment_forum
-field_name: comment_forum
+id: node.config_mapping_test.comment_config_mapping_test
+field_name: comment_config_mapping_test
 entity_type: node
-bundle: forum
+bundle: config_mapping_test
 label: Comments
 description: ''
 required: true

[ayrton:drupal | Sun 10:50:43] $ diff -uP core/modules/forum/config/optional/field.storage.node.comment_forum.yml core/modules/system/tests/modules/config_mapping_test/config/optional/field.storage.node.comment_config_mapping_test.yml
--- core/modules/forum/config/optional/field.storage.node.comment_forum.yml	2023-01-16 08:10:14
+++ core/modules/system/tests/modules/config_mapping_test/config/optional/field.storage.node.comment_config_mapping_test.yml	2024-01-14 10:44:22
@@ -4,12 +4,12 @@
   module:
     - comment
     - node
-id: node.comment_forum
-field_name: comment_forum
+id: node.comment_config_mapping_test
+field_name: comment_config_mapping_test
 entity_type: node
 type: comment
 settings:
-  comment_type: comment_forum
+  comment_type: comment_config_mapping_test
 module: comment
 locked: false
 cardinality: 1

[ayrton:drupal | Sun 13:14:50] $ diff -uP core/modules/forum/config/optional/node.type.forum.yml core/modules/system/tests/modules/config_mapping_test/config/optional/node.type.config_mapping_test.yml
--- core/modules/forum/config/optional/node.type.forum.yml	2024-01-14 10:14:20
+++ core/modules/system/tests/modules/config_mapping_test/config/optional/node.type.config_mapping_test.yml	2024-01-14 10:44:22
@@ -3,10 +3,10 @@
 dependencies:
   enforced:
     module:
-      - forum
-name: 'Forum topic'
-type: forum
-description: 'A <em>forum topic</em> starts a new discussion thread within a forum.'
+      - config_mapping_test
+name: 'config_mapping_test topic'
+type: config_mapping_test
+description: 'A <em>config_mapping_test topic</em> starts a new discussion thread within a config_mapping_test.'
 help: null
 new_revision: false
 preview_mode: 1

[ayrton:drupal | Sun 13:17:26] $ diff -uP core/modules/forum/forum.info.yml core/modules/system/tests/modules/config_mapping_test/config_mapping_test.info.yml
--- core/modules/forum/forum.info.yml	2023-09-25 18:03:35
+++ core/modules/system/tests/modules/config_mapping_test/config_mapping_test.info.yml	2024-01-14 10:44:22
@@ -1,12 +1,8 @@
-name: Forum
+name: Config mapping test
 type: module
-description: 'Provides discussion forums.'
+description: 'Test configuration mapping.'
 dependencies:
   - drupal:node
-  - drupal:history
-  - drupal:taxonomy
   - drupal:comment
-  - drupal:options
 package: Core
 version: VERSION
-configure: forum.overview

I reviewed all the uses of url_alter_test in core and agree it can be removed. The fiddling with Forum's weight was added in #320331-69: Turn custom_url_rewrite_inbound and custom_url_rewrite_outbound into hooks → , as in, well before the release of D7, when everything to do with URLs, routing, and module weights/dependencies was wildly different. That comment reads:

Ok this one is almost ready. Still trying to figure out what's going wrong with the wonky forum tests...

Some other quotes from that issue:

In IRC, Dave and I determined the source of the badness is that calling module_list(TRUE, TRUE) to refresh the module list from inside a reduced bootstrap level (we're still just at DRUPAL_BOOTSTRAP_PATH when drupal_path_initialize() runs) results in sheer badness. entity_get_info() is confused, we have an empty list of modules (or something), and we end up caching an empty array in {cache}.cid == 'entity_info', resulting in a completely broken site until you manually clear that cache entry. :(

So:

A) WTF, module_list()? Why can't you be refreshed twice during the bootstrap process?

B) WTF, entity_get_info()? Why are you willing to cache an empty array into the DB? ;)

For now, this patch is going to work-around it by calling module_list(FALSE, TRUE) to only get bootstrap modules, but *not* refresh the list.

Forum.module is very broken. Very upset and frustrated.

And to properly compare old vs new, make sure we are using worst-case scenario. Locale.module enabled and altering language prefixes (a non-default language path), and also using taxonomy_term_path() with forum.module.

So overall I think this is ready, but NW for the followup I mentioned in #12.1.

@xjm, thanks for the review.

#12.1. The followup issue for that is 📌 Remove deprecated modules from update testing database dumps etc. Active . It is already a related issue but it can't be a child because the plan is to do the dumps once for all the deprecated modules.

I am setting back to RTBC because I think that followup was the only item.

For #12.1 there is also 📌 [policy] Remove update hooks added until 10.3 from Drupal 11 RTBC for removing updates prior to 10.3.0, so we're covered whether updates or forum module itself get removed first.

#12.2 there's explicit test coverage for field items and 2038+ timstamps being added in 🐛 Timestamp field items are affected by 2038 bug Needs work . We've only fixed the one-off schema entries around core so far, some work is still outstanding on the 2038 bug.

Committed/pushed to 11.x, thanks!

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

quietone → closed merge request !6101