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

Fix options with assert_delivered_email #74

Closed
paulcsmith opened this issue Feb 16, 2016 · 13 comments
Closed

Fix options with assert_delivered_email #74

paulcsmith opened this issue Feb 16, 2016 · 13 comments
Milestone

Comments

@paulcsmith
Copy link
Contributor

It should normalize the email addresses, and should only match against the keys that are in the options. For asserting just what you pass in, do something like this

defmacro assert_delivered_email(email_params) do
    # Normalize the address as well
    quote do
      import ExUnit.Assertions
      assert_received({:delivered_email, unquote(email_params)})
    end
  end
@jbernardo95
Copy link
Contributor

@paulcsmith What do you mean by options ?

This ?

assert_delivered_email(subject: "Something")

@paulcsmith
Copy link
Contributor Author

Yes exactly :) The feature did not work quite right so I believe I removed it, or at the very least removed docs so people didn't use it

@jbernardo95
Copy link
Contributor

jbernardo95 commented Nov 3, 2016

@paulcsmith Ok, I see.

This would be backwards compatible as well, right ? Accepting an %Email{} or a keyword list with parameters to compare.

@paulcsmith
Copy link
Contributor Author

Yes that is correct. It should accept either of those two options

@jbernardo95
Copy link
Contributor

@paulcsmith 👍

I will open a pr regarding this :)

@jbernardo95
Copy link
Contributor

jbernardo95 commented Nov 3, 2016

@paulcsmith I have been digging through the code.

If we use the assert_received({:delivered_email, ^email}) method as you suggested we will lose the custom flunk_with_email_list error message, or we will have to have the assert_received inside a try block and make all those helper functions public.

Something like this:

defmacro assert_delivered_email(email) do
  quote do
    import ExUnit.Assertions
    email = Bamboo.Test.normalize_for_testing(unquote(email))
    try do
      assert_received({:delivered_email, ^email})
    rescue
      error in [ExUnit.AssertionError] ->
        Bamboo.Test.flunk_with_email_list(email)
    end
  end
end

What do you think of this ?

@jbernardo95
Copy link
Contributor

@paulcsmith Up

@paulcsmith
Copy link
Contributor Author

@jbernardo95 I think maybe another function should be added then. Something like assert_delivered_with. So you'd use it like assert_delivered_with(subject: "Welcome to the app")

Does that help/answer the question?

@jbernardo95
Copy link
Contributor

@paulcsmith Kind of, what I was saying is that if we declare assert_delivered_email as a macro we will have to make flunk_with_email_list public.

The other option we have is to put the import ExUnit.Assertions in the __using__ macro and that way we don't need the macro.

Does that clarify ?

@paulcsmith
Copy link
Contributor Author

I think making flunk_with_email_list public and declaring @doc false to it would probably work ok :D

@jbernardo95
Copy link
Contributor

Yep it does ! Didn't know about hiding methods with @doc false but it is perfect for this case.

@jbernardo95
Copy link
Contributor

@paulcsmith Here is a draft ^, take a look and tell me what you think :)

@jbernardo95
Copy link
Contributor

@paulcsmith Up.

paulcsmith pushed a commit that referenced this issue Dec 13, 2016
Resolves #74.

* Improves assert_delivered_email and adds assert_delivered_with

* Adds documentation to assert_delivered_with/1

* Fixes typos and changes assert email timeout to 100ms
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

No branches or pull requests

2 participants