Add refute_email_delivered_with helper, tests #484
Conversation
| @@ -203,6 +203,26 @@ defmodule Bamboo.Test do | |||
| end | |||
| end | |||
|
|
|||
| defmacro refute_email_delivered_with(email_params) do | |||
There was a problem hiding this comment.
Thanks for this!! Thoughts on adding some documentation for it?
| |> TestMailer.deliver_now() | ||
|
|
||
| refute_email_delivered_with(text_body: ~r/tea/) | ||
| refute_email_delivered_with(to: "bla") |
There was a problem hiding this comment.
I think there can only be one assertion of this type per email delivery -- the received email params are only pulled for the first assertion. If you switch this line to refute_email_delivered_with(to: [nil: "foo@bar.com"]) it passes which it should not? So I think for these additional assertions to be relevant, we need to call deliver_now again before each - similar to https://github.com/thoughtbot/bamboo/blob/3f79b73a4e27d9b27694fb4a72473caab46c857f/test/lib/bamboo/adapters/test_adapter_test.exs#L56-L63
There was a problem hiding this comment.
You're right, of course, will fix.
I wonder though if this is the best way to implement these assertions. It would be better to store the emails in an agent and match over all of them. That would require a big refactor, but it might be worth it.
There was a problem hiding this comment.
I wonder though if this is the best way to implement these assertions. It would be better to store the emails in an agent and match over all of them. That would require a big refactor, but it might be worth it.
What do you think about keeping it as is for now and merging this and opening a new issue for the potential refactor?
mtarnovan
force-pushed
the
add_refute_email_delivered_with
branch
from
f5ea1eb
to
1deb19c
Compare
| TestMailer.deliver_now(mail) | ||
|
|
||
| try do | ||
| refute_email_delivered_with(subject: subject) |
There was a problem hiding this comment.
This, and the others below, are in the test called when email does not match but it seems they are testing when email matches, which there is a separate test for? Thoughts on regrouping them, or alternatively removing some of them if they don't each add much additional value?
Also, if we do keep all of them, thoughts on extracting a helper function for the repeated code?
There was a problem hiding this comment.
Right, I don't think they add too much value. I've refactored the tests a bit if you want to take another look.
There was a problem hiding this comment.
Nice, thanks so much for this!
mtarnovan
force-pushed
the
add_refute_email_delivered_with
branch
from
3928f3e
to
e8fb16d
Compare
| if is_nil(received_email_params) do | ||
| refute false | ||
| else | ||
| refute Enum.any?(email_params, fn {k, v} -> do_match(received_email_params[k], v) end), |
There was a problem hiding this comment.
maybe this should be all? instead 🤔 @mtarnovan
See #474
Let me know if you want to see more tests or if i missed anything obvious. Thanks.