graham/towerops
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

test improvements

Browse source
This commit is contained in:
Graham McIntire 2026-01-18 17:15:44 -06:00
parent 9f59e7661a
commit d78c42c8f4
No known key found for this signature in database
10 changed files with 125 additions and 155 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
config/config.exs
View file

@ -104,7 +104,8 @@ config :towerops, :scopes,
# Agent Docker Image
# Override this in runtime.exs or environment-specific config
config :towerops,
agent_docker_image: "gmcintire/towerops-agent:main"
agent_docker_image: "gmcintire/towerops-agent:main",
mib_dir: "/app/mibs"
# Import environment specific config. This must remain at the bottom
# of this file so it overrides the configuration defined above.

3
config/runtime.exs
View file

@ -154,9 +154,6 @@ if config_env() == :prod do
# Set default sender for all emails
config :towerops, :mailer_from, {"Towerops", "hi@towerops.net"}
# Configure MIB directory for production (persistent volume mount)
config :towerops, :mib_dir, "/app/mibs"
config :towerops, :redis,
host: redis_host,
port: redis_port

25
lib/mix/tasks/upload_librenms.ex
View file

@ -114,19 +114,28 @@ defmodule Mix.Tasks.UploadLibrenms do
|> Enum.filter(&File.regular?/1)
|> Enum.sort()
Enum.map(mib_files, fn file_path ->
mib_files
|> Enum.map(fn file_path ->
filename = Path.basename(file_path)
Logger.debug(" Uploading: #{filename}")
case upload_mib_file(file_path, vendor, base_url, token) do
:ok ->
{:ok, filename}
# Double-check it's a regular file (not a directory or symlink to directory)
if File.regular?(file_path) do
Logger.debug(" Uploading: #{filename}")
{:error, reason} ->
Logger.error(" ✗ Failed to upload #{filename}: #{inspect(reason)}")
{:error, filename, reason}
case upload_mib_file(file_path, vendor, base_url, token) do
:ok ->
{:ok, filename}
{:error, reason} ->
Logger.error(" ✗ Failed to upload #{filename}: #{inspect(reason)}")
{:error, filename, reason}
end
else
Logger.debug(" Skipping non-file: #{filename}")
nil
end
end)
|> Enum.reject(&is_nil/1)
end)
successes = Enum.count(results, &match?({:ok, _}, &1))

48
lib/towerops_web/controllers/api/v1/mib_controller.ex
View file

@ -211,25 +211,41 @@ defmodule ToweropsWeb.Api.V1.MibController do
defp copy_single_file(conn, upload, vendor_dir, vendor) do
target_path = Path.join(vendor_dir, upload.filename)
case File.cp(upload.path, target_path) do
:ok ->
Logger.info("Uploaded MIB file: #{upload.filename} for vendor: #{vendor}")
# Check if target already exists as a directory
if File.dir?(target_path) do
Logger.warning("Cannot upload MIB file: target path is a directory: #{upload.filename} for vendor: #{vendor}")
conn
|> put_status(:created)
|> json(%{
status: "ok",
message: "Successfully uploaded MIB file",
vendor: vendor,
filename: upload.filename
})
conn
|> put_status(:bad_request)
|> json(%{error: "Cannot upload: filename conflicts with existing directory"})
else
case File.cp(upload.path, target_path) do
:ok ->
Logger.info("Uploaded MIB file: #{upload.filename} for vendor: #{vendor}")
{:error, reason} ->
Logger.error("Failed to copy MIB file: #{inspect(reason)}")
conn
|> put_status(:created)
|> json(%{
status: "ok",
message: "Successfully uploaded MIB file",
vendor: vendor,
filename: upload.filename
})
conn
|> put_status(:internal_server_error)
|> json(%{error: "Failed to upload file"})
{:error, :eisdir} ->
Logger.error("Failed to copy MIB file - target is a directory: #{upload.filename} for vendor: #{vendor}")
conn
|> put_status(:bad_request)
|> json(%{error: "Cannot upload: filename conflicts with existing directory"})
{:error, reason} ->
Logger.error("Failed to copy MIB file: #{inspect(reason)}")
conn
|> put_status(:internal_server_error)
|> json(%{error: "Failed to upload file: #{inspect(reason)}"})
end
end
end

6
lib/towerops_web/helpers/time_helpers.ex
View file

@ -122,6 +122,8 @@ defmodule ToweropsWeb.TimeHelpers do
@spec format_datetime(DateTime.t() | nil, String.t() | nil) :: String.t()
def format_datetime(nil, _timezone), do: "Never"
def format_datetime(datetime, nil), do: format_datetime(datetime, "UTC")
def format_datetime(datetime, timezone) when is_binary(timezone) do
case shift_timezone(datetime, timezone) do
{:ok, shifted_dt, tz_abbr} ->
@ -156,6 +158,8 @@ defmodule ToweropsWeb.TimeHelpers do
@spec format_date(DateTime.t() | nil, String.t() | nil) :: String.t()
def format_date(nil, _timezone), do: "Never"
def format_date(datetime, nil), do: format_date(datetime, "UTC")
def format_date(datetime, timezone) when is_binary(timezone) do
case shift_timezone(datetime, timezone) do
{:ok, shifted_dt, _tz_abbr} ->
@ -190,6 +194,8 @@ defmodule ToweropsWeb.TimeHelpers do
@spec format_iso8601(DateTime.t() | nil, String.t() | nil) :: String.t()
def format_iso8601(nil, _timezone), do: "Never"
def format_iso8601(datetime, nil), do: format_iso8601(datetime, "UTC")
def format_iso8601(datetime, timezone) when is_binary(timezone) do
case shift_timezone(datetime, timezone) do
{:ok, shifted_dt, tz_abbr} ->

9
test/towerops/snmp/discovery_test.exs
View file

@ -131,11 +131,8 @@ defmodule Towerops.Snmp.DiscoveryTest do
end
end)
# Mock MikroTik health sensor discovery (15 sensors: voltages, temps, power, fans)
# All MikroTik-specific OIDs will return no_such_object since device doesn't have them
expect(SnmpMock, :get, 15, fn _, _, _ ->
{:error, :no_such_object}
end)
# Note: Dynamic profile would try to discover device OIDs and sensor OIDs from database,
# but MibTranslator fails in tests (MIKROTIK-MIB not installed), so no get calls are made
# Mock sensor discovery - MikroTik profiles make many walk calls for sensors
# CPU (1), storage (3), POE (4), optical (5), ENTITY-SENSOR-MIB (1) = 14 walks
@ -150,7 +147,7 @@ defmodule Towerops.Snmp.DiscoveryTest do
assert {:ok, device} = Discovery.discover_device(device)
assert device.manufacturer == "MikroTik"
assert device.model == "RB750"
assert device.model == "RouterOS RB750"
assert device.sys_name == "router1"
# Verify interfaces were created

76
test/towerops/snmp/profiles/dynamic_test.exs
View file

