<span style="color: transparent; display: none; height: 0; max-height: 0; max-width: 0; opacity: 0; overflow: hidden; mso-hide: all; visibility: hidden; width: 0;">
  <p dir="auto">In the last few days I've seen a lot of test suite failures with messages similar to:</p>
<pre class="notranslate"><code class="notranslate">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
</code></pre>
<p dir="auto">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 <code class="notranslate">ActionMailer::Base.deliveries.first</code>, it is running the risk that it picks up the wrong email, depending on what order the tests are run.</p>
<p dir="auto">For tests inheriting from <code class="notranslate">ActionMailer::TestCase</code> and <code class="notranslate">ActionDispatch::IntegrationTest</code>, the test suite clears the queue automatically before and after each test. For our <code class="notranslate">system</code>, <code class="notranslate">controller</code> and <code class="notranslate">job</code> tests we need to do it manually, and this can be forgotten.</p>
<p dir="auto">My proposal is to create a test helper (<a href="https://github.com/rails/rails/blob/8e504b017a68981076746c2d19fd23c788e4293e/actionmailer/lib/action_mailer/test_case.rb#L16-L30">similar to upstream</a>) 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.</p>
<p dir="auto">Additionally, we could standardise on whether we assert against the <code class="notranslate">first</code> or <code class="notranslate">last</code> email in the queue - using <code class="notranslate">last</code> would possibly have avoided us noticing this, but even if we fix it properly it's reasonable to be consistent.</p><p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />Reply to this email directly, <a href="https://github.com/openstreetmap/openstreetmap-website/issues/5687">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AAK2OLPTCLOSACS43JOBJRT2QB3HFAVCNFSM6AAAAABXHQEOIKVHI2DSMVQWIX3LMV43ASLTON2WKOZSHA2TMMBVGIYTOOI">unsubscribe</a>.<br />You are receiving this because you are subscribed to this thread.<img src="https://github.com/notifications/beacon/AAK2OLKHXA5GGAD2P5N2SC32QB3HFA5CNFSM6AAAAABXHQEOIKWGG33NNVSW45C7OR4XAZNFJFZXG5LFVJRW63LNMVXHIX3JMTHKUO7F2M.gif" height="1" width="1" alt="" /><span style="color: transparent; font-size: 0; display: none; visibility: hidden; overflow: hidden; opacity: 0; width: 0; height: 0; max-width: 0; max-height: 0; mso-hide: all">Message ID: <span><openstreetmap/openstreetmap-website/issues/5687</span><span>@</span><span>github</span><span>.</span><span>com></span></span></p>
</span>


<div style="display: flex; flex-wrap: wrap; white-space: pre-wrap; align-items: center; "><img alt="gravitystorm" height="20" width="20" style="border-radius:50%; margin-right: 4px;" decoding="async" src="https://avatars.githubusercontent.com/u/360803?s=20&v=4" /><strong>gravitystorm</strong> created an issue <a href="https://github.com/openstreetmap/openstreetmap-website/issues/5687">(openstreetmap/openstreetmap-website#5687)</a></div>
<p dir="auto">In the last few days I've seen a lot of test suite failures with messages similar to:</p>
<pre class="notranslate"><code class="notranslate">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
</code></pre>
<p dir="auto">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 <code class="notranslate">ActionMailer::Base.deliveries.first</code>, it is running the risk that it picks up the wrong email, depending on what order the tests are run.</p>
<p dir="auto">For tests inheriting from <code class="notranslate">ActionMailer::TestCase</code> and <code class="notranslate">ActionDispatch::IntegrationTest</code>, the test suite clears the queue automatically before and after each test. For our <code class="notranslate">system</code>, <code class="notranslate">controller</code> and <code class="notranslate">job</code> tests we need to do it manually, and this can be forgotten.</p>
<p dir="auto">My proposal is to create a test helper (<a href="https://github.com/rails/rails/blob/8e504b017a68981076746c2d19fd23c788e4293e/actionmailer/lib/action_mailer/test_case.rb#L16-L30">similar to upstream</a>) 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.</p>
<p dir="auto">Additionally, we could standardise on whether we assert against the <code class="notranslate">first</code> or <code class="notranslate">last</code> email in the queue - using <code class="notranslate">last</code> would possibly have avoided us noticing this, but even if we fix it properly it's reasonable to be consistent.</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />Reply to this email directly, <a href="https://github.com/openstreetmap/openstreetmap-website/issues/5687">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AAK2OLPTCLOSACS43JOBJRT2QB3HFAVCNFSM6AAAAABXHQEOIKVHI2DSMVQWIX3LMV43ASLTON2WKOZSHA2TMMBVGIYTOOI">unsubscribe</a>.<br />You are receiving this because you are subscribed to this thread.<img src="https://github.com/notifications/beacon/AAK2OLKHXA5GGAD2P5N2SC32QB3HFA5CNFSM6AAAAABXHQEOIKWGG33NNVSW45C7OR4XAZNFJFZXG5LFVJRW63LNMVXHIX3JMTHKUO7F2M.gif" height="1" width="1" alt="" /><span style="color: transparent; font-size: 0; display: none; visibility: hidden; overflow: hidden; opacity: 0; width: 0; height: 0; max-width: 0; max-height: 0; mso-hide: all">Message ID: <span><openstreetmap/openstreetmap-website/issues/5687</span><span>@</span><span>github</span><span>.</span><span>com></span></span></p>

<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/openstreetmap/openstreetmap-website/issues/5687",
"url": "https://github.com/openstreetmap/openstreetmap-website/issues/5687",
"name": "View Issue"
},
"description": "View this Issue on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>