Elixirdaze - Become a Test Whisperer - what are your tests telling you?

Slides plus presenter notes.
Pitch: Your design is flawed and you have the proof right in front of you, if you know how to listen. When your tests are hard to write, they are speaking to you. They have a story to tell about your code. We’ll take on some real world examples where listening to our ExUnit tests saved our bacon.

Transcript

1. Become a Test Whisperer what are your tests telling you? Jeffrey Matthias jeffrey@matthias.org @idlehands • Engineer at weedmaps, but I’ve been lucky enough to have worked at 3 companies • Everything I have done has been part of a team. That means our successes are shared and so are our failures… this is some of both. The fun part is that, being at a conference local to where I’ve worked… a lot of those team members past and present are in the audience. • But that’s what we do, right? We make mistakes. Let’s talk about one way to recognize them
2. Testing is hard. • Writing test is easy. Writing good tests is hard • I feel like we are still finding out way with a lot of things in elixir
3. What is a <NOUN> whisperer? Let’s talk about the title of this talk Popular culture
7. Test Whisperer • This is what you call your talk when you’re scrambling for a title right before the submission deadline… only later to realize that it’s kind of dumb. But, like so many mistakes, let’s see what we can make of it. • When I looked it up, the emphasis was on telling things what to do… makes sense, whispering is talking.. but I want us to listen
8. –Martin Fowler "a code smell is a surface indication that usually corresponds to a deeper problem in the system” • Sometimes they are there and sometimes they are not • We hone our sense of code smell over time, mostly by failing
9. Test Smell • Test smell - when an issue in your tests corresponds to a deeper problem in your code
10. Become a Test Smeller what are your tests smelling you? Jeffrey Matthias jeffrey@matthias.org @idlehands • Let’s start over • Think this would have gotten me picked?
11. Testing What is it good for? • Why do we write tests? • Who likes writing tests? - I have a background in sculpture, product design, and automotive technology(glorified) • I get to create AND to know when something works…
12. –Charles Kettering “A problem well stated is a problem half-solved.” • A quote I found early in my coding career that really stuck with me • For me, defining tests helps me get to knowing what to build • I’m going to look at three examples of when defining the question helped raise a smell. • Nothing better than mixing metaphors.
13. • The code examples you are going to see today are my best attempt at recreating the patterns, but none of it is real, so if there is a mistake, you’ll have to pretend you don’t see it. • Let’s set the stage for our first example. Sometimes we are moving REALLY fast. • Every company I’ve worked for has had the goal of making money. • For the first example, I had a junior engineer come to me and say • So, I looked at it with him
14. 1 defmodule ConnectivityApi.TypeBEdgeControllerTest do 2 use ConnectivityApi.ConnCase 3 4 alias Database.Factory 5 6 @valid_params %{ 7 "attribute_1" => 153.0, 8 "attribute_2" => :switched, 9 "attribute_3" => true, 10 "name" => "type_b_edge", 11 "custom-id" => "Edge2" 12 } 13 14 describe "POST" do 15 test "it creates a type_a_edge when passed a graph head", ~m(conn graph_head)a do 16 end 17 18 test "it creates a type_a_edge when passed a node", ~m(conn edge)a do 19 end 20 21 test "fails when caller not authorized", ~m|conn edge|a do 22 end 23 24 test "fails without a node type", ~m|conn edge|a do 25 end 26 27 test "it cannot create type_a_edges if node and type don't match", ~m(conn edge)a do 28 end 29 30 test "cannot create type_a_edges without a source id", ~m(conn)a do 31 end 32 end 33 end • Ok… there are a lot of tests for a single action… show me where you’re getting this
15. 1 defmodule ConnectivityApi.TypeAEdgeControllerTest do 2 use ConnectivityApi.ConnCase 3 4 alias Database.Factory 5 6 @valid_params %{ 7 "attribute_1" => 100.0, 8 "name" => "type_a_edge", 9 "custom-id" => "Edge1" 10 } 11 12 describe "POST" do 13 test "it creates a type_a_edge when passed a graph head", ~m(conn graph_head)a do 14 end 15 16 test "it creates a type_a_edge when passed a node", ~m(conn edge)a do 17 end 18 19 test "fails when caller not authorized", ~m|conn edge|a do 20 end 21 22 test "fails without a node type", ~m|conn edge|a do 23 end 24 25 test "it cannot create type_a_edges if node and type don't match", ~m(conn edge)a do 26 end 27 28 test "cannot create type_a_edges without a source id", ~m(conn)a do 29 end 30 end 31 end • It was painful because finding those little differences sucked • so… what did the controller code look like?
16. 1 defmodule ConnectivityApi.TypeAEdgeController do 2 use EdgeControllerCrud, model: TypeAEdge, description: "type_a_edge" 3 end 1 defmodule ConnectivityApi.TypeBEdgeController do 2 use EdgeControllerCrud, model: TypeBEdge, description: "type_b_edge" 3 end So… what’s going on here? It looks like we have some macros!
17. 1 defmodule ConnectivityApi.EdgeControllerCrud do 2 defmacro __using__(model: model_name, description: public_name) do 3 quote do 4 use ConnectivityApi.Web, :controller 5 6 def show(conn, id, user, claims) do 7 unquote(__MODULE__).show(conn, id, user, claims, 8 model: Database.Query.unquote(model_name)) 9 end 10 11 def create(conn, data, user, claims) do 12 unquote(__MODULE__).create(conn, data, user, claims, 13 model: Database.Query.unquote(model_name), 14 model_description: unquote(public_name)) 15 end 16 17 def update(conn, data, user, claims) do 18 unquote(__MODULE__).update(conn, data, user, claims, 19 model: Database.Query.unquote(model_name)) 20 end 21 22 def delete(conn, id, user, claims) do 23 unquote(__MODULE__).delete(conn, id, user, claims, 24 model: Database.Query.unquote(model_name)) 25 end 26 end 27 end 28 29 use ConnectivityApi.Web, :controller 30 31 alias SharedServices.Authorization 32 alias ConnectivityApi.Query.Edge, as: EdgeQueries Calling on functions defined below. We’re going to focus on create
18. 1 defmodule ConnectivityApi.EdgeControllerCrud do 2 defmacro __using__(model: model_name, description: public_name) do 3 quote do 4 use ConnectivityApi.Web, :controller 5 6 def show(conn, id, user, claims) do 7 unquote(__MODULE__).show(conn, id, user, claims, 8 model: Database.Query.unquote(model_name)) 9 end 10 11 def create(conn, data, user, claims) do 12 unquote(__MODULE__).create(conn, data, user, claims, 13 model: Database.Query.unquote(model_name), 14 model_description: unquote(public_name)) 15 end 16 17 def update(conn, data, user, claims) do 18 unquote(__MODULE__).update(conn, data, user, claims, 19 model: Database.Query.unquote(model_name)) 20 end 21 22 def delete(conn, id, user, claims) do 23 unquote(__MODULE__).delete(conn, id, user, claims, 24 model: Database.Query.unquote(model_name)) 25 end 26 end 27 end 28 29 use ConnectivityApi.Web, :controller 30 31 alias SharedServices.Authorization 32 alias ConnectivityApi.Query.Edge, as: EdgeQueries • This applies to all of these, though
19. 1 def create(conn, %{"data" => data}, user, _claims, 2 model: model_name, model_description: description) do 3 with :ok <- check_has_graph_head_or_parent(attrs), 4 :ok <- validate_parent_type(data), 5 true <- Authorization.can?(user, :create, Database.Edge), 6 {:ok, edge} <- model_name.create(attrs) 7 do 8 render(conn, :show, data: EdgeQueries.with_graph_head(edge)) 9 else 10 :no_graph_head -> 11 # handle missing graph head 12 :missing_parent_type -> 13 # handle missing parent type 14 :parent_type_must_node_type -> 15 # handle type mismatch 16 {:error, changeset} -> 17 # handle ecto changeset 18 _ -> 19 render_not_found(conn) 20 end 21 end • It’s, um, robust
20. 1 def create(conn, %{"data" => data}, user, _claims, 2 model: model_name, model_description: description) do 3 with :ok <- check_has_graph_head_or_parent(attrs), 4 :ok <- validate_parent_type(data), 5 true <- Authorization.can?(user, :create, Database.Edge), 6 {:ok, edge} <- model_name.create(attrs) 7 do 8 render(conn, :show, data: EdgeQueries.with_graph_head(edge)) 9 else 10 :no_graph_head -> 11 # handle missing graph head 12 :missing_parent_type -> 13 # handle missing parent type 14 :parent_type_must_node_type -> 15 # handle type mismatch 16 {:error, changeset} -> 17 # handle ecto changeset 18 _ -> 19 render_not_found(conn) 20 end 21 end • I’m seeing a mix of controller and business logic • But macros have made this so easy!
21. To Macro or not to Macro? • Let’s take a second to talk about macros? They’re so cool. • Who cares? We’re really talking about drying up our code. Pick your way of doing that. • Today, we’re talking about macros, and more importantly, we’re talking about the wrong abstraction
22. • We combined our HTTP logic and our business logic. • All use cases need to be done through integration tests. Those are slow • They carry a higher cognitive load
23. • Let’s pull the business logic out of the controller - SRP • You can call this Service layer whatever you want.
24. 1 def create(conn, %{"data" => data}, user, _claims, 2 model: model_name, model_description: description) do 3 with :ok <- check_has_graph_head_or_parent(attrs), 4 :ok <- validate_parent_type(data), 5 true <- Authorization.can?(user, :create, Database.Edge), 6 {:ok, edge} <- model_name.create(attrs) 7 do 8 render(conn, :show, data: EdgeQueries.with_graph_head(edge)) 9 else 10 :no_graph_head -> 11 # handle missing graph head 12 :missing_parent_type -> 13 # handle missing parent type 14 :parent_type_must_node_type -> 15 # handle type mismatch 16 {:error, changeset} -> 17 # handle ecto changeset 18 _ -> 19 render_not_found(conn) 20 end 21 end • So, if we start back here and pull that extra concerns we get…
25. 1 def create(conn, %{"data" => data}, user, _claims, 2 model: model_name, model_description: description) do 3 with {true, _} <- {Authorization.can?(user, :create, Database.Edge), :unauthorized} 4 {:ok, edge} <- model_name.create(attrs) 5 do 6 render(conn, :show, data: edge) 7 else 8 {false, :unauthorized} -> 9 render_unauthorized() 10 {:error, message} -> 11 render_unprocessable() 12 _ -> 13 render_not_found(conn) 14 end 15 end • Not that much smaller, but we’re not playing golf, we’re reducing cognitive load. • Maybe not even a macro at this point. • Let’s look at the tests
26. 1 defmodule ConnectivityApi.TypeAEdgeControllerTest do 2 use ConnectivityApi.ConnCase 3 4 alias Database.Factory 5 6 @valid_params %{ 7 "attribute_1" => 100.0, 8 "name" => "type_a_edge", 9 "custom-id" => "Edge1" 10 } 11 12 describe "POST" do 13 test "it creates a type_a_edge when passed valid params", ~m(conn graph_head)a do 14 end 15 16 test "fails when caller not authorized", ~m|conn edge|a do 17 end 18 19 test "fails with invalid params", ~m|conn edge|a do 20 end 21 end 22 end But, let’s look at the tests… we have A success case and two “controller related” fail cases. Everything else gets pushed down somewhere else.
27. • A note on your service layer… • You can build tests that focus just on the behavior of the macros, too, so your service layers can also be simplified, but even if not, splitting them out reduces a lot of the cognitive load and reduces repetition in your testing.
28. • By speaking up, that engineer helped identify an issue • The next example is a little different. It’s something I’ve learned to keep an eye out for
29. 1 tag :capture_log 2 test "submit that thing and return error if API is unreachable", ~m{thing1 thing2}a do 3 expect(ThirdPartyServiceMock, :post, 4 fn(payload) -> 5 {:error, %HTTPoison.Error{id: nil, reason: :connect_timeout}}] 6 end) 7 assert {:error, :request_failed, :connect_timeout} = make_request(thing1, thing2) 8 end • Here’s a test. It’s got a mock, which is lovely, and it only tests one thing, which is great. • But it has a red flag on it.
30. 1 tag :capture_log 2 test "submit that thing and return error if API is unreachable", ~m{thing1 thing2}a do 3 expect(ThirdPartyServiceMock, :post, 4 fn(payload) -> 5 {:error, %HTTPoison.Error{id: nil, reason: :connect_timeout}}] 6 end) 7 assert {:error, :request_failed, :connect_timeout} = make_request(thing1, thing2) 8 end • Why is is this here? • I like clean test output. I really do. Nothing makes me happier than a row of green dots.
31. .......................................21:54:26.277 [error] Postgrex.Protocol (#PID<0.976.0>) disconnected: ** (DBConnection.ConnectionError) owner #PID<0.4304.0> exited while client #PID<0.4336.0> is still running with: shutdown..21:54:26.582 [error] Postgrex.Protocol (#PID<0.973.0>) disconnected: ** (DBConnection.ConnectionError) owner #PID<0.4395.0> exited while client #PID<0.4413.0> is still running with: shutdown ..................[warn] [21:54:26.971] API is a jerk: connect_timeout ......................................................................... .. Finished in 3.0 seconds 134 tests, 0 failures Tests passed! Ship it. • Sometimes all is good. You see the reason the capture is there. Other times… • I see another process that is running and dying. It may or may not be related to what we are trying to do. • It may be that we have a legit, important process running. I don’t know. But if we hide it and it turns out to be important… well, that’s bad.
32. 1 test "submit that thing and return error if API is unreachable", ~m{thing1 thing2}a do 2 assert capture_log(fn -> 3 expect(ThirdPartyServiceMock, :post, 4 fn(payload) -> 5 {:error, %HTTPoison.Error{id: nil, reason: :connect_timeout}}] 6 end) 7 assert {:error, :request_failed, :connect_timeout} = make_request(thing1, thing2) 8 end) =~ "connect_timeout" 9 end • Keeps your output clean without hiding anything
33. Assertion with =~ failed code: assert capture_log(fn -> expect(ThirdPartyServiceMock, :post, fn(payload) -> {:error, %HTTPoison.Error{id: nil, reason: :connect_timeout}}] end) assert {:error, :request_failed, :connect_timeout} = make_request(thing1, thing2) end) =~ "connect_timeout" left: "\e[33m21:54:26.277 [error] Postgrex.Protocol (#PID<0.977.0>) disconnected: ** (DBConnection.ConnectionError) owner #PID<0.4303.0> exited while client #PID<0.4336.0> is still running with: shutdown"\n\e[0m" right: “connect_timeout"
34. ......................................................................... ........................................................... Finished in 2.9 seconds 134 tests, 0 failures • There’s our row of green dots!
35. • Next - writing a new test for a bug/existing code • We had an issue where we were trying to do deletes on stale structs. • Digging into it, realized that we were getting two messages over a queue that were nearly identical and handling them on different servers. • To the tests!
36. 1 test "locks while updating", ~m(thing_to_change)a do 2 changed_things = %{ 3 things: [ 4 %{name: "My new thing name", value: 2_500}, 5 %{name: "My other new thing name", value: 30_000} 6 ] 7 } 8 params = 9 thing_to_change 10 |> Map.from_struct 11 |> Map.merge(changed_prices) 12 13 update_thing = fn(_) -> 14 try do 15 {:ok, _thing} = Thing.create_or_update(params) 16 true 17 rescue 18 Ecto.StaleEntryError -> false 19 end 20 end 21 22 success_results = 23 1..3 24 |> Enum.map(&Task.async(fn -> update_menu_item.(&1) end)) 25 |> Enum.map(&Task.await/1) 26 27 assert ! false in success_results, "A StaleEntryError occurred on at least one update." 28 end • I created the race condition even though it only existed when we had more than one server. Elixir made it EASY. • The solution was a database lock, I added that • I was proud • I was showing a coworker and he asked me, “but why the race condition?”
37. • Naughty other service • Love to blame them, but… • There are a few ways we could resolve this and which way is more about what you’re up to
38. • One is to have a single external listener, another to connect our nodes and have a single listener • I was mad, but he was’t missing the point.. elixir made it so easy that I wasn’t asking questions
39. smell your tests • When you find mistakes, look at your tests… was there a smell? • Elixir and erlang are awesome • Totally different dynamics • Smells both old and new - there is no magic list of things • Your tests are one more component of your code base… pay attention to them
40. ................................. Jeffrey Matthias @idlehands Finished in 40 minutes 35 slides, 0 failures? Thanks to: Brad Smith, Michael Bishop, Trace Helms, Christian Wesselhoeft, Adam Legg, Swain Brooks, Moxley Stratton, Alex Weidmann, Justin Weidmann, Parkifi, Enabla, Weedmaps If I missed your name, I’m sorry. • I co-organize the MEETUP!!!!