-
Notifications
You must be signed in to change notification settings - Fork 333
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
Improves assert_delivered_email and adds assert_delivered_with #228
Improves assert_delivered_email and adds assert_delivered_with #228
Conversation
Any updates on that one? 😸 |
@optikfluffel I guess this is related with #233, am I right ? |
@@ -146,7 +144,7 @@ defmodule Bamboo.Test do | |||
Checks whether an email was delivered. | |||
|
|||
Must be used with the `Bamboo.TestAdapter` or this will never pass. In case you | |||
are delivering from another process, the assertion waits up to 100ms before | |||
are delivering from another process, the assertion waits up to 150ms before |
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.
Was there a reason for changing this to 150ms?
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.
At the time I ran the tests, 100ms seemed to not be enough. But I ran the tests now 3 times in a row with the 100ms and they seem fine.
Changed back to 100ms.
error in [ExUnit.AssertionError] -> | ||
assert error.message =~ "0 emails delivered" | ||
else | ||
_ -> flunk "assert_delivered_email should failed" |
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.
should have failed
? both here and in the next test
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.
Yep, typos.
@jbernardo95 Thanks for tackling his. Just a few small comments and then I'll merge this in! Also, I think that feature is somewhat related, but I would probably test token generation in a unit test. It's easier to isolate. Then you could use this new function you wrote to just test the subject and skip the body in integration tests |
after | ||
@timeout -> flunk_with_email_list(email) | ||
@doc """ | ||
Checks whether an email its params equal to the ones provided. |
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.
One more small typo: Check whether an email's params are equal to the ones provided
e0b5a55
to
ccf3bbc
Compare
@paulcsmith Updated ! |
@jbernardo95 Very strange! I checked Circle and it passed, bit it isn't reporting to GitHub. Can you squash your commits and force push. Maybe that'll force it to reconnect |
ccf3bbc
to
d97d808
Compare
@paulcsmith Done, let's see if this helps. |
@paulcsmith It didn't help, this seems to be a CircleCI <> GitHub issue. The other new PR I submitted isn't getting any status either. Checked back at CircleCI and it seems to not be getting GitHub notifications of new code. |
@jbernardo95 I'll have to figure this out some other time. It appears to be working fine though so I'll merge this in. Thank you so much! |
Resolves #74.