mailer_helper update #11665
mailer_helper update #11665
Conversation
| @@ -38,7 +38,9 @@ | |||
| expect_delivered_email( | |||
| to: [user.confirmed_email_addresses.first.email], | |||
| subject: t('user_mailer.account_verified.subject', app_name: APP_NAME), | |||
| body: ['<table class="button expanded large radius">', 'localhost:3000'], | |||
| body: [ | |||
| 'http://www.example.com/redirect/return_to_sp/account_verified_cta', | |||
There was a problem hiding this comment.
Turns out body: is only comparing against the plaintext version of the email, so checking for HTML here fails.
Previously, we weren't correctly processing these body checks, so this was erroneously passing prior to this PR
There was a problem hiding this comment.
I sometimes wonder how many tests erroneously pass and conceal bugs. 😬 Glad you found this.
| # @param [String,nil] to If provided, the email address the message must've been sent to | ||
| # @param [String,nil] subject If provided, the subject the email must've had | ||
| # @param [String[],nil] Array of substrings that must appear in body. | ||
| def expect_email_not_delivered(to: nil, subject: nil, body: nil) |
There was a problem hiding this comment.
TBH I would like to rework this as an Rspec matcher, something like
expect { some_code }.to have_delivered_email(...)But I needed to limit scope a little here
There was a problem hiding this comment.
I agree with you on both counts—that would be nice, but this is a good step forward for now.
- Slightly improve error output - Capture behavior in a dedicated spec [skip changelog]
Assert that a certain email was not sent.
These specs weren't actually testing the thing we thought they were, since the text_part of the email did not contain HTML.
matthinz
force-pushed
the
matthinz/mailer-helper-update
branch
from
7329299 to
3dffdd8
Compare
| @@ -38,7 +38,9 @@ | |||
| expect_delivered_email( | |||
| to: [user.confirmed_email_addresses.first.email], | |||
| subject: t('user_mailer.account_verified.subject', app_name: APP_NAME), | |||
| body: ['<table class="button expanded large radius">', 'localhost:3000'], | |||
| body: [ | |||
| 'http://www.example.com/redirect/return_to_sp/account_verified_cta', | |||
There was a problem hiding this comment.
I sometimes wonder how many tests erroneously pass and conceal bugs. 😬 Glad you found this.
| # @param [String,nil] to If provided, the email address the message must've been sent to | ||
| # @param [String,nil] subject If provided, the subject the email must've had | ||
| # @param [String[],nil] Array of substrings that must appear in body. | ||
| def expect_email_not_delivered(to: nil, subject: nil, body: nil) |
There was a problem hiding this comment.
I agree with you on both counts—that would be nice, but this is a good step forward for now.
* Rework mailer_helper and add spec - Slightly improve error output - Capture behavior in a dedicated spec [skip changelog] * Add expect_email_not_delivered helper Assert that a certain email was not sent. * Update alert_user_about_account_verified_spec These specs weren't actually testing the thing we thought they were, since the text_part of the email did not contain HTML.
🛠 Summary of changes
mailer_helper.rb provides the
expect_delivered_emailhelper, which we use in our specs to assert that an email was sent.This PR:
expect_email_not_delivered