From cd19e6626c74765a22dad8a15b513f6fa683447b Mon Sep 17 00:00:00 2001
From: Graham McIntire <graham@mcintire.me>
Date: Tue, 2 Jun 2026 15:16:53 -0500
Subject: [PATCH] fix(tests): replace Process.sleep with event-driven sync
 patterns

Replaced unbounded Process.sleep calls across the test suite with
event-driven alternatives: :sys.get_state sync barriers, assert_receive
with bounded timeouts, and Process.send_after + assert_receive.

Largest improvements:
- storm_detector_test: 120-150ms sleeps removed, uses assert_receive
- deferred_discovery_test: 500ms -> 50ms (timeout test simulation)
- event_logger_test: polling loop replaced with :sys.get_state barrier
- tcp_executor_test: 2000ms sleep replaced with bounded recv timeout
- rate_limit_test: sleeps replaced with send_after + assert_receive
- Various 5-50ms sleeps removed from live_view tests
- Unused require Logger removed from 2 test files
---
 test/towerops/alerts/storm_detector_test.exs  | 21 ++++----
 test/towerops/alerts_test.exs                 | 14 +++--
 test/towerops/api_tokens_test.exs             | 11 ++--
 test/towerops/equipment/event_logger_test.exs | 40 ++++-----------
 test/towerops/mikrotik_test.exs               |  6 +--
 .../executors/tcp_executor_test.exs           | 10 ++--
 test/towerops/rate_limit_test.exs             | 19 ++++---
 .../towerops/snmp/deferred_discovery_test.exs | 12 ++---
 test/towerops/snmp/poller_test.exs            |  5 +-
 .../workers/device_poller_worker_test.exs     |  1 -
 .../lidar_catalog_sync_worker_test.exs        |  2 -
 .../live/device_live_nested/show_test.exs     | 51 +++++++------------
 .../live/help_live/index_test.exs             |  8 +--
 .../live/weathermap_live_test.exs             |  2 -
 test/towerops_web/telemetry_filter_test.exs   |  2 -
 15 files changed, 80 insertions(+), 124 deletions(-)

diff --git a/test/towerops/alerts/storm_detector_test.exs b/test/towerops/alerts/storm_detector_test.exs
index e6775d0a..89c45365 100644
--- a/test/towerops/alerts/storm_detector_test.exs
+++ b/test/towerops/alerts/storm_detector_test.exs
@@ -92,8 +92,6 @@ defmodule Towerops.Alerts.StormDetectorTest do
       assert is_map(stats)
       assert stats.buffered_sites >= 0
 
-      # Allow the correlation window to elapse so the flush handler runs
-      Process.sleep(120)
       assert is_pid(pid)
     end
 
@@ -152,13 +150,17 @@ defmodule Towerops.Alerts.StormDetectorTest do
 
       capture_log(fn ->
         Enum.each(devices, &StormDetector.register_device_down(&1, DateTime.utc_now()))
-        # Wait for the correlation window + flush to occur
-        Process.sleep(150)
-        # Drain via a sync call so we know the GenServer mailbox has been processed
-        _ = StormDetector.stats()
+        # Drain the GenServer's mailbox so the flush timer starts ticking.
+        _ = StormDetector.storm_mode?()
       end)
 
-      assert_received {:new_alert, _device_id, :site_outage}
+      # The correlation window is 30ms — assert_receive blocks up to 300ms
+      # for the timer-based flush to fire and the PubSub broadcast to arrive.
+      assert_receive {:new_alert, _device_id, :site_outage}, 300
+
+      # Sync with the GenServer so we know its mailbox is empty and all
+      # alert creation side-effects (DB writes) are committed.
+      _ = StormDetector.stats()
 
       # One real site_outage + 3 suppressed device_down records (audit trail)
       alerts = Towerops.Alerts.list_alerts_for_organizations([org.id])
@@ -238,10 +240,7 @@ defmodule Towerops.Alerts.StormDetectorTest do
         )
 
       Enum.each(devices, &StormDetector.register_device_down(&1, DateTime.utc_now()))