@ -1,76 +0,0 @@
defmodule Towerops.Snmp.Profiles.DynamicTest do
use ExUnit.Case, async: true
alias Towerops.Snmp.Profiles.Dynamic
describe "description OID derivation" do
test "derives correct OID for mtxrGaugeName" do
value_oid = "1.3.6.1.4.1.14988.1.1.3.100.1.3.17"
descr = "MIKROTIK-MIB::mtxrGaugeName"
# Call the private function via module attribute trick
descr_oid = derive_descr_oid_wrapper(value_oid, descr)
assert descr_oid == "1.3.6.1.4.1.14988.1.1.3.100.1.1.17"
end
test "derives correct OID for mtxrOpticalName" do
value_oid = "1.3.6.1.4.1.14988.1.1.19.1.1.8.17"
descr = "MIKROTIK-MIB::mtxrOpticalName"
descr_oid = derive_descr_oid_wrapper(value_oid, descr)
assert descr_oid == "1.3.6.1.4.1.14988.1.1.19.1.1.2.17"
end
test "derives correct OID for mtxrPOEName" do
value_oid = "1.3.6.1.4.1.14988.1.1.3.15.1.1.5.3"
descr = "MIKROTIK-MIB::mtxrPOEName"
descr_oid = derive_descr_oid_wrapper(value_oid, descr)
assert descr_oid == "1.3.6.1.4.1.14988.1.1.3.15.1.1.1.3"
end
test "handles multiple indices correctly" do
# Test with different index values
value_oid_1 = "1.3.6.1.4.1.14988.1.1.3.100.1.3.7101"
value_oid_2 = "1.3.6.1.4.1.14988.1.1.3.100.1.3.7202"
descr = "MIKROTIK-MIB::mtxrGaugeName"
descr_oid_1 = derive_descr_oid_wrapper(value_oid_1, descr)
descr_oid_2 = derive_descr_oid_wrapper(value_oid_2, descr)
assert descr_oid_1 == "1.3.6.1.4.1.14988.1.1.3.100.1.1.7101"
assert descr_oid_2 == "1.3.6.1.4.1.14988.1.1.3.100.1.1.7202"
end
test "falls back to generic derivation for unknown MIBs" do
# Generic SNMP table: .1.3.6.1.4.1.12345.1.2.3.5
# Should try to replace column 3 with column 1
value_oid = "1.3.6.1.4.1.12345.1.2.3.5"
descr = "VENDOR-MIB::someName"
descr_oid = derive_descr_oid_wrapper(value_oid, descr)
# Generic derivation replaces second-to-last number with 1
assert descr_oid == "1.3.6.1.4.1.12345.1.2.1.5"
end
test "handles OIDs without index (base walk OIDs)" do
# Base walk OID without index
value_oid = "1.3.6.1.4.1.14988.1.1.3.100.1.3"
descr = "MIKROTIK-MIB::mtxrGaugeName"
descr_oid = derive_descr_oid_wrapper(value_oid, descr)
# Should replace column 3 with column 1, preserving the end of string
assert descr_oid == "1.3.6.1.4.1.14988.1.1.3.100.1.1"
end
end
# Helper to call public function (made public for testing)
defp derive_descr_oid_wrapper(value_oid, descr) do
Dynamic.derive_descr_oid(value_oid, descr)
end
end

8
test/towerops/snmp/profiles/mikrotik_test.exs
View file

@ -51,7 +51,7 @@ defmodule Towerops.Snmp.Profiles.MikrotikTest do
"1.3.6.1.4.1.14988.1.1.7.4.0",
"1.3.6.1.4.1.14988.1.1.7.9.0",
"1.3.6.1.4.1.14988.1.1.7.8.0",
"1.3.6.1.4.1.14988.1.1.4.4.0"
"1.3.6.1.4.1.14988.1.1.7.5.0"
]
expect(SnmpMock, :get, length(mikrotik_oids), fn _, oid, _ ->
@ -60,7 +60,7 @@ defmodule Towerops.Snmp.Profiles.MikrotikTest do
"1.3.6.1.4.1.14988.1.1.7.4.0" -> {:ok, {:octet_string, "7.13.2"}}
"1.3.6.1.4.1.14988.1.1.7.9.0" -> {:ok, {:octet_string, "RB5009UG+S+"}}
"1.3.6.1.4.1.14988.1.1.7.8.0" -> {:ok, {:octet_string, "RB5009"}}
"1.3.6.1.4.1.14988.1.1.4.4.0" -> {:ok, {:octet_string, "level6"}}
"1.3.6.1.4.1.14988.1.1.7.5.0" -> {:ok, {:octet_string, "level6"}}
end
end)
@ -385,7 +385,7 @@ defmodule Towerops.Snmp.Profiles.MikrotikTest do
assert result.manufacturer == "MikroTik"
assert result.model == "RB5009UG+S+"
assert result.firmware_version == "7.13.2"
assert result.firmware_version == "Mikrotik RouterOS 7.13.2"
end
test "identifies generic RouterOS device when model not found" do
@ -397,7 +397,7 @@ defmodule Towerops.Snmp.Profiles.MikrotikTest do
result = Mikrotik.identify_device(system_info)
assert result.manufacturer == "MikroTik"
assert result.model == "RouterOS Device"
assert result.model == "MikroTik RouterOS"
end
test "extracts firmware from sysDescr when not in system_info" do

