-
Notifications
You must be signed in to change notification settings - Fork 342
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
Import ExUnit.Assertions in Bamboo.Test.assert_delivered_email_matches #578
Import ExUnit.Assertions in Bamboo.Test.assert_delivered_email_matches #578
Conversation
Requiring it doesn't make certain functions (like `flunk`) which are used in Bamboo.Test available and compilation of assert_delivered_email_matches fails when it's used outside of an ExUnit test module. It works fine when the macro is used in an ExUnit test because ExUnit.Assertions are imported there.
@@ -183,7 +183,7 @@ defmodule Bamboo.Test do | |||
""" | |||
defmacro assert_delivered_email_matches(email_pattern) do | |||
quote do | |||
require ExUnit.Assertions | |||
import ExUnit.Assertions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching this @eugenebolshakov . I actually think I would like to turn the rest of the import ExUnit.Assertions
-> require ExUnit.Assertions
. That way we're not importing all the assertions for every single macro we define.
That being said, you mentioned you're getting an error because flunk
isn't available:
I noticed that if I use assert_delivered_email_matches in my module then it doesn't compile because it complains about flunk from ExUnit.Assertions not being available. I thought that it could be fixed by importing ExUnit.Assertions in this macro like in the other macros instead of requiring it.
This function doesn't use flunk
, so it seems to me like the error you're getting is coming from somewhere else. Importing the assertions here might fix it, but perhaps only accidentally so?
Are you importing ExUnit.Assertions
in your own wrapper, where you use flunk
? Is there any more insight into your code that you could provide?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@germsvel Thanks!
This function doesn't use
flunk
, so it seems to me like the error you're getting is coming from somewhere else.
Yes, I was surprised by that too. I haven't tried to dig deeper though to understand why it's happening.
Is there any more insight into your code that you could provide?
Here's my wrapper module:
defmodule Bazaar.Email.Test do
import Bamboo.Test
defmacro __using__(_opts) do
quote do
use Bamboo.Test
import Bazaar.Email.Test
end
end
def assert_email_sent(email, template) do
# Bamboo.Test.assert_delivered_email_matches doesn't import
# ExUnit.Assertions, but just requires it unlike the other macros in
# Bamboo.Test. It might be a bug.
import ExUnit.Assertions
assert_delivered_email_matches(%{to: [nil: ^email], private: %{template_name: ^template}})
end
def refute_email_sent(email) do
refute_email_delivered_with(to: [nil: email])
end
end
I use
this wrapper module in my tests. As you can see I don't use flank
in my wrapper. If I remove import ExUnit.Assertions
from assert_email_sent
above I get this error:
== Compilation error in file test/support/email.ex ==
** (CompileError) test/support/email.ex:16: undefined function flunk/1
(elixir 1.11.2) src/elixir_locals.erl:114: anonymous fn/3 in :elixir_locals.ensure_no_undefined_local/3
(stdlib 3.13.2) erl_eval.erl:680: :erl_eval.do_apply/6
test/support/email.ex:16
is the line where assert_delivered_email_matches
is called. It doesn't complain about refute_email_delivered_with
which is another macro defined in Bamboo.Test
, but which imports ExUnit.Assertions
. Therefore I assumed that it was what made the difference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's really odd. Given that we're using require
here but all other macros are using import
, I'm okay merging this as is. I'd like to eventually move to require
, but not sure when that would happen (or even if it would happen). Thanks for opening this and walking me through it!
This fixes what is a probably very rare use-case: I'm using macros from
Bamboo.Test
in helper functions that provide a more convenient API for testing emails in my codebase, i.e. I'm basically wrappingBamboo.Test
in my own module.I noticed that if I use
assert_delivered_email_matches
in my module then it doesn't compile because it complains aboutflunk
fromExUnit.Assertions
not being available. I thought that it could be fixed by importingExUnit.Assertions
in this macro like in the other macros instead of requiring it.