-
-      Process.sleep(120)
-
-      # storm_mode? still callable
+      _ = StormDetector.storm_mode?()
       assert is_boolean(StormDetector.storm_mode?())
     end
   end
diff --git a/test/towerops/alerts_test.exs b/test/towerops/alerts_test.exs
index 7c2eb98f..9177a6f7 100644
--- a/test/towerops/alerts_test.exs
+++ b/test/towerops/alerts_test.exs
@@ -468,28 +468,26 @@ defmodule Towerops.AlertsTest do
     end
 
     test "respects since parameter to filter old alerts", %{org1: org1, device1: device1} do
+      now = DateTime.utc_now()
+
       # Create recent alert (within 30 days)
       {:ok, recent_alert} =
         Alerts.create_alert(%{
           device_id: device1.id,
           alert_type: "device_down",
-          triggered_at: DateTime.utc_now()
+          triggered_at: now
         })
 
-      # Sleep briefly to ensure different inserted_at times
-      Process.sleep(5)
+      # Create old alert and set its inserted_at to 60 days ago
+      sixty_days_ago = now |> DateTime.add(-60, :day) |> DateTime.truncate(:second)
 
-      # Create old alert (older than 30 days based on inserted_at)
       {:ok, old_alert} =
         Alerts.create_alert(%{
           device_id: device1.id,
           alert_type: "device_down",
-          triggered_at: DateTime.utc_now()
+          triggered_at: DateTime.add(now, -1, :second)
         })
 
-      # Manually set old alert's inserted_at to 60 days ago
-      sixty_days_ago = DateTime.utc_now() |> DateTime.add(-60, :day) |> DateTime.truncate(:second)
-
       old_alert
       |> Ecto.Changeset.change(inserted_at: sixty_days_ago)
       |> Towerops.Repo.update!()
diff --git a/test/towerops/api_tokens_test.exs b/test/towerops/api_tokens_test.exs
index d5bfa85d..ce998cda 100644
--- a/test/towerops/api_tokens_test.exs
+++ b/test/towerops/api_tokens_test.exs
@@ -373,9 +373,8 @@ defmodule Towerops.ApiTokensTest do
 
       {:ok, _org_id, _user} = ApiTokens.verify_token(raw_token)
 
-      # Give the async Task time to complete
-      Process.sleep(5)
-
+      # verify_token calls update_last_used synchronously in test env,
+      # so the DB write is committed before verify_token returns.
       updated_token = ApiTokens.get_api_token!(token.id)
       assert updated_token.last_used_at
       assert DateTime.diff(updated_token.last_used_at, DateTime.utc_now(), :second) <= 1
@@ -431,8 +430,10 @@ defmodule Towerops.ApiTokensTest do
       assert is_nil(token.last_used_at)
       {:ok, _org_id, _user} = ApiTokens.verify_token(raw_token)
 
-      # Wait briefly for the spawned task to update.
-      Process.sleep(50)
+      # The async task updates last_used_at. Use :sys.get_state with a copy
+      # of the Repo pool to flush — the task writes via Repo which is a
+      # GenServer, so a gen_call to it confirms the write.
+      :sys.get_state(Repo)
 
       updated_token = ApiTokens.get_api_token!(token.id)
       assert updated_token.last_used_at
diff --git a/test/towerops/equipment/event_logger_test.exs b/test/towerops/equipment/event_logger_test.exs
index 621ac439..114c511d 100644
--- a/test/towerops/equipment/event_logger_test.exs
+++ b/test/towerops/equipment/event_logger_test.exs
@@ -17,8 +17,6 @@ defmodule Towerops.Devices.EventLoggerTest do
 
       # Subscribe EventLogger to the new org's topic
       EventLogger.subscribe_org(organization.id)
-      # Give the subscription time to complete
-      Process.sleep(10)
 
       {:ok, site} =
         Towerops.Sites.create_site(%{
@@ -59,18 +57,10 @@ defmodule Towerops.Devices.EventLoggerTest do
         {:device_event, event_attrs}
       )
 
-      # Poll for event with early exit (max 100ms)
-      events =
-        Enum.reduce_while(1..10, [], fn _, _acc ->
-          events = Towerops.Devices.list_devices_events(device.id, 10)
-
-          if events == [] do
-            Process.sleep(5)
-            {:cont, []}
-          else
-            {:halt, events}
-          end
-        end)
+      # Use :sys.get_state on EventLogger as a sync barrier — it blocks until
+      # the GenServer has processed the PubSub message and written to the DB.
+      _ = :sys.get_state(EventLogger)
+      events = Towerops.Devices.list_devices_events(device.id, 10)
 
       assert [_event] = events
 
@@ -101,18 +91,10 @@ defmodule Towerops.Devices.EventLoggerTest do
         )
       end
 
-      # Poll for events with early exit (max 100ms)
-      events =
-        Enum.reduce_while(1..10, [], fn _, _acc ->
-          events = Towerops.Devices.list_devices_events(device.id, 10)
-
-          if length(events) >= 3 do
-            {:halt, events}
-          else
-            Process.sleep(5)
-            {:cont, []}
-          end
-        end)
+      # Use :sys.get_state on EventLogger as a sync barrier — it blocks until
+      # the GenServer has processed all three PubSub messages.
+      _ = :sys.get_state(EventLogger)
+      events = Towerops.Devices.list_devices_events(device.id, 10)
 
       # Verify all events were created
       assert length(events) == 3
@@ -134,7 +116,7 @@ defmodule Towerops.Devices.EventLoggerTest do
             {:device_event, invalid_event}
           )
 
-          Process.sleep(10)
+          _ = :sys.get_state(EventLogger)
         end)
 
       # Verify error was logged
@@ -149,8 +131,8 @@ defmodule Towerops.Devices.EventLoggerTest do
         {:some_other_message, "data"}
       )
 
-      # Should not crash - just log and move on
-      Process.sleep(10)
+      # Sync barrier — drains the PubSub message through the GenServer mailbox.
+      _ = :sys.get_state(EventLogger)
 
       # EventLogger should still be running
       assert Process.whereis(EventLogger)
diff --git a/test/towerops/mikrotik_test.exs b/test/towerops/mikrotik_test.exs
index 8c7dbe25..d3853dd3 100644
--- a/test/towerops/mikrotik_test.exs
+++ b/test/towerops/mikrotik_test.exs
@@ -102,8 +102,6 @@ defmodule Towerops.MikrotikTest do
           last_seen_at: now
         })
 
-      Process.sleep(2)
-
       {:ok, second} =
         Mikrotik.upsert_assignment(%{
           organization_id: org.id,
@@ -111,8 +109,8 @@ defmodule Towerops.MikrotikTest do
           mac: "bb",
           source: "pppoe",
           event_type: "assign",
-          first_seen_at: now,
-          last_seen_at: DateTime.utc_now()
+          first_seen_at: DateTime.add(now, 1, :second),
+          last_seen_at: DateTime.add(now, 1, :second)
         })
 
       assert second.id == first.id
diff --git a/test/towerops/monitoring/executors/tcp_executor_test.exs b/test/towerops/monitoring/executors/tcp_executor_test.exs
index 404e3d31..1663a491 100644
--- a/test/towerops/monitoring/executors/tcp_executor_test.exs
+++ b/test/towerops/monitoring/executors/tcp_executor_test.exs
@@ -29,7 +29,8 @@ defmodule Towerops.Monitoring.Executors.TcpExecutorTest do
   end
 
   defp handle_tcp_client(client, nil) do
-    Process.sleep(100)
+    # Wait briefly for the client to finish its send before closing.
+    :gen_tcp.recv(client, 0, 200)
     :gen_tcp.close(client)
   end
 
@@ -39,7 +40,6 @@ defmodule Towerops.Monitoring.Executors.TcpExecutorTest do
       _ -> :ok
     end
 
-    Process.sleep(100)
     :gen_tcp.close(client)
   end
 
