Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add test coverage of email templates #3414

Merged
merged 2 commits into from
Oct 17, 2022
Merged

Add test coverage of email templates #3414

merged 2 commits into from
Oct 17, 2022

Conversation

dnephin
Copy link
Contributor

@dnephin dnephin commented Oct 7, 2022

Summary

I was looking to make a change to some date formatting in one of the emails (to move it out of the data package), but I noticed that the change I was going to make would not be covered by tests.

This PR improves our test coverage for sending emails by:

  • removing the use of package-level variables, and capturing the email into a strut
  • making assertions about the text and html in the email

The expected values here will change, and can automatically be updated by running the tests with -update . The value here is that we get to preview the source of the email, and we'll catch unintended changes to the template.

Add assertions for expected values of rendered email.
So that the test is closer to the production flow.
Copy link
Collaborator

@BruceMacD BruceMacD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me

@dnephin dnephin requested a review from ssoroka October 11, 2022 16:18
@dnephin dnephin merged commit 31bd15b into main Oct 17, 2022
@dnephin dnephin deleted the dnephin/email-testing branch October 17, 2022 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants