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/5...@github.com>
_______________________________________________
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev
