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
This commit is contained in:
Graham McIntire 2026-06-02 15:16:53 -05:00
parent 075e779b19
commit cd19e6626c
15 changed files with 80 additions and 124 deletions

View file

@ -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

View file

@ -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!()

View file

@ -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

View file

@ -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)

View file

@ -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

View file

@ -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)
_ ->

View file

@ -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

View file

@ -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)

View file

@ -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

View file

@ -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}

View file

@ -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}})

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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"}}