[openstreetmap/openstreetmap-website] Make email-related testing more reliable (Issue #5687)

[Andy Allan]

gravitystorm created an issue (openstreetmap/openstreetmap-website#5687)

In the last few days I've seen a lot of test suite failures with messages similar to:

```
Failure:
UserSignupTest#test_Sign_up_with_confirmation_email [test/system/user_signup_test.rb:37]:
expected to find text "new_user_account" in "OpenStreetMap\nEdit\nHistory\nExport\nGPS Traces\nUser Diaries\nCommunities\nCopyright\nHelp\nAbout\nLog In\nSign Up\nThat confirmation code has expired or does not exist.\nCheck your email!\nWe sent you a confirmation email. Confirm your account by clicking on the link in the email and you'll be able to start mapping.\nIf you need us to resend the confirmation email, click the button below."

bin/rails test test/system/user_signup_test.rb:10
```

I think the root cause is that we're not clearing the ActionMailer::Deliveries queue consistently enough, and that there are several tests that don't do this properly. Later, if a test is running `ActionMailer::Base.deliveries.first`, it is running the risk that it picks up the wrong email, depending on what order the tests are run.

For tests inheriting from `ActionMailer::TestCase` and `ActionDispatch::IntegrationTest`, the test suite clears the queue automatically before and after each test. For our `system`, `controller` and `job` tests we need to do it manually, and this can be forgotten.

My proposal is to create a test helper ([similar to upstream](https://github.com/rails/rails/blob/8e504b017a68981076746c2d19fd23c788e4293e/actionmailer/lib/action_mailer/test_case.rb#L16-L30)) and include that automatically for all of our system, controller and job tests. I can't think of a good reason not to - it's slightly unnecessary in most tests, but it helps new developers by just taking care of things automatically. 

Additionally, we could standardise on whether we assert against the `first` or `last` email in the queue - using `last` would possibly have avoided us noticing this, but even if we fix it properly it's reasonable to be consistent.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/issues/5687
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/issues/5687 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20250216/be26cf90/attachment-0001.htm>