-
Notifications
You must be signed in to change notification settings - Fork 24
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
refactor api_v*_notification_controller tests #173
Conversation
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.
Cool, less code and more readability 🎉
@dropped_fields ["service", "body"] | ||
for dropped <- @dropped_fields do | ||
test "Request.SendNotification.FlatNotification schema without required #{dropped} field", %{ | ||
conn: conn | ||
} do | ||
body = | ||
ControllersHelper.flat_request() | ||
|> Map.drop([unquote(dropped)]) | ||
|> Jason.encode!() |
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.
Niiiice, less duplication 😄
Just one question, maybe I can learn something about elixir here. Why's that unquote
in unquote(dropped)
necessary?
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.
It's macros magic 😁 We're reading the values from so-called module attribute
declared with @
, and because we're kinda injecting it in test body, it needs to be evaluated (back to the string like I wrote the code) and inserted in the place of unquote(dropped)
. To understand more what this macro stuff is about, you can head to here, here, or this answer on elixir forum almost exactly covering this specific case.
body = | ||
ControllersHelper.flat_request() | ||
|> Map.put("badge", "seven") | ||
|> Jason.encode!() | ||
|
||
conn = post(conn, "/v1/notification/666", body) |
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.
Pipes are amazing xD
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 the refactoring. I like the pipe changes the most. When it comes to macros and test code generation, they are OK. I'm not a big fan of this magic. I know this trick reduces repeated code so let's have it.
Jason.encode!(%{ControllersHelper.silent_request() | "time_to_live" => "infinity"}) | ||
) | ||
body = | ||
ControllersHelper.alert_request() |
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.
Guys, maybe this year' golden shovel award goes to me, but shouldn't it be silent_request
? 🙄
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.
nobody knows
Here I introduce some quick refactor of the test suites developed while incorporating
OpenApiSpex
to the project, originally proposed by @aleklisi in #168. Since these changes has been interfering with the ones from #172, decreasing their readability, I decided to extract them to the separate PR.