Framework for testing Symfony Emails

> This needs to work with both the @Mail plug-in from #3165762: Add symfony/mailer into core

IMHO we don't need to test the Mail plugin.

The old mail system already has a way to test, by replacing the plugin with a test implementation. Unless you're specifically testing that implementation, then you don't need to test if that specific plugin works, just that the API to build mails work, no need to change anything here.

We seem do disagree a fair bit about the purpose of the Symfony @Mail Plugin. I expect it will remain experimental and believe that we should not add any more features to it, neither HTML mails, nor deprecate PhpMail or anything else. We should focus our attention on the new mail API, there's enough to do there.

> We seem do disagree a fair bit about the purpose of the Symfony @Mail Plugin.

I feel there is confusion more than disagreement.

• I would be content for the @Mail plug-in to remain experimental, and I'm happy to reject the "deprecate PhpMail" issue.
• I do think it's worth explicitly testing this implementation, at least a single test, so the experiment doesn't get broken.
• I absolutely agree there's enough to do on the new API. As I've said (about 5 times now😃) there's more than enough. My idea is that we could develop features on the @Mail experiment so that they are ready for use on the new API. So I am focusing on the new mail API as well as I possibly can by trying to split the rather large complex problem into smaller manageable pieces.

This issue seems like a perfect example - AFAICS it doesn't add any difficulty to make these tests work in both cases (plug-in and API), because the test is based on a Symfony transport which they both support. We can't really attempt to fix this issue until there is something for it to test. If it works additionally with the @Mail plug-in, then it could be fixed right now, and be ready waiting for the new API to use it.

@znerol included test code for the @Mail plug-in in an earlier MR for 📌 [PP-1] Add symfony mailer transports to DIC Postponed . I encouraged him to split it out as the issue was already large, and instead created this issue.

Hm.

I'm trying to hold back my knee-jerk reactions a bit more. What I think is important is that every step brings us closer to replacing the old mail system, with few as few detours as possible.

I struggle to see that on the e-mail issue for example, I think there are many aspects that are specific to the old system there.

After thinking a bit more about this, I can see a way that would benefit us. What we could essentially do is deprecate \Drupal\Core\Mail\Plugin\Mail\TestMailCollector (and the html version of that) with the symfony mailer with the test transport and the corresponding state storage and plain arrays assertions with your test trait (not sure if we should extend the existing AssertMailTrait or deprecate and replace with a new version), which sounds neat. Then we could convert all core tests to use that and should have to barely touch them when we eventually convert them to the new API.

What I'm not quite sure is how exactly we'd approach that deprecation. It's test only, but as a test API, I think we still want to provide BC for it, so we'd make it opt-in I'd assume? Kernel tests for example have the test implementation baked in as a config override in \Drupal\KernelTests\KernelTestBase::bootKernel, to make sure you don't accidently override this. We could do a standardized property for kernel and functional tests to use the new API instead, and if not set, trigger a deprecation. That flag is something we'd then need to remove again. This is a step we wouldn't need to do if we'd convert the test as we convert the implementations in modules.

There aren't actually that many tests which use that trait in core. there's one in config translation, two in contact module, one that tests the email action and that's about it. So it would be quite a manageable effort in core to just convert as we switch. It would allow contrib to do the same though, simplenews alone probably has many times more cases of asserting sent mails than core as a whole.

Thoughts?

FWIW, we do have some test coverage with SymfonyMailerTest, but that's only a unit test.

I like your thinking.

I agree the test collector is an API as 1000s of lines of test code in Contrib depend on them (e.g. simplenews). All current email test code is intimately tied to $message interface so needs to be reworked. Contrib modules will likely do this at the same time as moving to the new mailer API. However maybe we could it sooner in Core that would certainly be interesting to try, and it would validate the testing framework.

Currently DSM (Drupal Symfony Mailer) contains a test framework, so I'd also aim to switch that to use what's in Core once it's available.

> I mean that AssertMailTrait is a test API, and it would be a big problem for simplenews to make a non-BC change it in a minor release. I agree that Core can changes its own tests in a minor release.

Correct, we can't change existing methods on the trait, but we can introduce a new trait, the trait could automatically trigger the discussed test setup behaviour change that would send test mails through Symfony mailer, so you don't need anything extra, just use the new trait and you're good to go.

We could also add methods on that existing trait or change the behaviour on those methods, but I think I prefer having a new trait, especially when that includes the test setup trigger.

Yes I agree, a new trait seems like the solution. It's neat that the flag you discussed is included automatically in the trait.