Don't use macros for telemetry and logging test helpers

mtrudel · Jan 4, 2025 · 24285d7 · 24285d7
1 parent 8286248
commit 24285d7
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 38 deletions.
diff --git a/test/bandit/websocket/sock_test.exs b/test/bandit/websocket/sock_test.exs
@@ -1294,7 +1294,7 @@ defmodule WebSocketWebSockTest do
     end
 
     test "it should send `start` events on websocket connection", context do
-      TelemetryHelpers.attach_all_events(TelemetrySock)
+      TelemetryHelpers.attach_all_events(TelemetrySock) |> on_exit()
 
       client = SimpleWebSocketClient.tcp_client(context)
       SimpleWebSocketClient.http1_handshake(client, TelemetrySock)
@@ -1312,7 +1312,7 @@ defmodule WebSocketWebSockTest do
     end
 
     test "it should gather send and recv metrics for inclusion in `stop` events", context do
-      TelemetryHelpers.attach_all_events(TelemetrySock)
+      TelemetryHelpers.attach_all_events(TelemetrySock) |> on_exit()
 
       client = SimpleWebSocketClient.tcp_client(context)
       SimpleWebSocketClient.http1_handshake(client, TelemetrySock)
@@ -1355,7 +1355,7 @@ defmodule WebSocketWebSockTest do
     end
 
     test "it should send `stop` events on normal websocket client disconnection", context do
-      TelemetryHelpers.attach_all_events(TelemetrySock)
+      TelemetryHelpers.attach_all_events(TelemetrySock) |> on_exit()
 
       client = SimpleWebSocketClient.tcp_client(context)
       SimpleWebSocketClient.http1_handshake(client, TelemetrySock)
@@ -1380,7 +1380,7 @@ defmodule WebSocketWebSockTest do
     end
 
     test "it should send `stop` events on normal websocket server disconnection", context do
-      TelemetryHelpers.attach_all_events(TelemetrySock)
+      TelemetryHelpers.attach_all_events(TelemetrySock) |> on_exit()
 
       client = SimpleWebSocketClient.tcp_client(context)
       SimpleWebSocketClient.http1_handshake(client, TelemetrySock)
@@ -1405,7 +1405,7 @@ defmodule WebSocketWebSockTest do
     end
 
     test "it should send `stop` events on normal server shutdown", context do
-      TelemetryHelpers.attach_all_events(TelemetrySock)
+      TelemetryHelpers.attach_all_events(TelemetrySock) |> on_exit()
 
       client = SimpleWebSocketClient.tcp_client(context)
       SimpleWebSocketClient.http1_handshake(client, TelemetrySock)
@@ -1426,7 +1426,7 @@ defmodule WebSocketWebSockTest do
     test "it should send `stop` events on abnormal websocket server disconnection", context do
       output =
         capture_log(fn ->
-          TelemetryHelpers.attach_all_events(TelemetrySock)
+          TelemetryHelpers.attach_all_events(TelemetrySock) |> on_exit()
 
           client = SimpleWebSocketClient.tcp_client(context)
           SimpleWebSocketClient.http1_handshake(client, TelemetrySock)
@@ -1458,7 +1458,7 @@ defmodule WebSocketWebSockTest do
 
     test "it should send `stop` events on timeout", context do
       context = http_server(context, thousand_island_options: [read_timeout: 100])
-      TelemetryHelpers.attach_all_events(TelemetrySock)
+      TelemetryHelpers.attach_all_events(TelemetrySock) |> on_exit()
 
       client = SimpleWebSocketClient.tcp_client(context)
       SimpleWebSocketClient.http1_handshake(client, TelemetrySock)

diff --git a/test/support/logger_helpers.ex b/test/support/logger_helpers.ex
@@ -1,18 +1,14 @@
 defmodule LoggerHelpers do
   @moduledoc false
 
-  defmacro receive_all_log_events(plug_or_sock) do
-    quote do
-      # Yes, this burns atoms but logger needs atoms for refs. See note below about at_exit
-      ref = make_ref() |> inspect() |> String.to_atom()
+  def receive_all_log_events(plug_or_sock) do
+    # Yes, this burns atoms but logger needs atoms for refs. See note below about at_exit
+    ref = make_ref() |> inspect() |> String.to_atom()
 
-      :logger.add_handler(ref, LoggerHelpers, %{
-        config: %{pid: self(), plug: unquote(plug_or_sock)}
-      })
+    :logger.add_handler(ref, __MODULE__, %{config: %{pid: self(), plug: plug_or_sock}})
 
-      # Ideally we'd have an at_exit hook that calls remove_handler but it seems to be racy inside
-      # logger if we do so
-    end
+    # Ideally we'd have an at_exit hook that calls remove_handler but it seems to be racy inside
+    # logger if we do so
   end
 
   def log(%{meta: %{plug: {plug, _}}} = log_event, %{config: %{pid: pid, plug: plug}}),

diff --git a/test/support/server_helpers.ex b/test/support/server_helpers.ex
@@ -39,7 +39,7 @@ defmodule ServerHelpers do
           |> Bandit.child_spec()
           |> start_supervised()
 
-        TelemetryHelpers.attach_all_events(__MODULE__)
+        TelemetryHelpers.attach_all_events(__MODULE__) |> on_exit()
         LoggerHelpers.receive_all_log_events(__MODULE__)
 
         {:ok, {_ip, port}} = ThousandIsland.listener_info(server_pid)

diff --git a/test/support/telemetry_helpers.ex b/test/support/telemetry_helpers.ex
@@ -1,27 +1,18 @@
 defmodule TelemetryHelpers do
   @moduledoc false
 
-  defmacro attach_all_events(plug_or_websock) do
-    events = [
-      [:bandit, :request, :start],
-      [:bandit, :request, :stop],
-      [:bandit, :request, :exception],
-      [:bandit, :websocket, :start],
-      [:bandit, :websocket, :stop]
-    ]
+  @events [
+    [:bandit, :request, :start],
+    [:bandit, :request, :stop],
+    [:bandit, :request, :exception],
+    [:bandit, :websocket, :start],
+    [:bandit, :websocket, :stop]
+  ]
 
-    quote do
-      ref = make_ref()
-
-      :telemetry.attach_many(
-        ref,
-        unquote(events),
-        &TelemetryHelpers.handle_event/4,
-        {self(), unquote(plug_or_websock)}
-      )
-
-      on_exit(fn -> :telemetry.detach(ref) end)
-    end
+  def attach_all_events(plug_or_websock) do
+    ref = make_ref()
+    :telemetry.attach_many(ref, @events, &__MODULE__.handle_event/4, {self(), plug_or_websock})
+    fn -> :telemetry.detach(ref) end
   end
 
   def handle_event(event, measurements, %{plug: {plug, _}} = metadata, {pid, plug}),