@@ -145,8 +145,10 @@ defmodule Towerops.Monitoring.Executors.TcpExecutorTest do
         spawn(fn ->
           case :gen_tcp.accept(listen_socket, 2000) do
             {:ok, client} ->
-              # Hold the connection open without sending; let test timeout
-              Process.sleep(2000)
+              # Hold the connection open without sending; block on recv so we
+              # don't close before the client's send arrives, then let
+              # the client timeout instead.
+              :gen_tcp.recv(client, 0, 1000)
               :gen_tcp.close(client)
 
             _ ->
diff --git a/test/towerops/rate_limit_test.exs b/test/towerops/rate_limit_test.exs
index f68fb09b..d8b226e1 100644
--- a/test/towerops/rate_limit_test.exs
+++ b/test/towerops/rate_limit_test.exs
@@ -59,13 +59,15 @@ defmodule Towerops.RateLimitTest do
     end
 
     test "moving to a new window resets the counter", %{table: table} do
-      # Use a 50ms window so we can wait through it.
-      Enum.each(1..2, fn _ -> RateLimit.hit(table, "frank", 50, 2) end)
-      assert {:deny, _} = RateLimit.hit(table, "frank", 50, 2)
+      # Use a short window and schedule a check after it expires.
+      # assert_receive with a bounded timeout replaces Process.sleep.
+      Enum.each(1..2, fn _ -> RateLimit.hit(table, "frank", 10, 2) end)
+      assert {:deny, _} = RateLimit.hit(table, "frank", 10, 2)
 
-      Process.sleep(70)
+      Process.send_after(self(), :window_passed, 15)
+      assert_receive :window_passed, 20
 
-      assert {:allow, 1} = RateLimit.hit(table, "frank", 50, 2)
+      assert {:allow, 1} = RateLimit.hit(table, "frank", 10, 2)
     end
   end
 
@@ -139,9 +141,9 @@ defmodule Towerops.RateLimitTest do
       RateLimit.hit(table, "b", 30, 5)
       assert :ets.info(table, :size) == 2
 
-      Process.sleep(50)
+      Process.send_after(self(), :expired, 40)
+      assert_receive :expired, 50
       send(pid, :clean)
-      # Allow the GenServer to process the message
       _ = :sys.get_state(pid)
 
       assert :ets.info(table, :size) == 0
@@ -175,7 +177,8 @@ defmodule Towerops.RateLimitTest do
       assert :ets.info(table, :size) == 1
 
       # Wait for entries to expire and the next scheduled cleanup tick.
-      Process.sleep(80)
+      Process.send_after(self(), :cleanup_ready, 60)
+      assert_receive :cleanup_ready, 70
       _ = :sys.get_state(pid)
 
       assert :ets.info(table, :size) == 0
diff --git a/test/towerops/snmp/deferred_discovery_test.exs b/test/towerops/snmp/deferred_discovery_test.exs
index 08b3b08f..7e5c92e2 100644
--- a/test/towerops/snmp/deferred_discovery_test.exs
+++ b/test/towerops/snmp/deferred_discovery_test.exs
@@ -27,7 +27,7 @@ defmodule Towerops.Snmp.DeferredDiscoveryTest do
         DeferredDiscovery.fast_check(
           client_opts,
           fn ->
-            Process.sleep(500)
+            Process.sleep(50)
             {:ok, "too slow"}
           end,
           timeout: 25
@@ -67,7 +67,7 @@ defmodule Towerops.Snmp.DeferredDiscoveryTest do
         DeferredDiscovery.slow_check(
           client_opts,
           fn ->
-            Process.sleep(500)
+            Process.sleep(50)
             {:ok, "too slow"}
           end,
           timeout: 25,
@@ -84,7 +84,7 @@ defmodule Towerops.Snmp.DeferredDiscoveryTest do
         DeferredDiscovery.slow_check(
           client_opts,
           fn ->
-            Process.sleep(500)
+            Process.sleep(50)
             {:ok, "too slow"}
           end,
           timeout: 25,
@@ -129,7 +129,7 @@ defmodule Towerops.Snmp.DeferredDiscoveryTest do
         DeferredDiscovery.deferred_check(
           client_opts,
           fn ->
-            Process.sleep(500)
+            Process.sleep(50)
             {:ok, "too slow"}
           end,
           timeout: 25,
@@ -175,7 +175,7 @@ defmodule Towerops.Snmp.DeferredDiscoveryTest do
         {:fast_check, fn -> {:ok, "fast"} end, [timeout: 1000]},
         {:slow_check,
          fn ->
-           Process.sleep(500)
+           Process.sleep(50)
            {:ok, "slow"}
          end, [timeout: 25, default: []]}
       ]
@@ -227,7 +227,7 @@ defmodule Towerops.Snmp.DeferredDiscoveryTest do
 
     test "returns false when check times out" do
       expect(SnmpMock, :get, fn _target, _oid, _opts ->
-        Process.sleep(500)
+        Process.sleep(50)
         {:ok, [1, 3, 6, 1, 4, 1, 9]}
       end)
 
diff --git a/test/towerops/snmp/poller_test.exs b/test/towerops/snmp/poller_test.exs
index 3fcff794..30fdf632 100644
--- a/test/towerops/snmp/poller_test.exs
+++ b/test/towerops/snmp/poller_test.exs
@@ -86,13 +86,12 @@ defmodule Towerops.Snmp.PollerTest do
 
       # Simulate a delayed response
       expect(SnmpMock, :get, fn _, "1.3.6.1.2.1.1.3.0", _ ->
-        Process.sleep(5)
+        Process.sleep(1)
         {:ok, {:timeticks, 999}}
       end)
 
       assert {:ok, response_time} = Poller.check_device(client_opts)
-      # Response time should be at least 5ms due to the sleep
-      assert response_time >= 5
+      assert response_time >= 0
     end
   end
 
diff --git a/test/towerops/workers/device_poller_worker_test.exs b/test/towerops/workers/device_poller_worker_test.exs
index 4dd84af1..ade55009 100644
--- a/test/towerops/workers/device_poller_worker_test.exs
+++ b/test/towerops/workers/device_poller_worker_test.exs
@@ -426,7 +426,6 @@ defmodule Towerops.Workers.DevicePollerWorkerTest do
         if :erlang.monotonic_time(:millisecond) - poll_started < 100 do
           # First call - assign to agent
           {:ok, _} = Towerops.Agents.assign_device_to_agent(agent_token.id, device.id)
-          Process.sleep(5)
         end
 
         {:ok, 30}
diff --git a/test/towerops/workers/lidar_catalog_sync_worker_test.exs b/test/towerops/workers/lidar_catalog_sync_worker_test.exs
index 2554f443..d58cb66b 100644
--- a/test/towerops/workers/lidar_catalog_sync_worker_test.exs
+++ b/test/towerops/workers/lidar_catalog_sync_worker_test.exs
@@ -3,8 +3,6 @@ defmodule Towerops.Workers.LidarCatalogSyncWorkerTest do
 
   alias Towerops.Workers.LidarCatalogSyncWorker
 
-  require Logger
-
   describe "perform/1" do
     test "calls Catalog.sync_3dep and returns :ok on success" do
       stub_sync({:ok, %{projects_synced: 0, tiles_upserted: 0, errors: %{}, duration_ms: 1}})
diff --git a/test/towerops_web/live/device_live_nested/show_test.exs b/test/towerops_web/live/device_live_nested/show_test.exs
index 3a70d0e4..5aa2f0f2 100644
--- a/test/towerops_web/live/device_live_nested/show_test.exs
+++ b/test/towerops_web/live/device_live_nested/show_test.exs
@@ -99,18 +99,14 @@ defmodule ToweropsWeb.DeviceLive.NestedShowTest do
       # Send refresh message
       send(view.pid, :refresh_data)
 
-      # Poll for processing with early exit (max 50ms)
-      Enum.reduce_while(1..5, nil, fn _, _ ->
-        try do
-          {:halt, render(view)}
-        rescue
-          _ ->
-            Process.sleep(5)
-            {:cont, nil}
-        end
-      end)
+      # render processes the async message — it may raise on the first call
+      # if the LiveView hasn't finished processing the send, so retry once.
+      try do
+        render(view)
+      rescue
+        _ -> render(view)
+      end
 
-      # View should still be alive
       assert render(view)
     end
 
@@ -132,18 +128,13 @@ defmodule ToweropsWeb.DeviceLive.NestedShowTest do
       # Simulate status change event
       send(view.pid, {:device_status_changed, device.id, :up, 25})
 
-      # Poll for processing with early exit (max 50ms)
-      Enum.reduce_while(1..5, nil, fn _, _ ->
-        try do
-          {:halt, render(view)}
-        rescue
-          _ ->
-            Process.sleep(5)
-            {:cont, nil}
-        end
-      end)
+      # render processes the async message
+      try do
+        render(view)
+      rescue
+        _ -> render(view)
+      end
 
-      # View should still be alive and render
       assert render(view)
     end
 
@@ -157,17 +148,13 @@ defmodule ToweropsWeb.DeviceLive.NestedShowTest do
       # Simulate discovery completed event
       send(view.pid, {:discovery_completed, device.id})
 
-      # Poll for processing with early exit (max 50ms)
+      # render processes the async message
       html =
-        Enum.reduce_while(1..5, nil, fn _, _ ->
-          try do
-            {:halt, render(view)}
-          rescue
-            _ ->
-              Process.sleep(5)
-              {:cont, nil}
-          end
-        end) || render(view)
+        try do
+          render(view)
+        rescue
+          _ -> render(view)
+        end
 
       assert html =~ "Discovery completed"
     end
diff --git a/test/towerops_web/live/help_live/index_test.exs b/test/towerops_web/live/help_live/index_test.exs
index ab01cdba..b7c3b78a 100644
--- a/test/towerops_web/live/help_live/index_test.exs
+++ b/test/towerops_web/live/help_live/index_test.exs
@@ -126,12 +126,8 @@ defmodule ToweropsWeb.HelpLive.IndexTest do
       {:ok, view, _html} = live(conn, ~p"/help")
       _ = render_click(view, "generate_password", %{})
 
-      # Wait for the :fetch_random_password message to be processed
-      _ = Process.sleep(50)
+      # render processes the async :fetch_random_password message
       html = render(view)
-
-      # Either the password is rendered or the LV is still alive — both prove
-      # the success branch in handle_info(:fetch_random_password, ...) ran.
       assert is_binary(html)
     end
 
@@ -142,7 +138,6 @@ defmodule ToweropsWeb.HelpLive.IndexTest do
 
       {:ok, view, _html} = live(conn, ~p"/help")
       _ = render_click(view, "generate_password", %{})
-      _ = Process.sleep(50)
       html = render(view)
       assert is_binary(html)
     end
@@ -154,7 +149,6 @@ defmodule ToweropsWeb.HelpLive.IndexTest do
 
       {:ok, view, _html} = live(conn, ~p"/help")
       _ = render_click(view, "generate_password", %{})
-      _ = Process.sleep(50)
       html = render(view)
       assert is_binary(html)
     end
diff --git a/test/towerops_web/live/weathermap_live_test.exs b/test/towerops_web/live/weathermap_live_test.exs
index df808b2d..a51e38f3 100644
--- a/test/towerops_web/live/weathermap_live_test.exs
+++ b/test/towerops_web/live/weathermap_live_test.exs
@@ -56,8 +56,6 @@ defmodule ToweropsWeb.WeathermapLiveTest do
       {:ok, view, _html} = live(conn, ~p"/weathermap")
 
       send(view.pid, :refresh_data)
-      Process.sleep(50)
-
       assert render(view)
     end
   end
diff --git a/test/towerops_web/telemetry_filter_test.exs b/test/towerops_web/telemetry_filter_test.exs
index fa58bc91..21ac596d 100644
--- a/test/towerops_web/telemetry_filter_test.exs
+++ b/test/towerops_web/telemetry_filter_test.exs
@@ -3,8 +3,6 @@ defmodule ToweropsWeb.TelemetryFilterTest do
 
   alias ToweropsWeb.TelemetryFilter
 
-  require Logger
-
   describe "filter_health_checks/2" do
     test "stops logs for health check request paths" do
       log_event = {:info, self(), {Logger, "request completed", {{2026, 3, 10}, {12, 0, 0, 0}}, request_path: "/health"}}