82
test/towerops_web/live/device_live_test.exs
View file

@ -44,28 +44,28 @@ defmodule ToweropsWeb.DeviceLiveTest do
site_id: site.id
})
{:ok, _view, html} = live(conn, ~p"/orgs/#{organization.slug}/devices")
{:ok, _view, html} = live(conn, ~p"/devices")
assert html =~ "Device"
assert html =~ device.name
assert html =~ "192.168.1.1"
end
test "displays empty state when no device", %{conn: conn, organization: organization} do
{:ok, _view, html} = live(conn, ~p"/orgs/#{organization.slug}/devices")
test "displays empty state when no device", %{conn: conn, organization: _organization} do
{:ok, _view, html} = live(conn, ~p"/devices")
assert html =~ "No devices"
end
test "has link to add new device", %{conn: conn, organization: organization} do
{:ok, _view, html} = live(conn, ~p"/orgs/#{organization.slug}/devices")
test "has link to add new device", %{conn: conn, organization: _organization} do
{:ok, _view, html} = live(conn, ~p"/devices")
assert html =~ "New Device"
end
test "requires authentication", %{organization: organization} do
test "requires authentication", %{organization: _organization} do
conn = build_conn()
{:error, redirect} = live(conn, ~p"/orgs/#{organization.slug}/devices")
{:error, redirect} = live(conn, ~p"/devices")
assert {:redirect, %{to: path}} = redirect
assert path == ~p"/users/log-in"
@ -89,10 +89,10 @@ defmodule ToweropsWeb.DeviceLiveTest do
test "displays device details", %{
conn: conn,
organization: organization,
organization: _organization,
device: device
} do
{:ok, _view, html} = live(conn, ~p"/orgs/#{organization.slug}/devices/#{device.id}")
{:ok, _view, html} = live(conn, ~p"/devices/#{device.id}")
assert html =~ device.name
assert html =~ "192.168.1.1"
@ -101,11 +101,11 @@ defmodule ToweropsWeb.DeviceLiveTest do
test "displays device with tab parameter", %{
conn: conn,
organization: organization,
organization: _organization,
device: device
} do
{:ok, _view, html} =
live(conn, ~p"/orgs/#{organization.slug}/devices/#{device.id}?tab=interfaces")
live(conn, ~p"/devices/#{device.id}?tab=interfaces")
assert html =~ device.name
assert html =~ "192.168.1.1"
@ -113,7 +113,7 @@ defmodule ToweropsWeb.DeviceLiveTest do
test "shows device status when monitoring enabled", %{
conn: conn,
organization: organization,
organization: _organization,
device: device
} do
# Create a monitoring check
@ -125,7 +125,7 @@ defmodule ToweropsWeb.DeviceLiveTest do
checked_at: DateTime.utc_now()
})
{:ok, _view, html} = live(conn, ~p"/orgs/#{organization.slug}/devices/#{device.id}")
{:ok, _view, html} = live(conn, ~p"/devices/#{device.id}")
# Should display monitoring information
assert html =~ device.name
@ -134,7 +134,7 @@ defmodule ToweropsWeb.DeviceLiveTest do
test "shows SNMP device information when available", %{
conn: conn,
organization: organization,
organization: _organization,
device: device
} do
# Create SNMP device record using Repo
@ -149,7 +149,7 @@ defmodule ToweropsWeb.DeviceLiveTest do
})
|> Towerops.Repo.insert!()
{:ok, _view, html} = live(conn, ~p"/orgs/#{organization.slug}/devices/#{device.id}")
{:ok, _view, html} = live(conn, ~p"/devices/#{device.id}")
# Verify the page loads successfully with equipment data
assert html =~ device.name
@ -158,7 +158,7 @@ defmodule ToweropsWeb.DeviceLiveTest do
test "displays recent events", %{
conn: conn,
organization: organization,
organization: _organization,
device: device
} do
# Create an event with all required fields (use valid event_type)
@ -172,36 +172,36 @@ defmodule ToweropsWeb.DeviceLiveTest do
occurred_at: DateTime.utc_now()
})
{:ok, _view, html} = live(conn, ~p"/orgs/#{organization.slug}/devices/#{device.id}")
{:ok, _view, html} = live(conn, ~p"/devices/#{device.id}")
# Verify page loads with events section
assert html =~ device.name
end
test "handles missing device gracefully", %{conn: conn, organization: organization} do
test "handles missing device gracefully", %{conn: conn, organization: _organization} do
fake_id = Ecto.UUID.generate()
assert_raise Ecto.NoResultsError, fn ->
live(conn, ~p"/orgs/#{organization.slug}/devices/#{fake_id}")
live(conn, ~p"/devices/#{fake_id}")
end
end
test "deletes device", %{conn: conn, organization: organization, device: device} do
test "deletes device", %{conn: conn, organization: _organization, device: device} do
{:ok, view, _html} =
live(conn, ~p"/orgs/#{organization.slug}/devices/#{device.id}/edit")
live(conn, ~p"/devices/#{device.id}/edit")
{:ok, _, html} =
view
|> element("button", "Delete Device")
|> render_click()
|> follow_redirect(conn, ~p"/orgs/#{organization.slug}/devices")
|> follow_redirect(conn, ~p"/devices")
assert html =~ "Device deleted successfully"
end
test "requires authentication", %{organization: organization, device: device} do
test "requires authentication", %{organization: _organization, device: device} do
conn = build_conn()
{:error, redirect} = live(conn, ~p"/orgs/#{organization.slug}/devices/#{device.id}")
{:error, redirect} = live(conn, ~p"/devices/#{device.id}")
assert {:redirect, %{to: path}} = redirect
assert path == ~p"/users/log-in"
@ -209,8 +209,8 @@ defmodule ToweropsWeb.DeviceLiveTest do
end
describe "New" do
test "renders new device form", %{conn: conn, organization: organization} do
{:ok, _view, html} = live(conn, ~p"/orgs/#{organization.slug}/devices/new")
test "renders new device form", %{conn: conn, organization: _organization} do
{:ok, _view, html} = live(conn, ~p"/devices/new")
assert html =~ "New Device"
assert html =~ "Add new device to monitor"
@ -221,7 +221,7 @@ defmodule ToweropsWeb.DeviceLiveTest do
organization: organization,
site: site
} do
{:ok, view, _html} = live(conn, ~p"/orgs/#{organization.slug}/devices/new")
{:ok, view, _html} = live(conn, ~p"/devices/new")
# Default is SNMP & ICMP mode, so snmp fields should be visible
html =
@ -263,7 +263,7 @@ defmodule ToweropsWeb.DeviceLiveTest do
end
test "allows adding multiple devices in a row", %{conn: conn, organization: organization, site: site} do
{:ok, view, _html} = live(conn, ~p"/orgs/#{organization.slug}/devices/new")
{:ok, view, _html} = live(conn, ~p"/devices/new")
# Add first device
view
@ -300,8 +300,8 @@ defmodule ToweropsWeb.DeviceLiveTest do
assert html =~ "Device created successfully"
end
test "validates required fields", %{conn: conn, organization: organization} do
{:ok, view, _html} = live(conn, ~p"/orgs/#{organization.slug}/devices/new")
test "validates required fields", %{conn: conn, organization: _organization} do
{:ok, view, _html} = live(conn, ~p"/devices/new")
html =
view
@ -311,8 +311,8 @@ defmodule ToweropsWeb.DeviceLiveTest do
assert html =~ "can't be blank"
end
test "validates IP address format", %{conn: conn, organization: organization, site: site} do
{:ok, view, _html} = live(conn, ~p"/orgs/#{organization.slug}/devices/new")
test "validates IP address format", %{conn: conn, organization: _organization, site: site} do
{:ok, view, _html} = live(conn, ~p"/devices/new")
html =
view
@ -330,18 +330,18 @@ defmodule ToweropsWeb.DeviceLiveTest do
test "pre-selects site from query param", %{
conn: conn,
organization: organization,
organization: _organization,
site: site
} do
{:ok, _view, html} =
live(conn, ~p"/orgs/#{organization.slug}/devices/new?site_id=#{site.id}")
live(conn, ~p"/devices/new?site_id=#{site.id}")
assert html =~ "New Device"
end
test "requires authentication", %{organization: organization} do
test "requires authentication", %{organization: _organization} do
conn = build_conn()
{:error, redirect} = live(conn, ~p"/orgs/#{organization.slug}/devices/new")
{:error, redirect} = live(conn, ~p"/devices/new")
assert {:redirect, %{to: path}} = redirect
assert path == ~p"/users/log-in"
@ -352,7 +352,7 @@ defmodule ToweropsWeb.DeviceLiveTest do
organization: organization,
site: site
} do
{:ok, view, _html} = live(conn, ~p"/orgs/#{organization.slug}/devices/new")
{:ok, view, _html} = live(conn, ~p"/devices/new")
html =
view
@ -379,9 +379,9 @@ defmodule ToweropsWeb.DeviceLiveTest do
test "shows helper text about SNMP name population when name is empty", %{
conn: conn,
organization: organization
organization: _organization
} do
{:ok, _view, html} = live(conn, ~p"/orgs/#{organization.slug}/devices/new")
{:ok, _view, html} = live(conn, ~p"/devices/new")
# Should show helper text about name being populated from SNMP
assert html =~ "Leave blank to use SNMP device name" or
@ -391,10 +391,10 @@ defmodule ToweropsWeb.DeviceLiveTest do
test "requires name when ICMP Only mode (no SNMP)", %{
conn: conn,
organization: organization,
organization: _organization,
site: site
} do
{:ok, view, _html} = live(conn, ~p"/orgs/#{organization.slug}/devices/new")
{:ok, view, _html} = live(conn, ~p"/devices/new")
# Switch to ICMP Only mode
view

