Ensure functional tests use the test mail collector

+++ b/core/modules/migrate_drupal_ui/tests/src/Functional/MigrateUpgradeExecuteTestBase.php
@@ -57,4 +58,17 @@ public function doUpgradeAndIncremental() {
+   */
+  public function useTestMailCollector() {
+    $store = \Drupal::service('tempstore.private')->get('migrate_drupal_ui');
+    $migration_array = $store->get('migrations');
+    // Only d7_system_mail changes the mail configuration.
+    if (isset($migration_array['d7_system_mail'])) {
+      unset($migration_array['d7_system_mail']);
+    }
+    $store->set('migrations', $migration_array);
+  }

Alternative idea: Use settings.php config overrides to ensure the test mail handler is set, so even *if* config is changed and we'd want to change that, we would not actually use that configuration?

That is how \Drupal\KernelTests\KernelTestBase::bootKernel() for example works, but it only puts it into $GLOBALS. For functional tests, we need to write it out into the generated settings.php.

I'm doing that with redis in https://git.drupalcode.org/project/redis/-/blob/8.x-1.x/tests/src/Functi... for example.

+++ b/core/modules/migrate_drupal_ui/tests/src/Functional/d7/Upgrade7Test.php
@@ -228,6 +228,11 @@ public function testUpgradeAndIncremental() {
 
     $this->assertFollowUpMigrationResults();
+
+    // Confirm one user activation email was sent.
+    $captured_emails = \Drupal::state()->get('system.test_mail_collector', []);
+    $this->assertCount(1, $captured_emails);
+    $this->assertEquals('user_status_activated', $captured_emails[0]['id']);
   }

I suppose you could question whether a migration should send such a mail at all, I've had problems with that too in the past where I had to disable e.g. content notifications.

Could imagine a migrate_mail_alter() that prevents all mails from being sent, configurable maybe.

But it's actually useful in this case to make sure that we _do_ collect mails. Maybe we could be even more explicit, also in the D6 test to make sure that the runtime config really uses the correct mail configuration. Although that wouldn't catch the config during web requests either.

No, that only works in-memory in the current process, not in web requests.

I'm talking about the stuff around line 90 in the redis test, where the override is written out into settings.php of that test.

No strong feeling on which option is better, the advantage of that would be that you don't need to make runtime changes to what is being migrated.

Hm, that's an interesting fail. So suddenly we _are_ catching that a mail was sent and before we weren't?

The previous logic only specifically removed the d7 migration, are you sure there's no d6 equivalent that we are now actually catching too?

+++ b/core/modules/migrate_drupal_ui/tests/src/Functional/MigrateUpgradeTestBase.php
@@ -358,4 +358,14 @@ protected function assertFileMigrations() {
+  protected function assertEmailsSent() {
+    // There should be on user activation email.

nitpick: onE user activation email.

that is kinda weird though. If there is no migration for that setting then why did it still try to send a real mail before? I tried to look a bit, but I don't know those tests or the migrations too well

And rerunning #5 here now fails as well, so the pass in #5 must have been some kind of testbot hiccup. Which is slightly bothersome, but not relevant to this patch..

Back to RTBC per #23 after #3255836: Test fails due to Composer 2.2 solved the unrelated test failure.

+++ b/core/modules/migrate_drupal_ui/tests/src/Functional/MigrateUpgradeExecuteTestBase.php
@@ -57,4 +59,16 @@ public function doUpgradeAndIncremental() {
+  /**
+   * Helper to set the test mail collector in settings.php.
+   */
+  public function useTestMailCollector() {
+    $filename = $this->siteDirectory . '/settings.php';
+    chmod($filename, 0666);
+    $contents = file_get_contents($filename);
+    $contents .= "\n" . '$config["system.mail"]["interface"]["default"] = "test_mail_collector";';
+    file_put_contents($filename, $contents);
+    OpCodeCache::invalidate(DRUPAL_ROOT . '/' . $filename);
+  }

This should do:

    // Set up an override.
    $settings['config']['system.mail']['interface']['default'] = (object) [
      'value' => 'test_mail_collector',
      'required' => TRUE,
    ];
    $this->writeSettings($settings);

instead... less complexity and relying on the existing API in tests to do this.

I must confess I didn't know about that one.

Committed and pushed 4f043a2ecc9 to 10.0.x and f7c9202be9d to 9.4.x and ee025067d5c to 9.3.x. Thanks!