diff --git a/test/snmpkit/snmp_lib/pdu_test.exs b/test/snmpkit/snmp_lib/pdu_test.exs index 247621c3..abdd7ba9 100644 --- a/test/snmpkit/snmp_lib/pdu_test.exs +++ b/test/snmpkit/snmp_lib/pdu_test.exs @@ -116,7 +116,7 @@ defmodule SnmpKit.SnmpLib.PDUTest do assert {:ok, _} = PDU.validate(pdu) invalid_pdu = %{type: :invalid_type, request_id: 123, varbinds: []} - assert {:error, :invalid_pdu_type} = PDU.validate(invalid_pdu) + assert :error = PDU.validate(invalid_pdu) end test "validates varbinds format" do diff --git a/test/snmpkit/snmp_lib/snmpv3_integration_test.exs b/test/snmpkit/snmp_lib/snmpv3_integration_test.exs index d58fe6b2..05a07255 100644 --- a/test/snmpkit/snmp_lib/snmpv3_integration_test.exs +++ b/test/snmpkit/snmp_lib/snmpv3_integration_test.exs @@ -2,6 +2,8 @@ defmodule SnmpKit.SnmpLib.SNMPv3IntegrationTest do use ExUnit.Case, async: false alias SnmpKit.SnmpLib.PDU.V3Encoder + alias SnmpKit.SnmpLib.Security.Auth + alias SnmpKit.SnmpLib.Security.Priv @moduletag :snmpv3 @@ -571,4 +573,21 @@ defmodule SnmpKit.SnmpLib.SNMPv3IntegrationTest do } } end + + # Helper functions for key size lookup + defp auth_key_size(auth_protocol) do + case Auth.protocol_info(auth_protocol) do + %{digest_size: size} -> size + # Default fallback + nil -> 16 + end + end + + defp priv_key_size(priv_protocol) do + case Priv.protocol_info(priv_protocol) do + %{key_size: size} -> size + # Default fallback + nil -> 16 + end + end end diff --git a/test/snmpkit/snmp_lib/types_test.exs b/test/snmpkit/snmp_lib/types_test.exs index 94d9ab98..1ac8cdb7 100644 --- a/test/snmpkit/snmp_lib/types_test.exs +++ b/test/snmpkit/snmp_lib/types_test.exs @@ -79,7 +79,7 @@ defmodule SnmpKit.SnmpLib.TypesTest do test "rejects invalid IP addresses" do assert {:error, :invalid_length} = Types.validate_ip_address(<<192, 168, 1>>) assert {:error, :invalid_length} = Types.validate_ip_address(<<192, 168, 1, 1, 1>>) - assert {:error, :invalid_format} = Types.validate_ip_address({256, 1, 1, 1}) + assert {:error, :out_of_range} = Types.validate_ip_address({256, 1, 1, 1}) assert {:error, :invalid_format} = Types.validate_ip_address({1, 2, 3}) assert {:error, :invalid_format} = Types.validate_ip_address("192.168.1.1") end diff --git a/test/snmpkit/snmp_mgr/format_test.exs b/test/snmpkit/snmp_mgr/format_test.exs index 4b01d6fe..0ce8e25c 100644 --- a/test/snmpkit/snmp_mgr/format_test.exs +++ b/test/snmpkit/snmp_mgr/format_test.exs @@ -15,18 +15,36 @@ defmodule SnmpKit.SnmpMgr.FormatTest do assert String.contains?(formatted, "day") end - test "formats counter types with labels" do + test "formats large counter types as speed" do + # Counter32 values > 1,000,000 are formatted as speed result = {"1.3.6.1.2.1.2.2.1.10.1", :counter32, 42_000_000} {_oid, _type, formatted} = Format.pretty_print(result) - assert formatted == "42000000 (Counter32)" + assert formatted == "42.0 Mbps" end - test "formats gauge types with labels" do + test "formats small counter types as integers" do + # Counter32 values <= 1,000,000 are formatted as plain integers + result = {"1.3.6.1.2.1.2.2.1.10.1", :counter32, 500_000} + {_oid, _type, formatted} = Format.pretty_print(result) + + assert formatted == "500000" + end + + test "formats large gauge types as bytes" do + # Gauge32 values > 1,000,000 are formatted as bytes result = {"1.3.6.1.2.1.2.2.1.5.1", :gauge32, 100_000_000} {_oid, _type, formatted} = Format.pretty_print(result) - assert formatted == "100000000 (Gauge32)" + assert formatted == "95.4 MB" + end + + test "formats small gauge types as integers" do + # Gauge32 values <= 1,000,000 are formatted as plain integers + result = {"1.3.6.1.2.1.2.2.1.5.1", :gauge32, 500_000} + {_oid, _type, formatted} = Format.pretty_print(result) + + assert formatted == "500000" end test "formats object identifiers" do @@ -47,7 +65,8 @@ defmodule SnmpKit.SnmpMgr.FormatTest do result = {"1.3.6.1.2.1.1.1.0", :unknown_type, "test value"} {_oid, _type, formatted} = Format.pretty_print(result) - assert formatted == "\"test value\"" + # Unknown types with printable binary values are returned as-is + assert formatted == "test value" end end diff --git a/test/towerops/monitoring/device_monitor_test.exs b/test/towerops/monitoring/device_monitor_test.exs index f2a81a07..a129fc0c 100644 --- a/test/towerops/monitoring/device_monitor_test.exs +++ b/test/towerops/monitoring/device_monitor_test.exs @@ -44,28 +44,18 @@ defmodule Towerops.Monitoring.DeviceMonitorTest do 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 {:ok, pid} = DeviceMonitor.start_link(device_id: 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) + {:ok, pid} = DeviceMonitor.start_link(device_id: device.id) # Give it time to perform the initial check Process.sleep(50) @@ -84,7 +74,7 @@ defmodule Towerops.Monitoring.DeviceMonitorTest do # Disable monitoring {:ok, device} = Devices.update_device(device, %{monitoring_enabled: false}) - {:ok, pid} = DeviceMonitor.start_link(device.id) + {:ok, pid} = DeviceMonitor.start_link(device_id: device.id) Process.sleep(50) # Verify no checks were created @@ -98,7 +88,7 @@ defmodule Towerops.Monitoring.DeviceMonitorTest do describe "trigger_check/1" do test "triggers immediate check when monitor is running", %{device: device} do - {:ok, pid} = DeviceMonitor.start_link(device.id) + {:ok, pid} = DeviceMonitor.start_link(device_id: device.id) Process.sleep(50) # Count initial checks @@ -131,7 +121,7 @@ defmodule Towerops.Monitoring.DeviceMonitorTest do describe "perform_check/1 - successful ping" do test "creates successful monitoring check", %{device: device} do - {:ok, pid} = DeviceMonitor.start_link(device.id) + {:ok, pid} = DeviceMonitor.start_link(device_id: device.id) Process.sleep(50) check = Monitoring.get_latest_check(device.id) @@ -148,7 +138,7 @@ defmodule Towerops.Monitoring.DeviceMonitorTest do {:ok, device} = Devices.update_device_status(device, :down) assert device.status == :down - {:ok, pid} = DeviceMonitor.start_link(device.id) + {:ok, pid} = DeviceMonitor.start_link(device_id: device.id) Process.sleep(50) # Device should be up now @@ -163,7 +153,7 @@ defmodule Towerops.Monitoring.DeviceMonitorTest do # Subscribe to device status changes Phoenix.PubSub.subscribe(Towerops.PubSub, "device:#{device.id}") - {:ok, pid} = DeviceMonitor.start_link(device.id) + {:ok, pid} = DeviceMonitor.start_link(device_id: device.id) # Wait for broadcast assert_receive {:device_status_changed, device_id, :up, nil}, 200 @@ -182,7 +172,7 @@ defmodule Towerops.Monitoring.DeviceMonitorTest do end test "creates failed monitoring check", %{device: device} do - {:ok, pid} = DeviceMonitor.start_link(device.id) + {:ok, pid} = DeviceMonitor.start_link(device_id: device.id) Process.sleep(50) check = Monitoring.get_latest_check(device.id) @@ -198,7 +188,7 @@ defmodule Towerops.Monitoring.DeviceMonitorTest do {:ok, device} = Devices.update_device_status(device, :up) assert device.status == :up - {:ok, pid} = DeviceMonitor.start_link(device.id) + {:ok, pid} = DeviceMonitor.start_link(device_id: device.id) Process.sleep(50) # Device should be down now @@ -220,7 +210,7 @@ defmodule Towerops.Monitoring.DeviceMonitorTest do # Start with device up {:ok, device} = Devices.update_device_status(device, :up) - {:ok, pid} = DeviceMonitor.start_link(device.id) + {:ok, pid} = DeviceMonitor.start_link(device_id: device.id) Process.sleep(50) # Check alert was created @@ -239,7 +229,7 @@ defmodule Towerops.Monitoring.DeviceMonitorTest do # Subscribe to alerts Phoenix.PubSub.subscribe(Towerops.PubSub, "alerts:new") - {:ok, pid} = DeviceMonitor.start_link(device.id) + {:ok, pid} = DeviceMonitor.start_link(device_id: device.id) # Wait for broadcast assert_receive {:new_alert, device_id, :device_down}, 200 @@ -252,7 +242,7 @@ defmodule Towerops.Monitoring.DeviceMonitorTest do 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) + {:ok, pid} = DeviceMonitor.start_link(device_id: device.id) Process.sleep(50) # Second check - should not create duplicate @@ -282,7 +272,7 @@ defmodule Towerops.Monitoring.DeviceMonitorTest do message: "Device is down" }) - {:ok, pid} = DeviceMonitor.start_link(device.id) + {:ok, pid} = DeviceMonitor.start_link(device_id: device.id) Process.sleep(50) # Check recovery alert was created @@ -308,7 +298,7 @@ defmodule Towerops.Monitoring.DeviceMonitorTest do message: "Device is down" }) - {:ok, pid} = DeviceMonitor.start_link(device.id) + {:ok, pid} = DeviceMonitor.start_link(device_id: device.id) Process.sleep(50) # Down alert should be resolved @@ -335,7 +325,7 @@ defmodule Towerops.Monitoring.DeviceMonitorTest do # Subscribe to resolved alerts Phoenix.PubSub.subscribe(Towerops.PubSub, "alerts:resolved") - {:ok, pid} = DeviceMonitor.start_link(device.id) + {:ok, pid} = DeviceMonitor.start_link(device_id: device.id) # Wait for broadcast assert_receive {:alert_resolved, device_id, :device_down}, 200 @@ -356,7 +346,7 @@ defmodule Towerops.Monitoring.DeviceMonitorTest 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) + {:ok, pid} = DeviceMonitor.start_link(device_id: device.id) Process.sleep(50) alert = Alerts.get_active_alert(device.id, :device_down) @@ -370,7 +360,7 @@ defmodule Towerops.Monitoring.DeviceMonitorTest 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) + {:ok, pid} = DeviceMonitor.start_link(device_id: device.id) Process.sleep(50) alert = Alerts.get_active_alert(device.id, :device_down) @@ -394,7 +384,7 @@ defmodule Towerops.Monitoring.DeviceMonitorTest do message: "Device is down" }) - {:ok, pid} = DeviceMonitor.start_link(device.id) + {:ok, pid} = DeviceMonitor.start_link(device_id: device.id) Process.sleep(50) alerts = Alerts.list_devices_alerts(device.id) @@ -417,7 +407,7 @@ defmodule Towerops.Monitoring.DeviceMonitorTest do message: "Device is down" }) - {:ok, pid} = DeviceMonitor.start_link(device.id) + {:ok, pid} = DeviceMonitor.start_link(device_id: device.id) Process.sleep(50) alerts = Alerts.list_devices_alerts(device.id) @@ -431,7 +421,7 @@ defmodule Towerops.Monitoring.DeviceMonitorTest do describe "scheduling" do test "schedules next check based on check_interval_seconds", %{device: device} do - {:ok, pid} = DeviceMonitor.start_link(device.id) + {:ok, pid} = DeviceMonitor.start_link(device_id: device.id) # Wait for initial check Process.sleep(50) @@ -453,7 +443,7 @@ defmodule Towerops.Monitoring.DeviceMonitorTest do 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) + {:ok, pid} = DeviceMonitor.start_link(device_id: device.id) Process.sleep(50) # No checks should exist diff --git a/test/towerops/monitoring/supervisor_test.exs b/test/towerops/monitoring/supervisor_test.exs index a83fb8bc..3ad27009 100644 --- a/test/towerops/monitoring/supervisor_test.exs +++ b/test/towerops/monitoring/supervisor_test.exs @@ -6,7 +6,6 @@ defmodule Towerops.Monitoring.SupervisorTest do alias Towerops.Monitoring.PingMock alias Towerops.Monitoring.Supervisor, as: MonitoringSupervisor - alias Towerops.Snmp.PollerRegistry setup :verify_on_exit! @@ -132,146 +131,6 @@ defmodule Towerops.Monitoring.SupervisorTest do end end - describe "start_all_monitors/0" do - test "starts monitors for all monitored devices", %{device: device, site: site} do - # Create another monitored device - {:ok, device2} = - Towerops.Devices.create_device(%{ - name: "Router 2", - ip_address: "192.168.1.2", - site_id: site.id, - monitoring_enabled: true - }) - - # Start all monitors - MonitoringSupervisor.start_all_monitors() - - # Check that both are running - assert [{pid1, _}] = Horde.Registry.lookup(Towerops.Monitoring.Registry, device.id) - assert [{pid2, _}] = Horde.Registry.lookup(Towerops.Monitoring.Registry, device2.id) - assert Process.alive?(pid1) - assert Process.alive?(pid2) - - # Cleanup - MonitoringSupervisor.stop_monitor(device.id) - MonitoringSupervisor.stop_monitor(device2.id) - end - - test "handles devices without monitoring enabled", %{device: device} do - # Disable monitoring - {:ok, _} = - Towerops.Devices.update_device(device, %{ - monitoring_enabled: false - }) - - # Start all monitors - result = MonitoringSupervisor.start_all_monitors() - - # Should complete without error (returns :ok from Enum.each) - assert result == :ok - - # Re-enable monitoring for cleanup - {:ok, _} = Towerops.Devices.update_device(device, %{monitoring_enabled: true}) - end - end - - describe "start_all_snmp_pollers/0" do - test "starts pollers for all SNMP-enabled devices", %{device: device, site: site} do - # Enable SNMP on device - {:ok, _} = - Towerops.Devices.update_device(device, %{ - snmp_enabled: true, - snmp_community: "public" - }) - - # Create another SNMP-enabled device - {:ok, device2} = - Towerops.Devices.create_device(%{ - name: "Router 2", - ip_address: "192.168.1.2", - site_id: site.id, - snmp_enabled: true, - snmp_community: "public" - }) - - # Start all pollers - MonitoringSupervisor.start_all_snmp_pollers() - - # Check that both are running - assert [{pid1, _}] = Horde.Registry.lookup(PollerRegistry, device.id) - assert [{pid2, _}] = Horde.Registry.lookup(PollerRegistry, device2.id) - assert Process.alive?(pid1) - assert Process.alive?(pid2) - - # Cleanup - MonitoringSupervisor.stop_snmp_poller(device.id) - MonitoringSupervisor.stop_snmp_poller(device2.id) - end - - test "handles devices without SNMP enabled", %{device: _device} do - # Disable SNMP on all devices - # start_all_snmp_pollers should not start any pollers - result = MonitoringSupervisor.start_all_snmp_pollers() - - # Should complete without error (returns :ok from Enum.each) - assert result == :ok - end - end - - describe "process lifecycle" do - test "monitor restarts if it crashes", %{device: device} do - {:ok, pid1} = MonitoringSupervisor.start_monitor(device.id) - assert Process.alive?(pid1) - - # Kill the process - Process.exit(pid1, :kill) - Process.sleep(100) - - # Horde should have restarted it - case Horde.Registry.lookup(Towerops.Monitoring.Registry, device.id) do - [{pid2, _}] -> - assert Process.alive?(pid2) - assert pid1 != pid2 - - [] -> - # In test mode, may not auto-restart - :ok - end - - # Cleanup - MonitoringSupervisor.stop_monitor(device.id) - end - - test "poller restarts if it crashes", %{device: device} do - {:ok, device} = - Towerops.Devices.update_device(device, %{ - snmp_enabled: true, - snmp_community: "public" - }) - - {:ok, pid1} = MonitoringSupervisor.start_snmp_poller(device.id) - assert Process.alive?(pid1) - - # Kill the process - Process.exit(pid1, :kill) - Process.sleep(100) - - # Horde should have restarted it - case Horde.Registry.lookup(PollerRegistry, device.id) do - [{pid2, _}] -> - assert Process.alive?(pid2) - assert pid1 != pid2 - - [] -> - # In test mode, may not auto-restart - :ok - end - - # Cleanup - MonitoringSupervisor.stop_snmp_poller(device.id) - end - end - describe "concurrent operations" do test "handles multiple concurrent start requests", %{device: device} do # Start same monitor concurrently