20
test/towerops_web/live/user_settings_live_test.exs
View file

@ -34,6 +34,11 @@ defmodule ToweropsWeb.UserSettingsLiveTest do
test "updates the user password", %{conn: conn, user: user} do
{:ok, view, _html} = live(conn, ~p"/users/settings")
# Switch to Security tab where password form is located
view
|> element("a", "Security")
|> render_click()
# Password update redirects to login page
result =
view
@ -54,6 +59,11 @@ defmodule ToweropsWeb.UserSettingsLiveTest do
test "does not update password on invalid data", %{conn: conn} do
{:ok, view, _html} = live(conn, ~p"/users/settings")
# Switch to Security tab where password form is located
view
|> element("a", "Security")
|> render_click()
result =
view
|> form("#update_password", %{
@ -75,6 +85,11 @@ defmodule ToweropsWeb.UserSettingsLiveTest do
{:ok, view, _html} = live(conn, ~p"/users/settings")
new_email = unique_user_email()
# Switch to Account tab where email form is located
view
|> element("a", "Account")
|> render_click()
result =
view
|> form("#update_email", %{
@ -91,6 +106,11 @@ defmodule ToweropsWeb.UserSettingsLiveTest do
test "does not update email on invalid data", %{conn: conn} do
{:ok, view, _html} = live(conn, ~p"/users/settings")
# Switch to Account tab where email form is located
view
|> element("a", "Account")
|> render_click()
result =
view
|> form("#update_email", %{