Fix all Credo issues

Resolves 26 Credo warnings/issues:

Code Readability (2 issues):
- Break long line in mac_address.ex (line 316 > 120 chars)
- Break long @spec in agents.ex (line 332 > 120 chars)

Refactoring Opportunities (2 issues):
- Reduce cyclomatic complexity in device_live/form.ex
  - Extract helper functions to simplify SNMPv3 credential resolution
  - Split logic into smaller, focused functions
- Reduce nesting depth in snmp_oid.ex
  - Extract nested logic into separate helper functions

Warnings (22 issues):
- Fix comparison warning in validator.ex
  - Split validate_sensor_value/1 into separate clauses for int/float
  - Add credo directive for valid NaN check (value != value)
- Replace 21 expensive length/1 checks with Enum.empty?/1
  - Change assertions from `assert length(list) >= 1` to `refute Enum.empty?(list)`
  - Change assertions from `assert length(list) > 0` to `refute Enum.empty?(list)`
  - Affected files: device_poller_worker_test.exs, mib_test.exs,
    snmp_tokenizer_test.exs, parser_test.exs, error_test.exs, compiler_test.exs

All tests passing (5578 tests, 0 failures).
Credo now reports: "found no issues"

lib/towerops/agent/validator.ex

@@ -368,14 +368,17 @@ defmodule Towerops.Agent.Validator do
   defp validate_job_id(_), do: {:error, {:invalid_job_id, "Job ID must be non-empty string"}}
 
   # Validate sensor value (must be finite float/int)
-  defp validate_sensor_value(value) when is_float(value) or is_integer(value) do
+  defp validate_sensor_value(value) when is_integer(value), do: :ok
+
+  defp validate_sensor_value(value) when is_float(value) do
     # Check for NaN (NaN is the only value that doesn't equal itself)
     # Check for infinity by comparing with max/min float values
     cond do
-      is_float(value) and value != value ->
+      # credo:disable-for-next-line Credo.Check.Warning.OperationOnSameValues
+      value != value ->
         {:error, {:invalid_sensor_value, "Sensor value must be finite number (not NaN)"}}
 
-      is_float(value) and (value > 1.0e308 or value < -1.0e308) ->
+      value > 1.0e308 or value < -1.0e308 ->
         {:error, {:invalid_sensor_value, "Sensor value must be finite number (not infinity)"}}
 
       true ->

lib/towerops/agents.ex

