diff --git a/lib/towerops/workers/monitor_worker.ex b/lib/towerops/workers/monitor_worker.ex index 34df0627..d538b197 100644 --- a/lib/towerops/workers/monitor_worker.ex +++ b/lib/towerops/workers/monitor_worker.ex @@ -14,6 +14,10 @@ defmodule Towerops.Workers.MonitorWorker do require Logger + defp ping_adapter do + Application.get_env(:towerops, :ping_module, Towerops.Monitoring.Ping) + end + @doc """ Performs a monitoring check for a device. @@ -84,33 +88,7 @@ defmodule Towerops.Workers.MonitorWorker do end defp ping_device(device) do - # Use system ping command (works on most Unix-like systems) - case System.cmd("ping", ["-c", "1", "-W", "1", device.ip_address], stderr_to_stdout: true) do - {output, 0} -> - # Extract latency from ping output - latency = extract_latency(output) - {:ok, latency} - - {_output, _status} -> - {:error, :timeout} - end - rescue - error -> - {:error, error} - end - - defp extract_latency(output) do - # Parse ping output to extract latency - # Example: "time=1.234 ms" - case Regex.run(~r/time=([\d.]+)/, output) do - [_, latency_str] -> - case Float.parse(latency_str) do - {latency, _} -> latency - :error -> 0.0 - end - - nil -> - 0.0 - end + # Use configured ping adapter (mockable in tests) + ping_adapter().ping(device.ip_address, 1000) end end diff --git a/test/towerops/monitoring/ping_test.exs b/test/towerops/monitoring/ping_test.exs index f0ac311f..c27fee40 100644 --- a/test/towerops/monitoring/ping_test.exs +++ b/test/towerops/monitoring/ping_test.exs @@ -36,8 +36,9 @@ defmodule Towerops.Monitoring.PingTest do assert match?({:ok, _}, result) or match?({:error, _}, result) end + @tag :integration test "ping/1 uses default timeout" do - # Just verify it accepts single argument + # Just verify it accepts single argument - does real ICMP ping result = Ping.ping("127.0.0.1") assert match?({:ok, _}, result) or match?({:error, _}, result) end @@ -74,8 +75,9 @@ defmodule Towerops.Monitoring.PingTest do end describe "response time calculation" do + @tag :integration test "returns response time as float in milliseconds on success" do - # This test will only pass if we have privileges + # This test will only pass if we have privileges - does real ICMP ping case Ping.ping("127.0.0.1", 5000) do {:ok, response_time} -> assert is_float(response_time) @@ -166,7 +168,9 @@ defmodule Towerops.Monitoring.PingTest do assert function_exported?(Ping, :ping, 2) end + @tag :integration test "returns correct tuple format from ping/1" do + # Does real ICMP ping result = Ping.ping("127.0.0.1") case result do @@ -178,7 +182,9 @@ defmodule Towerops.Monitoring.PingTest do end end + @tag :integration test "returns correct tuple format from ping/2" do + # Does real ICMP ping result = Ping.ping("127.0.0.1", 1000) case result do diff --git a/test/towerops/workers/monitor_worker_test.exs b/test/towerops/workers/monitor_worker_test.exs index b2136fbe..98ceb171 100644 --- a/test/towerops/workers/monitor_worker_test.exs +++ b/test/towerops/workers/monitor_worker_test.exs @@ -1,11 +1,15 @@ defmodule Towerops.Workers.MonitorWorkerTest do use Towerops.DataCase, async: true + import Mox import Towerops.AccountsFixtures alias Towerops.Monitoring + alias Towerops.Monitoring.PingMock alias Towerops.Workers.MonitorWorker + setup :verify_on_exit! + setup do user = user_fixture() {:ok, organization} = Towerops.Organizations.create_organization(%{name: "Test Org"}, user.id) @@ -30,28 +34,25 @@ defmodule Towerops.Workers.MonitorWorkerTest do describe "perform/1" do test "creates a check when monitoring is enabled", %{device: device} do - # Call perform - it will ping localhost + # Mock successful ping + expect(PingMock, :ping, fn _ip, _timeout -> + {:ok, 10.5} + end) + result = MonitorWorker.perform(device.id) - # Should return :ok regardless of ping success/failure + # Should return :ok assert result == :ok - # Verify a check was created + # Verify a successful check was created checks = Monitoring.list_devices_checks(device.id, 10) assert length(checks) == 1 check = hd(checks) assert check.device_id == device.id - assert check.status in [:success, :failure] + assert check.status == :success assert check.checked_at - - # If ping succeeded, should have response time - if check.status == :success do - assert is_float(check.response_time_ms) - assert check.response_time_ms >= 0 - else - assert check.response_time_ms == nil - end + assert check.response_time_ms == 10.5 end test "skips check when monitoring is disabled", %{device: device} do @@ -72,6 +73,11 @@ defmodule Towerops.Workers.MonitorWorkerTest do end test "broadcasts update after check", %{device: device} do + # Mock successful ping + expect(PingMock, :ping, fn _ip, _timeout -> + {:ok, 10.5} + end) + # Subscribe to device topic Phoenix.PubSub.subscribe(Towerops.PubSub, "device:#{device.id}") @@ -92,7 +98,6 @@ defmodule Towerops.Workers.MonitorWorkerTest do organization_id: organization.id }) - # Use a non-routable IP (documentation range) {:ok, device} = Towerops.Devices.create_device(%{ name: "Unreachable Device", @@ -101,6 +106,11 @@ defmodule Towerops.Workers.MonitorWorkerTest do site_id: site.id }) + # Mock failed ping + expect(PingMock, :ping, fn _ip, _timeout -> + {:error, :timeout} + end) + assert :ok = MonitorWorker.perform(device.id) # Verify check was created as failure @@ -113,16 +123,22 @@ defmodule Towerops.Workers.MonitorWorkerTest do end test "successfully completes monitoring check", %{device: device} do + # Mock successful ping + expect(PingMock, :ping, fn _ip, _timeout -> + {:ok, 15.0} + end) + assert :ok = MonitorWorker.perform(device.id) - # Verify a check was created + # Verify a successful check was created checks = Monitoring.list_devices_checks(device.id, 10) assert length(checks) >= 1 check = hd(checks) assert check.device_id == device.id - assert check.status in [:success, :failure] + assert check.status == :success assert check.checked_at + assert check.response_time_ms == 15.0 end test "correctly skips monitoring when disabled", %{device: device} do