From f5ff6c5c616ef044e04d5a7a7fa93bd19889fcf7 Mon Sep 17 00:00:00 2001
From: Graham McIntire <graham@mcintire.me>
Date: Tue, 20 Jan 2026 13:53:46 -0600
Subject: [PATCH] Add comprehensive tests for Towerops.Monitoring.DeviceMonitor

- Test start_link/1 and Horde Registry registration
- Test initial check on monitoring enabled/disabled
- Test trigger_check/1 for running and stopped monitors
- Test successful and failed ping handling
- Test status change detection (up->down, down->up)
- Test alert creation and resolution
- Test PubSub broadcasts for status changes and alerts
- Test alert message formatting for ICMP vs SNMP
- Test check scheduling based on check_interval_seconds
- Coverage improved from 43.93% to 96.67%

Total tests: 1066 -> 1089 (23 new tests)
Total coverage: 48.72% -> 49.65%
---
 .../monitoring/device_monitor_test.exs        | 471 ++++++++++++++++++
 1 file changed, 471 insertions(+)
 create mode 100644 test/towerops/monitoring/device_monitor_test.exs

diff --git a/test/towerops/monitoring/device_monitor_test.exs b/test/towerops/monitoring/device_monitor_test.exs
new file mode 100644
index 00000000..8c248c1a
--- /dev/null
+++ b/test/towerops/monitoring/device_monitor_test.exs
@@ -0,0 +1,471 @@
+defmodule Towerops.Monitoring.DeviceMonitorTest do
+  use Towerops.DataCase, async: false
+
+  import Mox
+  import Swoosh.TestAssertions
+  import Towerops.AccountsFixtures
+
+  alias Towerops.Alerts
+  alias Towerops.Devices
+  alias Towerops.Monitoring
+  alias Towerops.Monitoring.DeviceMonitor
+  alias Towerops.Monitoring.PingMock
+
+  setup :verify_on_exit!
+  setup :set_mox_global
+
+  setup do
+    # Use stub_with for GenServer tests since monitors may ping multiple times
+    stub_with(PingMock, Towerops.Monitoring.PingStub)
+
+    # Start the Monitoring.Supervisor for these tests
+    start_supervised!(Towerops.Monitoring.Supervisor)
+
+    # Create test data
+    user = user_fixture()
+    {:ok, organization} = Towerops.Organizations.create_organization(%{name: "Test Org"}, user.id)
+
+    {:ok, site} =
+      Towerops.Sites.create_site(%{
+        name: "Test Site",
+        organization_id: organization.id
+      })
+
+    {:ok, device} =
+      Devices.create_device(%{
+        name: "Test Router",
+        ip_address: "192.168.1.1",
+        site_id: site.id,
+        monitoring_enabled: true,
+        check_interval_seconds: 30
+      })
+
+    # Consume confirmation email from user creation
+    assert_email_sent(subject: "Confirmation instructions")
+
+    %{device: device, site: site, organization: organization}
+  end
+
+  describe "start_link/1" do
+    test "starts a monitor GenServer for a device", %{device: device} do
+      assert {:ok, pid} = DeviceMonitor.start_link(device.id)
+      assert is_pid(pid)
+      assert Process.alive?(pid)
+
+      # Cleanup
+      GenServer.stop(pid)
+    end
+
+    test "registers monitor in Horde Registry", %{device: device} do
+      {:ok, _pid} = DeviceMonitor.start_link(device.id)
+
+      assert [{pid, _}] = Horde.Registry.lookup(Towerops.Monitoring.Registry, device.id)
+      assert Process.alive?(pid)
+
+      # Cleanup
+      GenServer.stop(pid)
+    end
+  end
+
+  describe "init/1" do
+    test "performs immediate check when monitoring is enabled", %{device: device} do
+      {:ok, pid} = DeviceMonitor.start_link(device.id)
+
+      # Give it time to perform the initial check
+      Process.sleep(50)
+
+      # Verify check was created
+      checks = Monitoring.list_devices_checks(device.id)
+      assert length(checks) >= 1
+      assert hd(checks).status == :success
+      assert hd(checks).response_time_ms == 10
+
+      # Cleanup
+      GenServer.stop(pid)
+    end
+
+    test "does not check when monitoring is disabled", %{device: device} do
+      # Disable monitoring
+      {:ok, device} = Devices.update_device(device, %{monitoring_enabled: false})
+
+      {:ok, pid} = DeviceMonitor.start_link(device.id)
+      Process.sleep(50)
+
+      # Verify no checks were created
+      checks = Monitoring.list_devices_checks(device.id)
+      assert checks == []
+
+      # Cleanup
+      GenServer.stop(pid)
+    end
+  end
+
+  describe "trigger_check/1" do
+    test "triggers immediate check when monitor is running", %{device: device} do
+      {:ok, pid} = DeviceMonitor.start_link(device.id)
+      Process.sleep(50)
+
+      # Count initial checks
+      initial_count = length(Monitoring.list_devices_checks(device.id))
+
+      DeviceMonitor.trigger_check(device.id)
+      Process.sleep(50)
+
+      # Should have at least one more check
+      final_count = length(Monitoring.list_devices_checks(device.id))
+      assert final_count > initial_count
+
+      # Cleanup
+      GenServer.stop(pid)
+    end
+
+    test "enqueues check job when monitor is not running", %{device: device} do
+      # Disable monitoring so monitor doesn't start
+      {:ok, device} = Devices.update_device(device, %{monitoring_enabled: false})
+
+      DeviceMonitor.trigger_check(device.id)
+      Process.sleep(100)
+
+      # Verify check was created
+      checks = Monitoring.list_devices_checks(device.id)
+      assert length(checks) == 1
+      assert hd(checks).response_time_ms == 10
+    end
+  end
+
+  describe "perform_check/1 - successful ping" do
+    test "creates successful monitoring check", %{device: device} do
+      {:ok, pid} = DeviceMonitor.start_link(device.id)
+      Process.sleep(50)
+
+      check = Monitoring.get_latest_check(device.id)
+      assert check.status == :success
+      assert check.response_time_ms == 10
+      assert check.device_id == device.id
+
+      # Cleanup
+      GenServer.stop(pid)
+    end
+
+    test "updates device status to :up", %{device: device} do
+      # Start with device down
+      {:ok, device} = Devices.update_device_status(device, :down)
+      assert device.status == :down
+
+      {:ok, pid} = DeviceMonitor.start_link(device.id)
+      Process.sleep(50)
+
+      # Device should be up now
+      device = Devices.get_device!(device.id)
+      assert device.status == :up
+
+      # Cleanup
+      GenServer.stop(pid)
+    end
+
+    test "broadcasts device status change via PubSub", %{device: device} do
+      # Subscribe to device status changes
+      Phoenix.PubSub.subscribe(Towerops.PubSub, "device:#{device.id}")
+
+      {:ok, pid} = DeviceMonitor.start_link(device.id)
+
+      # Wait for broadcast
+      assert_receive {:device_status_changed, device_id, :up, nil}, 200
+      assert device_id == device.id
+
+      # Cleanup
+      GenServer.stop(pid)
+    end
+  end
+
+  describe "perform_check/1 - failed ping" do
+    setup do
+      # Override stub to return errors for this describe block
+      stub(PingMock, :ping, fn _ -> {:error, :timeout} end)
+      :ok
+    end
+
+    test "creates failed monitoring check", %{device: device} do
+      {:ok, pid} = DeviceMonitor.start_link(device.id)
+      Process.sleep(50)
+
+      check = Monitoring.get_latest_check(device.id)
+      assert check.status == :failure
+      assert is_nil(check.response_time_ms)
+
+      # Cleanup
+      GenServer.stop(pid)
+    end
+
+    test "updates device status to :down", %{device: device} do
+      # Start with device up
+      {:ok, device} = Devices.update_device_status(device, :up)
+      assert device.status == :up
+
+      {:ok, pid} = DeviceMonitor.start_link(device.id)
+      Process.sleep(50)
+
+      # Device should be down now
+      device = Devices.get_device!(device.id)
+      assert device.status == :down
+
+      # Cleanup
+      GenServer.stop(pid)
+    end
+  end
+
+  describe "status change - up to down" do
+    setup do
+      stub(PingMock, :ping, fn _ -> {:error, :timeout} end)
+      :ok
+    end
+
+    test "creates device_down alert", %{device: device} do
+      # Start with device up
+      {:ok, device} = Devices.update_device_status(device, :up)
+
+      {:ok, pid} = DeviceMonitor.start_link(device.id)
+      Process.sleep(50)
+
+      # Check alert was created
+      assert Alerts.has_active_alert?(device.id, :device_down)
+      alert = Alerts.get_active_alert(device.id, :device_down)
+      assert alert.alert_type == :device_down
+      assert alert.message =~ "not responding"
+
+      # Cleanup
+      GenServer.stop(pid)
+    end
+
+    test "broadcasts new_alert via PubSub", %{device: device} do
+      {:ok, _device} = Devices.update_device_status(device, :up)
+
+      # Subscribe to alerts
+      Phoenix.PubSub.subscribe(Towerops.PubSub, "alerts:new")
+
+      {:ok, pid} = DeviceMonitor.start_link(device.id)
+
+      # Wait for broadcast
+      assert_receive {:new_alert, device_id, :device_down}, 200
+      assert device_id == device.id
+
+      # Cleanup
+      GenServer.stop(pid)
+    end
+
+    test "does not create duplicate alerts when already down", %{device: device} do
+      {:ok, device} = Devices.update_device_status(device, :up)
+
+      {:ok, pid} = DeviceMonitor.start_link(device.id)
+      Process.sleep(50)
+
+      # Second check - should not create duplicate
+      GenServer.cast(pid, :check_now)
+      Process.sleep(50)
+
+      # Only one active alert should exist
+      alerts = Alerts.list_devices_alerts(device.id)
+      down_alerts = Enum.filter(alerts, &(&1.alert_type == :device_down and is_nil(&1.resolved_at)))
+      assert length(down_alerts) == 1
+
+      # Cleanup
+      GenServer.stop(pid)
+    end
+  end
+
+  describe "status change - down to up" do
+    test "creates device_up alert", %{device: device} do
+      # Start with device down and create down alert
+      {:ok, device} = Devices.update_device_status(device, :down)
+
+      {:ok, _alert} =
+        Alerts.create_alert(%{
+          device_id: device.id,
+          alert_type: :device_down,
+          triggered_at: DateTime.utc_now(),
+          message: "Device is down"
+        })
+
+      {:ok, pid} = DeviceMonitor.start_link(device.id)
+      Process.sleep(50)
+
+      # Check recovery alert was created
+      alerts = Alerts.list_devices_alerts(device.id)
+      up_alerts = Enum.filter(alerts, &(&1.alert_type == :device_up))
+      assert length(up_alerts) >= 1
+      up_alert = hd(up_alerts)
+      assert up_alert.message =~ "now responding"
+
+      # Cleanup
+      GenServer.stop(pid)
+    end
+
+    test "resolves existing device_down alert", %{device: device} do
+      {:ok, device} = Devices.update_device_status(device, :down)
+
+      # Create down alert
+      {:ok, down_alert} =
+        Alerts.create_alert(%{
+          device_id: device.id,
+          alert_type: :device_down,
+          triggered_at: DateTime.utc_now(),
+          message: "Device is down"
+        })
+
+      {:ok, pid} = DeviceMonitor.start_link(device.id)
+      Process.sleep(50)
+
+      # Down alert should be resolved
+      refute Alerts.has_active_alert?(device.id, :device_down)
+      resolved_alert = Alerts.get_alert!(down_alert.id)
+      assert resolved_alert.resolved_at
+
+      # Cleanup
+      GenServer.stop(pid)
+    end
+
+    test "broadcasts alert_resolved via PubSub", %{device: device} do
+      {:ok, _device} = Devices.update_device_status(device, :down)
+
+      # Create existing down alert
+      {:ok, _alert} =
+        Alerts.create_alert(%{
+          device_id: device.id,
+          alert_type: :device_down,
+          triggered_at: DateTime.utc_now(),
+          message: "Device is down"
+        })
+
+      # Subscribe to resolved alerts
+      Phoenix.PubSub.subscribe(Towerops.PubSub, "alerts:resolved")
+
+      {:ok, pid} = DeviceMonitor.start_link(device.id)
+
+      # Wait for broadcast
+      assert_receive {:alert_resolved, device_id, :device_down}, 200
+      assert device_id == device.id
+
+      # Cleanup
+      GenServer.stop(pid)
+    end
+  end
+
+  describe "alert messages" do
+    setup do
+      stub(PingMock, :ping, fn _ -> {:error, :timeout} end)
+      :ok
+    end
+
+    test "uses ICMP message when SNMP is disabled", %{device: device} do
+      {:ok, device} = Devices.update_device(device, %{snmp_enabled: false})
+      {:ok, _device} = Devices.update_device_status(device, :up)
+
+      {:ok, pid} = DeviceMonitor.start_link(device.id)
+      Process.sleep(50)
+
+      alert = Alerts.get_active_alert(device.id, :device_down)
+      assert alert.message == "Device is not responding to ping"
+
+      # Cleanup
+      GenServer.stop(pid)
+    end
+
+    test "uses SNMP message when SNMP is enabled", %{device: device} do
+      {:ok, device} = Devices.update_device(device, %{snmp_enabled: true})
+      {:ok, _device} = Devices.update_device_status(device, :up)
+
+      {:ok, pid} = DeviceMonitor.start_link(device.id)
+      Process.sleep(50)
+
+      alert = Alerts.get_active_alert(device.id, :device_down)
+      assert alert.message == "Device is not responding to SNMP"
+
+      # Cleanup
+      GenServer.stop(pid)
+    end
+  end
+
+  describe "alert messages - recovery" do
+    test "recovery message mentions ICMP when SNMP disabled", %{device: device} do
+      {:ok, device} = Devices.update_device(device, %{snmp_enabled: false})
+      {:ok, device} = Devices.update_device_status(device, :down)
+
+      {:ok, _alert} =
+        Alerts.create_alert(%{
+          device_id: device.id,
+          alert_type: :device_down,
+          triggered_at: DateTime.utc_now(),
+          message: "Device is down"
+        })
+
+      {:ok, pid} = DeviceMonitor.start_link(device.id)
+      Process.sleep(50)
+
+      alerts = Alerts.list_devices_alerts(device.id)
+      up_alert = Enum.find(alerts, &(&1.alert_type == :device_up))
+      assert up_alert.message == "Device is now responding to ping"
+
+      # Cleanup
+      GenServer.stop(pid)
+    end
+
+    test "recovery message mentions SNMP when SNMP enabled", %{device: device} do
+      {:ok, device} = Devices.update_device(device, %{snmp_enabled: true})
+      {:ok, device} = Devices.update_device_status(device, :down)
+
+      {:ok, _alert} =
+        Alerts.create_alert(%{
+          device_id: device.id,
+          alert_type: :device_down,
+          triggered_at: DateTime.utc_now(),
+          message: "Device is down"
+        })
+
+      {:ok, pid} = DeviceMonitor.start_link(device.id)
+      Process.sleep(50)
+
+      alerts = Alerts.list_devices_alerts(device.id)
+      up_alert = Enum.find(alerts, &(&1.alert_type == :device_up))
+      assert up_alert.message == "Device is now responding to SNMP"
+
+      # Cleanup
+      GenServer.stop(pid)
+    end
+  end
+
+  describe "scheduling" do
+    test "schedules next check based on check_interval_seconds", %{device: device} do
+      # Set short interval for testing
+      {:ok, device} = Devices.update_device(device, %{check_interval_seconds: 1})
+
+      {:ok, pid} = DeviceMonitor.start_link(device.id)
+
+      # Wait for initial check
+      Process.sleep(50)
+
+      # Wait for scheduled check (1 second interval)
+      Process.sleep(1100)
+
+      # Should have at least 2 checks
+      checks = Monitoring.list_devices_checks(device.id, 10)
+      assert length(checks) >= 2
+
+      # Cleanup
+      GenServer.stop(pid)
+    end
+
+    test "does not schedule next check when monitoring disabled", %{device: device} do
+      {:ok, device} = Devices.update_device(device, %{monitoring_enabled: false})
+
+      {:ok, pid} = DeviceMonitor.start_link(device.id)
+      Process.sleep(50)
+
+      # No checks should exist
+      checks = Monitoring.list_devices_checks(device.id)
+      assert checks == []
+
+      # Cleanup
+      GenServer.stop(pid)
+    end
+  end
+end