@@ -329,7 +329,8 @@ defmodule Towerops.Agents do
       {:error, %Ecto.Changeset{}}
 
   """
-  @spec assign_device_to_agent(Ecto.UUID.t(), Ecto.UUID.t()) :: {:ok, AgentAssignment.t()} | {:error, Ecto.Changeset.t()}
+  @spec assign_device_to_agent(Ecto.UUID.t(), Ecto.UUID.t()) ::
+          {:ok, AgentAssignment.t()} | {:error, Ecto.Changeset.t()}
   def assign_device_to_agent(agent_token_id, device_id) do
     result =
       %AgentAssignment{}

lib/towerops/ecto_types/mac_address.ex

@@ -313,7 +313,11 @@ defmodule Towerops.EctoTypes.MacAddress do
 
   defp binary_to_dot_string(<<o1, o2, o3, o4, o5, o6>>) do
     Enum.join(
-      [format_octet(o1) <> format_octet(o2), format_octet(o3) <> format_octet(o4), format_octet(o5) <> format_octet(o6)],
+      [
+        format_octet(o1) <> format_octet(o2),
+        format_octet(o3) <> format_octet(o4),
+        format_octet(o5) <> format_octet(o6)
+      ],
       "."
     )
   end

lib/towerops/ecto_types/snmp_oid.ex

@@ -261,23 +261,25 @@ defmodule Towerops.EctoTypes.SnmpOid do
     # Try SnmpKit resolution
     case SnmpKit.resolve(oid_string) do
       {:ok, resolved} ->
-        # Ensure leading dot
-        normalized = if String.starts_with?(resolved, "."), do: resolved, else: "." <> resolved
-        {:ok, normalized}
+        {:ok, ensure_leading_dot(resolved)}
 
       {:error, _} = error ->
-        # If it's already a numeric OID, return it
-        if Regex.match?(@numeric_oid_pattern, oid_string) do
-          normalized =
-            if String.starts_with?(oid_string, "."), do: oid_string, else: "." <> oid_string
-
-          {:ok, normalized}
-        else
-          error
-        end
+        resolve_numeric_oid(oid_string, error)
     end
   end
 
+  defp resolve_numeric_oid(oid_string, error) do
+    if Regex.match?(@numeric_oid_pattern, oid_string) do
+      {:ok, ensure_leading_dot(oid_string)}
+    else
+      error
+    end
+  end
+
+  defp ensure_leading_dot(oid_string) do
+    if String.starts_with?(oid_string, "."), do: oid_string, else: "." <> oid_string
+  end
+
   # Private helpers
 
   defp parse_oid(""), do: :error

lib/towerops_web/live/device_live/form.ex

@@ -323,57 +323,58 @@ defmodule ToweropsWeb.DeviceLive.Form do
 
   # Resolve SNMPv3 credentials with site/org inheritance
   defp resolve_snmpv3_credentials_for_test(form_data, changeset, assigns) do
-    changes = changeset.changes
+    cond do
+      # Use device-level credentials if available
+      form_data.snmpv3_username && form_data.snmpv3_username != "" ->
+        build_device_snmpv3_creds(form_data, changeset.changes)
 
-    # Check if device has credentials (either from DB or form input)
-    if form_data.snmpv3_username && form_data.snmpv3_username != "" do
-      # Use device-level values
-      # For passwords, prefer changeset changes (newly entered), fall back to DB values
-      %{
-        version: "3",
-        security_level: form_data.snmpv3_security_level,
-        username: form_data.snmpv3_username,
-        auth_protocol: form_data.snmpv3_auth_protocol,
-        auth_password: Map.get(changes, :snmpv3_auth_password) || form_data.snmpv3_auth_password,
-        priv_protocol: form_data.snmpv3_priv_protocol,
-        priv_password: Map.get(changes, :snmpv3_priv_password) || form_data.snmpv3_priv_password
-      }
-    else
-      # Inherit from site or org
-      site_id = form_data.site_id
-      site = site_id && Enum.find(assigns.available_sites, &(&1.id == site_id))
+      # Inherit from site if available
+      site_has_snmpv3_creds?(form_data, assigns) ->
+        site = find_site(form_data.site_id, assigns.available_sites)
+        build_snmpv3_creds(site)
 
-      cond do
-        # Try site-level credentials
-        site && site.snmpv3_username ->
-          %{
-            version: "3",
-            security_level: site.snmpv3_security_level,
-            username: site.snmpv3_username,
-            auth_protocol: site.snmpv3_auth_protocol,
-            auth_password: site.snmpv3_auth_password,
-            priv_protocol: site.snmpv3_priv_protocol,
-            priv_password: site.snmpv3_priv_password
-          }
+      # Fall back to organization credentials
+      assigns.organization.snmpv3_username ->
+        build_snmpv3_creds(assigns.organization)
 
-        # Fall back to org-level credentials
-        assigns.organization.snmpv3_username ->
-          %{
-            version: "3",
-            security_level: assigns.organization.snmpv3_security_level,
-            username: assigns.organization.snmpv3_username,
-            auth_protocol: assigns.organization.snmpv3_auth_protocol,
-            auth_password: assigns.organization.snmpv3_auth_password,
-            priv_protocol: assigns.organization.snmpv3_priv_protocol,
-            priv_password: assigns.organization.snmpv3_priv_password
-          }
-
-        true ->
-          %{version: "3"}
-      end
+      # No credentials available
+      true ->
+        %{version: "3"}
     end
   end
 
+  defp site_has_snmpv3_creds?(form_data, assigns) do
+    site = find_site(form_data.site_id, assigns.available_sites)
+    site && site.snmpv3_username
+  end
+
+  defp find_site(nil, _sites), do: nil
+  defp find_site(site_id, sites), do: Enum.find(sites, &(&1.id == site_id))
+
+  defp build_device_snmpv3_creds(form_data, changes) do
+    %{
+      version: "3",
+      security_level: form_data.snmpv3_security_level,
+      username: form_data.snmpv3_username,
+      auth_protocol: form_data.snmpv3_auth_protocol,
+      auth_password: Map.get(changes, :snmpv3_auth_password) || form_data.snmpv3_auth_password,
+      priv_protocol: form_data.snmpv3_priv_protocol,
+      priv_password: Map.get(changes, :snmpv3_priv_password) || form_data.snmpv3_priv_password
+    }
+  end
+
+  defp build_snmpv3_creds(source) do
+    %{
+      version: "3",
+      security_level: source.snmpv3_security_level,
+      username: source.snmpv3_username,
+      auth_protocol: source.snmpv3_auth_protocol,
+      auth_password: source.snmpv3_auth_password,
+      priv_protocol: source.snmpv3_priv_protocol,
+      priv_password: source.snmpv3_priv_password
+    }
+  end
+
   # Resolve SNMP community with site/org inheritance
   defp resolve_snmp_community_for_test(form_data, changeset, assigns) do
     version = form_data.snmp_version || "2c"

test/snmpkit/snmp_lib/mib/compiler_test.exs

@@ -191,7 +191,7 @@ defmodule SnmpKit.SnmpLib.MIB.CompilerTest do
 
       assert {:error, errors} = Compiler.compile_all([file1, file2])
       assert is_list(errors)
-      assert length(errors) > 0
+      refute Enum.empty?(errors)
 
       # Cleanup
       File.rm_rf!(dir)
@@ -367,7 +367,7 @@ defmodule SnmpKit.SnmpLib.MIB.CompilerTest do
 
       assert {:error, errors} = Compiler.compile_string(mib_content)
       assert is_list(errors)
-      assert length(errors) > 0
+      refute Enum.empty?(errors)
     end
 
     test "handles missing file errors" do

test/snmpkit/snmp_lib/mib/error_test.exs

@@ -95,42 +95,42 @@ defmodule SnmpKit.SnmpLib.MIB.ErrorTest do
     test "generates suggestions for unexpected_token MAX-ACCESS" do
       error = Error.new(:unexpected_token, expected: :max_access, actual: :access)
 
-      assert length(error.suggestions) > 0
+      refute Enum.empty?(error.suggestions)
       assert Enum.any?(error.suggestions, &String.contains?(&1, "MAX-ACCESS"))
     end
 
     test "generates suggestions for unexpected_token mandatory/current" do
       error = Error.new(:unexpected_token, expected: :current, actual: :mandatory)
 
-      assert length(error.suggestions) > 0
+      refute Enum.empty?(error.suggestions)
       assert Enum.any?(error.suggestions, &String.contains?(&1, "deprecated"))
     end
 
     test "generates suggestions for unexpected_token OBJECT-TYPE" do
       error = Error.new(:unexpected_token, expected: :object_type, actual: :object_identity)
 
-      assert length(error.suggestions) > 0
+      refute Enum.empty?(error.suggestions)
       assert Enum.any?(error.suggestions, &String.contains?(&1, "OBJECT-TYPE"))
     end
 
     test "generates suggestions for unterminated_string" do
       error = Error.new(:unterminated_string)
 
-      assert length(error.suggestions) > 0
+      refute Enum.empty?(error.suggestions)
       assert Enum.any?(error.suggestions, &String.contains?(&1, "quote"))
     end
 
     test "generates suggestions for file_not_found with RFC file" do
       error = Error.new(:file_not_found, file: "RFC1213-MIB.txt")
 
-      assert length(error.suggestions) > 0
+      refute Enum.empty?(error.suggestions)
       assert Enum.any?(error.suggestions, &String.contains?(&1, "naming conventions"))
     end
 
     test "generates suggestions for file_not_found with MIB file" do
       error = Error.new(:file_not_found, file: "TEST-MIB.mib")
 
-      assert length(error.suggestions) > 0
+      refute Enum.empty?(error.suggestions)
       assert Enum.any?(error.suggestions, &String.contains?(&1, "extension"))
     end
 
@@ -144,7 +144,7 @@ defmodule SnmpKit.SnmpLib.MIB.ErrorTest do
     test "generates suggestions for import_error" do
       error = Error.new(:import_error, symbol: "test", from_module: "TEST-MIB")
 
-      assert length(error.suggestions) > 0
+      refute Enum.empty?(error.suggestions)
       assert Enum.any?(error.suggestions, &String.contains?(&1, "import"))
     end
 

test/snmpkit/snmp_lib/mib/parser_test.exs

@@ -30,7 +30,7 @@ defmodule SnmpKit.SnmpLib.MIB.ParserTest do
       result = Parser.tokenize(mib)
       assert {:ok, tokens} = result
       assert is_list(tokens)
-      assert length(tokens) > 0
+      refute Enum.empty?(tokens)
     end
 
     test "tokenizes reserved words" do

test/snmpkit/snmp_lib/mib/snmp_tokenizer_test.exs

@@ -46,7 +46,7 @@ defmodule SnmpKit.SnmpLib.MIB.SnmpTokenizerTest do
         assert {:ok, tokens} = SnmpTokenizer.tokenize(input, &SnmpTokenizer.null_get_line/0)
 
         # First token should be the reserved word
-        assert length(tokens) >= 1
+        refute Enum.empty?(tokens)
       end
     end
 
@@ -84,7 +84,7 @@ defmodule SnmpKit.SnmpLib.MIB.SnmpTokenizerTest do
 
       # Strings might be tokenized as :string or just handled as tokens
       # Just verify tokenization succeeds
-      assert length(tokens) >= 1
+      refute Enum.empty?(tokens)
     end
 
     test "tokenizes special characters" do
@@ -136,7 +136,7 @@ defmodule SnmpKit.SnmpLib.MIB.SnmpTokenizerTest do
       for access <- access_values do
         input = String.to_charlist(access)
         assert {:ok, tokens} = SnmpTokenizer.tokenize(input, &SnmpTokenizer.null_get_line/0)
-        assert length(tokens) >= 1
+        refute Enum.empty?(tokens)
       end
     end
 
@@ -146,7 +146,7 @@ defmodule SnmpKit.SnmpLib.MIB.SnmpTokenizerTest do
       for type <- types do
         input = String.to_charlist(type)
         assert {:ok, tokens} = SnmpTokenizer.tokenize(input, &SnmpTokenizer.null_get_line/0)
-        assert length(tokens) >= 1
+        refute Enum.empty?(tokens)
       end
     end
 
@@ -369,7 +369,7 @@ defmodule SnmpKit.SnmpLib.MIB.SnmpTokenizerTest do
       long_id = "a" |> String.duplicate(200) |> String.to_charlist()
 
       assert {:ok, tokens} = SnmpTokenizer.tokenize(long_id, &SnmpTokenizer.null_get_line/0)
-      assert length(tokens) >= 1
+      refute Enum.empty?(tokens)
     end
 
     test "handles many tokens" do
@@ -421,7 +421,7 @@ defmodule SnmpKit.SnmpLib.MIB.SnmpTokenizerTest do
       input = ~c"\"\""
 
       assert {:ok, tokens} = SnmpTokenizer.tokenize(input, &SnmpTokenizer.null_get_line/0)
-      assert length(tokens) >= 1
+      refute Enum.empty?(tokens)
     end
 
     test "handles consecutive operators" do
@@ -506,7 +506,7 @@ defmodule SnmpKit.SnmpLib.MIB.SnmpTokenizerTest do
         assert {:ok, tokens} = SnmpTokenizer.tokenize(input, &SnmpTokenizer.null_get_line/0)
 
         # Should successfully tokenize without error
-        assert length(tokens) >= 1
+        refute Enum.empty?(tokens)
       end
     end
 
@@ -527,7 +527,7 @@ defmodule SnmpKit.SnmpLib.MIB.SnmpTokenizerTest do
 
       # "object" should be an atom (lowercase identifier)
       atoms = Enum.filter(tokens, &match?({:atom, _, _}, &1))
-      assert length(atoms) >= 1
+      refute Enum.empty?(atoms)
     end
   end
 end

test/snmpkit/snmp_lib/mib_test.exs

@@ -10,7 +10,7 @@ defmodule SnmpKit.SnmpLib.MIBTest do
 
       assert {:error, errors} = result
       assert is_list(errors)
-      assert length(errors) > 0
+      refute Enum.empty?(errors)
       assert %Error{type: :file_not_found} = List.first(errors)
     end
 

test/towerops/workers/device_poller_worker_test.exs

@@ -232,7 +232,7 @@ defmodule Towerops.Workers.DevicePollerWorkerTest do
 
       # 8. Verify Interface Stats
       stats = Repo.all(Towerops.Snmp.InterfaceStat)
-      assert length(stats) >= 1
+      refute Enum.empty?(stats)
       stat = List.last(stats)
       assert stat.interface_id == interface.id
       assert stat.if_in_octets == 1000
@@ -250,7 +250,7 @@ defmodule Towerops.Workers.DevicePollerWorkerTest do
 
       # 11. Verify Processor
       readings = Repo.all(Towerops.Snmp.ProcessorReading)
-      assert length(readings) >= 1
+      refute Enum.empty?(readings)
       reading = List.last(readings)
       assert reading.processor_id == processor.id
       assert reading.load_percent == 25.0