From 5ccda65dd1c68dd88a954ad086a0dafd62604e82 Mon Sep 17 00:00:00 2001
From: Graham McIntire <graham@mcintire.me>
Date: Tue, 20 Jan 2026 14:02:45 -0600
Subject: [PATCH] Add comprehensive tests for Towerops.Snmp context
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Added 18 new tests covering:
- get_latest_sensor_readings_batch/1 (batch operations)
- get_latest_interface_stats_batch/1 (batch operations)
- list_neighbors/1 (neighbor queries with ordering)
- get_neighbor/1 (single neighbor retrieval)
- upsert_neighbor/1 (create and update operations)
- delete_stale_neighbors/2 (time-based cleanup)

Tests verify:
- Batch operations match individual queries
- Empty input handling
- Latest record selection
- Ordering by protocol and name
- Stale data cleanup across devices
- Update semantics for existing records

Coverage improvement:
- Towerops.Snmp: 63.33% → 73.33% (+10%)
- Total tests: 1089 → 1106 (+17 tests)
- Overall coverage: 49.69% → 49.82%
---
 test/towerops/snmp_test.exs | 561 ++++++++++++++++++++++++++++++++++++
 1 file changed, 561 insertions(+)

diff --git a/test/towerops/snmp_test.exs b/test/towerops/snmp_test.exs
index 4d7739cc..1499cd67 100644
--- a/test/towerops/snmp_test.exs
+++ b/test/towerops/snmp_test.exs
@@ -7,6 +7,7 @@ defmodule Towerops.SnmpTest do
   alias Towerops.Snmp.Device
   alias Towerops.Snmp.Interface
   alias Towerops.Snmp.InterfaceStat
+  alias Towerops.Snmp.Neighbor
   alias Towerops.Snmp.Sensor
   alias Towerops.Snmp.SensorReading
   alias Towerops.Snmp.SnmpMock
@@ -748,4 +749,564 @@ defmodule Towerops.SnmpTest do
       assert changeset.errors[:interface_id]
     end
   end
+
+  describe "get_latest_sensor_readings_batch/1" do
+    test "returns empty map for empty sensor list", %{device: _device, snmp_device: _snmp_device} do
+      assert Snmp.get_latest_sensor_readings_batch([]) == %{}
+    end
+
+    test "returns readings for multiple sensors", %{device: _device, snmp_device: snmp_device} do
+      sensor1 =
+        %Sensor{}
+        |> Sensor.changeset(%{
+          snmp_device_id: snmp_device.id,
+          sensor_type: "temperature",
+          sensor_index: "1",
+          sensor_oid: "1.2.3.4"
+        })
+        |> Repo.insert!()
+
+      sensor2 =
+        %Sensor{}
+        |> Sensor.changeset(%{
+          snmp_device_id: snmp_device.id,
+          sensor_type: "power",
+          sensor_index: "2",
+          sensor_oid: "1.2.3.5"
+        })
+        |> Repo.insert!()
+
+      reading1 =
+        %SensorReading{}
+        |> SensorReading.changeset(%{
+          sensor_id: sensor1.id,
+          value: 25.0,
+          status: "ok",
+          checked_at: DateTime.utc_now()
+        })
+        |> Repo.insert!()
+
+      reading2 =
+        %SensorReading{}
+        |> SensorReading.changeset(%{
+          sensor_id: sensor2.id,
+          value: 100.0,
+          status: "ok",
+          checked_at: DateTime.utc_now()
+        })
+        |> Repo.insert!()
+
+      batch = Snmp.get_latest_sensor_readings_batch([sensor1.id, sensor2.id])
+      assert map_size(batch) == 2
+      assert batch[sensor1.id].id == reading1.id
+      assert batch[sensor2.id].id == reading2.id
+    end
+
+    test "returns only latest reading per sensor", %{device: _device, snmp_device: snmp_device} do
+      sensor =
+        %Sensor{}
+        |> Sensor.changeset(%{
+          snmp_device_id: snmp_device.id,
+          sensor_type: "temperature",
+          sensor_index: "1",
+          sensor_oid: "1.2.3.4"
+        })
+        |> Repo.insert!()
+
+      _old_reading =
+        %SensorReading{}
+        |> SensorReading.changeset(%{
+          sensor_id: sensor.id,
+          value: 20.0,
+          status: "ok",
+          checked_at: DateTime.add(DateTime.utc_now(), -3600, :second)
+        })
+        |> Repo.insert!()
+
+      new_reading =
+        %SensorReading{}
+        |> SensorReading.changeset(%{
+          sensor_id: sensor.id,
+          value: 25.0,
+          status: "ok",
+          checked_at: DateTime.utc_now()
+        })
+        |> Repo.insert!()
+
+      batch = Snmp.get_latest_sensor_readings_batch([sensor.id])
+      assert map_size(batch) == 1
+      assert batch[sensor.id].id == new_reading.id
+      assert batch[sensor.id].value == 25.0
+    end
+
+    test "excludes sensors without readings", %{device: _device, snmp_device: snmp_device} do
+      sensor_with_reading =
+        %Sensor{}
+        |> Sensor.changeset(%{
+          snmp_device_id: snmp_device.id,
+          sensor_type: "temperature",
+          sensor_index: "1",
+          sensor_oid: "1.2.3.4"
+        })
+        |> Repo.insert!()
+
+      sensor_without_reading =
+        %Sensor{}
+        |> Sensor.changeset(%{
+          snmp_device_id: snmp_device.id,
+          sensor_type: "power",
+          sensor_index: "2",
+          sensor_oid: "1.2.3.5"
+        })
+        |> Repo.insert!()
+
+      %SensorReading{}
+      |> SensorReading.changeset(%{
+        sensor_id: sensor_with_reading.id,
+        value: 25.0,
+        status: "ok",
+        checked_at: DateTime.utc_now()
+      })
+      |> Repo.insert!()
+
+      batch = Snmp.get_latest_sensor_readings_batch([sensor_with_reading.id, sensor_without_reading.id])
+      assert map_size(batch) == 1
+      assert Map.has_key?(batch, sensor_with_reading.id)
+      refute Map.has_key?(batch, sensor_without_reading.id)
+    end
+  end
+
+  describe "get_latest_interface_stats_batch/1" do
+    test "returns empty map for empty interface list", %{device: _device, snmp_device: _snmp_device} do
+      assert Snmp.get_latest_interface_stats_batch([]) == %{}
+    end
+
+    test "returns stats for multiple interfaces", %{device: _device, snmp_device: snmp_device} do
+      interface1 =
+        %Interface{}
+        |> Interface.changeset(%{
+          snmp_device_id: snmp_device.id,
+          if_index: 1,
+          if_name: "eth0"
+        })
+        |> Repo.insert!()
+
+      interface2 =
+        %Interface{}
+        |> Interface.changeset(%{
+          snmp_device_id: snmp_device.id,
+          if_index: 2,
+          if_name: "eth1"
+        })
+        |> Repo.insert!()
+
+      stat1 =
+        %InterfaceStat{}
+        |> InterfaceStat.changeset(%{
+          interface_id: interface1.id,
+          if_in_octets: 1000,
+          if_out_octets: 2000,
+          checked_at: DateTime.utc_now()
+        })
+        |> Repo.insert!()
+
+      stat2 =
+        %InterfaceStat{}
+        |> InterfaceStat.changeset(%{
+          interface_id: interface2.id,
+          if_in_octets: 5000,
+          if_out_octets: 6000,
+          checked_at: DateTime.utc_now()
+        })
+        |> Repo.insert!()
+
+      batch = Snmp.get_latest_interface_stats_batch([interface1.id, interface2.id])
+      assert map_size(batch) == 2
+      assert batch[interface1.id].id == stat1.id
+      assert batch[interface2.id].id == stat2.id
+    end
+
+    test "returns only latest stat per interface", %{device: _device, snmp_device: snmp_device} do
+      interface =
+        %Interface{}
+        |> Interface.changeset(%{
+          snmp_device_id: snmp_device.id,
+          if_index: 1,
+          if_name: "eth0"
+        })
+        |> Repo.insert!()
+
+      _old_stat =
+        %InterfaceStat{}
+        |> InterfaceStat.changeset(%{
+          interface_id: interface.id,
+          if_in_octets: 1000,
+          if_out_octets: 2000,
+          checked_at: DateTime.add(DateTime.utc_now(), -3600, :second)
+        })
+        |> Repo.insert!()
+
+      new_stat =
+        %InterfaceStat{}
+        |> InterfaceStat.changeset(%{
+          interface_id: interface.id,
+          if_in_octets: 5000,
+          if_out_octets: 6000,
+          checked_at: DateTime.utc_now()
+        })
+        |> Repo.insert!()
+
+      batch = Snmp.get_latest_interface_stats_batch([interface.id])
+      assert map_size(batch) == 1
+      assert batch[interface.id].id == new_stat.id
+      assert batch[interface.id].if_in_octets == 5000
+    end
+
+    test "excludes interfaces without stats", %{device: _device, snmp_device: snmp_device} do
+      interface_with_stat =
+        %Interface{}
+        |> Interface.changeset(%{
+          snmp_device_id: snmp_device.id,
+          if_index: 1,
+          if_name: "eth0"
+        })
+        |> Repo.insert!()
+
+      interface_without_stat =
+        %Interface{}
+        |> Interface.changeset(%{
+          snmp_device_id: snmp_device.id,
+          if_index: 2,
+          if_name: "eth1"
+        })
+        |> Repo.insert!()
+
+      %InterfaceStat{}
+      |> InterfaceStat.changeset(%{
+        interface_id: interface_with_stat.id,
+        if_in_octets: 1000,
+        if_out_octets: 2000,
+        checked_at: DateTime.utc_now()
+      })
+      |> Repo.insert!()
+
+      batch = Snmp.get_latest_interface_stats_batch([interface_with_stat.id, interface_without_stat.id])
+      assert map_size(batch) == 1
+      assert Map.has_key?(batch, interface_with_stat.id)
+      refute Map.has_key?(batch, interface_without_stat.id)
+    end
+  end
+
+  describe "list_neighbors/1" do
+    test "returns all neighbors for a device", %{device: device, snmp_device: snmp_device} do
+      interface =
+        %Interface{}
+        |> Interface.changeset(%{
+          snmp_device_id: snmp_device.id,
+          if_index: 1,
+          if_name: "eth0"
+        })
+        |> Repo.insert!()
+
+      neighbor =
+        %Neighbor{}
+        |> Neighbor.changeset(%{
+          device_id: device.id,
+          interface_id: interface.id,
+          protocol: "lldp",
+          remote_chassis_id: "00:11:22:33:44:55",
+          remote_system_name: "neighbor-switch",
+          last_discovered_at: DateTime.utc_now()
+        })
+        |> Repo.insert!()
+
+      neighbors = Snmp.list_neighbors(device.id)
+      assert length(neighbors) == 1
+      assert hd(neighbors).id == neighbor.id
+      assert Ecto.assoc_loaded?(hd(neighbors).interface)
+    end
+
+    test "returns empty list for device with no neighbors", %{device: device, snmp_device: _snmp_device} do
+      assert Snmp.list_neighbors(device.id) == []
+    end
+
+    test "orders neighbors by protocol and system name", %{device: device, snmp_device: snmp_device} do
+      interface =
+        %Interface{}
+        |> Interface.changeset(%{
+          snmp_device_id: snmp_device.id,
+          if_index: 1,
+          if_name: "eth0"
+        })
+        |> Repo.insert!()
+
+      neighbor_lldp_b =
+        %Neighbor{}
+        |> Neighbor.changeset(%{
+          device_id: device.id,
+          interface_id: interface.id,
+          protocol: "lldp",
+          remote_system_name: "switch-b",
+          last_discovered_at: DateTime.utc_now()
+        })
+        |> Repo.insert!()
+
+      neighbor_cdp_a =
+        %Neighbor{}
+        |> Neighbor.changeset(%{
+          device_id: device.id,
+          interface_id: interface.id,
+          protocol: "cdp",
+          remote_system_name: "router-a",
+          last_discovered_at: DateTime.utc_now()
+        })
+        |> Repo.insert!()
+
+      neighbor_lldp_a =
+        %Neighbor{}
+        |> Neighbor.changeset(%{
+          device_id: device.id,
+          interface_id: interface.id,
+          protocol: "lldp",
+          remote_system_name: "switch-a",
+          last_discovered_at: DateTime.utc_now()
+        })
+        |> Repo.insert!()
+
+      neighbors = Snmp.list_neighbors(device.id)
+      assert length(neighbors) == 3
+      # Should be ordered: cdp first, then lldp alphabetically
+      assert Enum.at(neighbors, 0).id == neighbor_cdp_a.id
+      assert Enum.at(neighbors, 1).id == neighbor_lldp_a.id
+      assert Enum.at(neighbors, 2).id == neighbor_lldp_b.id
+    end
+  end
+
+  describe "get_neighbor/1" do
+    test "returns neighbor by id", %{device: device, snmp_device: snmp_device} do
+      interface =
+        %Interface{}
+        |> Interface.changeset(%{
+          snmp_device_id: snmp_device.id,
+          if_index: 1,
+          if_name: "eth0"
+        })
+        |> Repo.insert!()
+
+      neighbor =
+        %Neighbor{}
+        |> Neighbor.changeset(%{
+          device_id: device.id,
+          interface_id: interface.id,
+          protocol: "lldp",
+          remote_chassis_id: "00:11:22:33:44:55",
+          last_discovered_at: DateTime.utc_now()
+        })
+        |> Repo.insert!()
+
+      found = Snmp.get_neighbor(neighbor.id)
+      assert found.id == neighbor.id
+      assert found.remote_chassis_id == "00:11:22:33:44:55"
+    end
+
+    test "returns nil for non-existent id" do
+      assert Snmp.get_neighbor(Ecto.UUID.generate()) == nil
+    end
+  end
+
+  describe "upsert_neighbor/1" do
+    test "creates new neighbor when none exists", %{device: device, snmp_device: snmp_device} do
+      interface =
+        %Interface{}
+        |> Interface.changeset(%{
+          snmp_device_id: snmp_device.id,
+          if_index: 1,
+          if_name: "eth0"
+        })
+        |> Repo.insert!()
+
+      attrs = %{
+        device_id: device.id,
+        interface_id: interface.id,
+        protocol: "lldp",
+        remote_chassis_id: "00:11:22:33:44:55",
+        remote_system_name: "new-neighbor",
+        last_discovered_at: DateTime.utc_now()
+      }
+
+      {:ok, neighbor} = Snmp.upsert_neighbor(attrs)
+      assert neighbor.remote_system_name == "new-neighbor"
+      assert neighbor.remote_chassis_id == "00:11:22:33:44:55"
+    end
+
+    test "updates existing neighbor when match found", %{device: device, snmp_device: snmp_device} do
+      interface =
+        %Interface{}
+        |> Interface.changeset(%{
+          snmp_device_id: snmp_device.id,
+          if_index: 1,
+          if_name: "eth0"
+        })
+        |> Repo.insert!()
+
+      existing =
+        %Neighbor{}
+        |> Neighbor.changeset(%{
+          device_id: device.id,
+          interface_id: interface.id,
+          protocol: "lldp",
+          remote_chassis_id: "00:11:22:33:44:55",
+          remote_system_name: "old-name",
+          last_discovered_at: DateTime.add(DateTime.utc_now(), -3600, :second)
+        })
+        |> Repo.insert!()
+
+      new_time = DateTime.truncate(DateTime.utc_now(), :second)
+
+      attrs = %{
+        device_id: device.id,
+        interface_id: interface.id,
+        protocol: "lldp",
+        remote_chassis_id: "00:11:22:33:44:55",
+        remote_system_name: "updated-name",
+        last_discovered_at: new_time
+      }
+
+      {:ok, updated} = Snmp.upsert_neighbor(attrs)
+      assert updated.id == existing.id
+      assert updated.remote_system_name == "updated-name"
+      # Compare with truncation since database stores :utc_datetime (no microseconds)
+      assert DateTime.compare(updated.last_discovered_at, new_time) in [:eq, :gt]
+    end
+  end
+
+  describe "delete_stale_neighbors/2" do
+    test "deletes neighbors older than cutoff", %{device: device, snmp_device: snmp_device} do
+      interface =
+        %Interface{}
+        |> Interface.changeset(%{
+          snmp_device_id: snmp_device.id,
+          if_index: 1,
+          if_name: "eth0"
+        })
+        |> Repo.insert!()
+
+      old_time = DateTime.add(DateTime.utc_now(), -7200, :second)
+      recent_time = DateTime.add(DateTime.utc_now(), -60, :second)
+
+      old_neighbor =
+        %Neighbor{}
+        |> Neighbor.changeset(%{
+          device_id: device.id,
+          interface_id: interface.id,
+          protocol: "lldp",
+          remote_chassis_id: "00:11:22:33:44:55",
+          last_discovered_at: old_time
+        })
+        |> Repo.insert!()
+
+      recent_neighbor =
+        %Neighbor{}
+        |> Neighbor.changeset(%{
+          device_id: device.id,
+          interface_id: interface.id,
+          protocol: "cdp",
+          remote_chassis_id: "aa:bb:cc:dd:ee:ff",
+          last_discovered_at: recent_time
+        })
+        |> Repo.insert!()
+
+      cutoff = DateTime.add(DateTime.utc_now(), -3600, :second)
+      {deleted_count, _} = Snmp.delete_stale_neighbors(device.id, cutoff)
+
+      assert deleted_count == 1
+      assert Snmp.get_neighbor(old_neighbor.id) == nil
+      assert Snmp.get_neighbor(recent_neighbor.id)
+    end
+
+    test "does not delete neighbors from other devices", %{organization: organization} do
+      {:ok, site} =
+        Towerops.Sites.create_site(%{
+          name: "Another Site",
+          organization_id: organization.id
+        })
+
+      {:ok, device1} =
+        Towerops.Devices.create_device(%{
+          name: "Router 1",
+          ip_address: "192.168.1.1",
+          site_id: site.id
+        })
+
+      {:ok, device2} =
+        Towerops.Devices.create_device(%{
+          name: "Router 2",
+          ip_address: "192.168.1.2",
+          site_id: site.id
+        })
+
+      snmp_device1 =
+        %Device{}
+        |> Device.changeset(%{
+          device_id: device1.id,
+          sys_name: "router1"
+        })
+        |> Repo.insert!()
+
+      snmp_device2 =
+        %Device{}
+        |> Device.changeset(%{
+          device_id: device2.id,
+          sys_name: "router2"
+        })
+        |> Repo.insert!()
+
+      interface1 =
+        %Interface{}
+        |> Interface.changeset(%{
+          snmp_device_id: snmp_device1.id,
+          if_index: 1,
+          if_name: "eth0"
+        })
+        |> Repo.insert!()
+
+      interface2 =
+        %Interface{}
+        |> Interface.changeset(%{
+          snmp_device_id: snmp_device2.id,
+          if_index: 1,
+          if_name: "eth0"
+        })
+        |> Repo.insert!()
+
+      old_time = DateTime.add(DateTime.utc_now(), -7200, :second)
+
+      neighbor1 =
+        %Neighbor{}
+        |> Neighbor.changeset(%{
+          device_id: device1.id,
+          interface_id: interface1.id,
+          protocol: "lldp",
+          last_discovered_at: old_time
+        })
+        |> Repo.insert!()
+
+      neighbor2 =
+        %Neighbor{}
+        |> Neighbor.changeset(%{
+          device_id: device2.id,
+          interface_id: interface2.id,
+          protocol: "lldp",
+          last_discovered_at: old_time
+        })
+        |> Repo.insert!()
+
+      cutoff = DateTime.add(DateTime.utc_now(), -3600, :second)
+      {deleted_count, _} = Snmp.delete_stale_neighbors(device1.id, cutoff)
+
+      assert deleted_count == 1
+      assert Snmp.get_neighbor(neighbor1.id) == nil
+      assert Snmp.get_neighbor(neighbor2.id)
+    end
+  end
 end