Use refute_timeout configured for all refute_ assertions #603
Conversation
| @@ -255,7 +267,7 @@ defmodule Bamboo.Test do | |||
| receive do | |||
| {:delivered_email, email} -> Map.from_struct(email) | |||
| after | |||
| 100 -> [] | |||
| Bamboo.Test.refute_timeout() -> [] | |||
There was a problem hiding this comment.
If this is going to be a potentially test breaking concern is it worth a deprecation warning or minor version bump?
There was a problem hiding this comment.
Yeah, this is a good question. I honestly don't know. Technically, this is a bug that we're fixing, so it could be a patch. But if we consider the existing behavior as correct and us changing this, it could be considered potentially breaking. What do you think?
There was a problem hiding this comment.
Now that I've thought about this more. Maybe the change we should make is to default the refute_timeout to 100ms instead of 0ms. That would make this not a breaking change. Other tests might wait longer if the email fails to deliver, but they pass as soon as the email is received, so 100ms is a better default than 0ms.
I also like that ExUnit's refute_receive_timeout defaults to 100ms. So it keeps us consistent with that, which probably matches people's expectations more closely.
There was a problem hiding this comment.
That seems like a nice approach to this. They can always change the default explicitly but half a second longer test suite seems like a low price to pay for a suitable default.
There was a problem hiding this comment.
@brimatteng I updated the default to 100ms, so there's no longer a potential for a breaking change, and I updated the PR description.
I'm also hoping to quickly follow up with a PR that allows users to pass the timeout directly, so users should be able to change the timeout per test as desired.
What changed? ============ All refutation test helpers in `Bamboo.Test` should be using the `refute_timeout` configuration when testing on shared mode. ```elixir config :bamboo, :refute_timeout, 10 ``` The `refute_email_delivered_with/1` macro was not using the configuration, but instead it had a hardcoded 100ms. We update it to use the `refute_timeout`, since not using it was a bug. Finally, the `refute_timeout/0` function currently returns 0ms when Bamboo isn't on shared mode. We change that to be 100ms to match the previously hardcoded value in `refute_email_delivered_with/1` and to match the [default `refute_receive_timeout` from ExUnit](https://hexdocs.pm/ex_unit/ExUnit.html#configure/1-options).
germsvel
force-pushed
the
refute-helpers-need-refute-timeout
branch
from
427c795
to
9ba84c1
Compare
What changed?
All refutation test helpers in
Bamboo.Testshould be using therefute_timeoutconfiguration when testing on shared mode.The
refute_email_delivered_with/1macro was not using the configuration, but instead it had a hardcoded 100ms. We update it to use therefute_timeout, since not using it was a bug.Finally, the
refute_timeout/0function currently returns 0ms when Bamboo isn't on shared mode. We change that to be 100ms to match the previously hardcoded value inrefute_email_delivered_with/1and to match the defaultrefute_receive_timeoutfrom ExUnit.