From 3f7170cfa21ca084c0a29e85a62ada401e27f363 Mon Sep 17 00:00:00 2001
From: Graham McIntire <graham@mcintire.me>
Date: Fri, 8 May 2026 14:15:35 -0500
Subject: [PATCH] Add coverage tests for LeaderElection, MapLive integration,
 ApiDocsLive, BroadcastTaskSupervisor, StatusLive, and Packets

---
 .../aprsme/broadcast_task_supervisor_test.exs |  16 +
 test/aprsme/cluster/leader_election_test.exs  |  78 +++++
 test/aprsme/packets_test.exs                  |  32 ++
 test/aprsme_web/live/api_docs_live_test.exs   |  29 ++
 .../live/map_live/integration_test.exs        | 288 ++++++++++++++++++
 .../live/status_live/index_test.exs           |  31 ++
 6 files changed, 474 insertions(+)

diff --git a/test/aprsme/broadcast_task_supervisor_test.exs b/test/aprsme/broadcast_task_supervisor_test.exs
index b02f3a9..88b99af 100644
--- a/test/aprsme/broadcast_task_supervisor_test.exs
+++ b/test/aprsme/broadcast_task_supervisor_test.exs
@@ -145,6 +145,22 @@ defmodule Aprsme.BroadcastTaskSupervisorTest do
       assert is_float(stats.scheduler_usage)
       assert stats.scheduler_usage >= 0.0
     end
+
+    test "scheduler_usage with non-test env exercises the live scheduler sampling" do
+      # Mutating :aprsme :env into :prod for one call lets get_stats hit the
+      # live :scheduler.utilization/1 branch. We restore env immediately
+      # afterward so we don't disrupt downstream tests.
+      original = Application.get_env(:aprsme, :env)
+      Application.put_env(:aprsme, :env, :prod)
+
+      try do
+        stats = BroadcastTaskSupervisor.get_stats()
+        assert is_float(stats.scheduler_usage)
+        assert stats.scheduler_usage >= 0.0
+      after
+        Application.put_env(:aprsme, :env, original)
+      end
+    end
   end
 
   describe "performance under load" do
diff --git a/test/aprsme/cluster/leader_election_test.exs b/test/aprsme/cluster/leader_election_test.exs
index 52fb2f2..4b3a61c 100644
--- a/test/aprsme/cluster/leader_election_test.exs
+++ b/test/aprsme/cluster/leader_election_test.exs
@@ -599,4 +599,82 @@ defmodule Aprsme.Cluster.LeaderElectionTest do
       assert new_state.leader_node == nil
     end
   end
+
+  describe "handle_info(:force_election_timeout, state)" do
+    test "single-node mode warning branch when no other nodes" do
+      # not election_forced, not leader → goes into the if branch.
+      # Node.list() returns [] in this single-node test.
+      state = %LeaderElection{
+        cluster_enabled: true,
+        election_forced: false,
+        is_leader: false,
+        leader_node: nil
+      }
+
+      assert {:noreply, new_state} = LeaderElection.handle_info(:force_election_timeout, state)
+      assert new_state.election_forced == true
+    end
+
+    test "no-op branch when election already forced" do
+      state = %LeaderElection{
+        cluster_enabled: true,
+        election_forced: true,
+        is_leader: false,
+        leader_node: nil
+      }
+
+      assert {:noreply, new_state} = LeaderElection.handle_info(:force_election_timeout, state)
+      assert new_state == state
+    end
+  end
+
+  describe "handle_info(:check_cluster_and_elect, state)" do
+    test "no-op branch when election_forced is true" do
+      state = %LeaderElection{election_forced: true}
+
+      assert {:noreply, ^state} =
+               LeaderElection.handle_info(:check_cluster_and_elect, state)
+    end
+
+    test "reschedules itself when no nodes are connected (cluster not formed)" do
+      state = %LeaderElection{election_forced: false}
+
+      # Node.list() returns [] in single-node test mode → reschedules in 2s.
+      assert {:noreply, new_state} =
+               LeaderElection.handle_info(:check_cluster_and_elect, state)
+
+      assert new_state.election_forced == false
+    end
+  end
+
+  describe "handle_info/2 for unrecognized messages" do
+    test "logs and replies :noreply with state unchanged" do
+      state = %LeaderElection{is_leader: false}
+      assert {:noreply, ^state} = LeaderElection.handle_info(:something_unknown, state)
+    end
+  end
+
+  describe "handle_call(:current_leader, _from, state)" do
+    test "returns the leader_node from state" do
+      state = %LeaderElection{leader_node: :some@node}
+      assert {:reply, :some@node, ^state} = LeaderElection.handle_call(:current_leader, self(), state)
+    end
+  end
+
+  describe "terminate/2" do
+    test "non-leader terminate returns :ok without unregistering global" do
+      state = %LeaderElection{is_leader: false, leader_node: nil}
+      assert :ok = LeaderElection.terminate(:shutdown, state)
+    end
+
+    test "leader terminate logs and unregisters global" do
+      :global.unregister_name(@election_key)
+      :global.register_name(@election_key, self())
+
+      state = %LeaderElection{is_leader: true, leader_node: node()}
+      assert :ok = LeaderElection.terminate(:shutdown, state)
+      # The global registration should now be cleared.
+      assert :global.whereis_name(@election_key) == :undefined
+    end
+  end
 end
diff --git a/test/aprsme/packets_test.exs b/test/aprsme/packets_test.exs
index af60445..f755f05 100644
--- a/test/aprsme/packets_test.exs
+++ b/test/aprsme/packets_test.exs
@@ -1811,4 +1811,36 @@ defmodule Aprsme.PacketsTest do
       assert is_list(result)
     end
   end
+
+  describe "store_bad_packet/2 with various error shapes" do
+    test "store_bad_packet/2 with binary packet_data and a struct error uses Exception.message" do
+      result = Packets.store_bad_packet("rawpacket-binary", %ArgumentError{message: "boom"})
+      assert match?({:ok, _}, result) or match?({:error, _}, result)
+    end
+
+    test "store_bad_packet/2 with binary packet_data and a non-struct error uses inspect" do
+      result = Packets.store_bad_packet("rawpacket-binary", :some_atom)
+      assert match?({:ok, _}, result) or match?({:error, _}, result)
+    end
+
+    test "store_bad_packet/2 with map packet_data and a typed map" do
+      result = Packets.store_bad_packet(%{raw_packet: "raw-x"}, %{type: "MyType", message: "explained"})
+      assert match?({:ok, _}, result) or match?({:error, _}, result)
+    end
+
+    test "store_bad_packet/2 with map packet_data and a struct error" do
+      result = Packets.store_bad_packet(%{raw_packet: "raw-y"}, %ArgumentError{message: "boom"})
+      assert match?({:ok, _}, result) or match?({:error, _}, result)
+    end
+
+    test "store_bad_packet/2 with map missing :raw_packet falls back to inspect" do
+      result = Packets.store_bad_packet(%{some: "junk"}, %{type: "X", message: "Y"})
+      assert match?({:ok, _}, result) or match?({:error, _}, result)
+    end
+
+    test "store_bad_packet/2 with binary packet_data and a typed-map error" do
+      result = Packets.store_bad_packet("rawpacket-binary", %{type: "Custom", message: "msg"})
+      assert match?({:ok, _}, result) or match?({:error, _}, result)
+    end
+  end
 end
diff --git a/test/aprsme_web/live/api_docs_live_test.exs b/test/aprsme_web/live/api_docs_live_test.exs
index f2fca31..9c0c11b 100644
--- a/test/aprsme_web/live/api_docs_live_test.exs
+++ b/test/aprsme_web/live/api_docs_live_test.exs
@@ -145,6 +145,35 @@ defmodule AprsmeWeb.ApiDocsLiveTest do
       refute rendered =~ "\"latitude\""
     end
 
+    test "packet with empty-string device_identifier exercises format_equipment empty path", %{conn: conn} do
+      # Empty-string device_identifier hits the "" -> nil branch (line 168) in format_equipment.
+      _packet =
+        packet_fixture(%{
+          sender: "K0EMPTY",
+          base_callsign: "K0EMPTY",
+          ssid: "0",
+          received_at: DateTime.utc_now(),
+          lat: Decimal.new("33.0"),
+          lon: Decimal.new("-96.5"),
+          has_position: true,
+          device_identifier: ""
+        })
+
+      {:ok, lv, _html} = live(conn, ~p"/api", on_error: :warn)
+
+      lv
+      |> element("#test_callsign")
+      |> render_change(%{"callsign" => "K0EMPTY"})
+
+      lv
+      |> form("form[phx-submit=\"test_api\"]", %{"callsign" => "K0EMPTY"})
+      |> render_submit()
+
+      :ok = Process.sleep(150)
+      rendered = render(lv)
+      assert rendered =~ "K0EMPTY"
+    end
+
     test "packet with message fields renders message block", %{conn: conn} do
       # Exercises format_message(packet) when at least one message field is set.
       _packet =
diff --git a/test/aprsme_web/live/map_live/integration_test.exs b/test/aprsme_web/live/map_live/integration_test.exs
index 31a09ba..bf4c5b8 100644
--- a/test/aprsme_web/live/map_live/integration_test.exs
+++ b/test/aprsme_web/live/map_live/integration_test.exs
@@ -158,4 +158,292 @@ defmodule AprsmeWeb.MapLive.IntegrationTest do
       assert render(view) =~ "aprs-map"
     end
   end
+
+  describe "MapLive handle_event coverage for less-exercised events" do
+    test "popup_closed, get_assigns, set_slideover_state, request_geolocation, geolocation_error", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/?lat=33.0&lng=-96.0&z=10", on_error: :warn)
+      render_hook(view, "map_ready", %{})
+
+      assert render_hook(view, "popup_closed", %{})
+      assert render_hook(view, "set_slideover_state", %{"open" => false})
+      assert render_hook(view, "set_slideover_state", %{"open" => true})
+      assert render_hook(view, "request_geolocation", %{})
+      assert render_hook(view, "geolocation_error", %{"error" => "denied"})
+      assert render_hook(view, "toggle_slideover", %{})
+    end
+
+    test "marker_hover_start and marker_hover_end exercise hover handlers", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/?lat=33.0&lng=-96.0&z=12", on_error: :warn)
+      render_hook(view, "map_ready", %{})
+
+      assert render_hook(view, "marker_hover_start", %{
+               "id" => "m-1",
+               "path" => "WIDE1-1",
+               "lat" => 33.0,
+               "lng" => -96.0
+             })
+
+      assert render_hook(view, "marker_hover_end", %{})
+    end
+
+    test "search_callsign with empty input no-ops", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/", on_error: :warn)
+      assert render_hook(view, "search_callsign", %{"callsign" => "   "})
+    end
+
+    test "track_callsign with empty input clears tracking", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/?call=ZZZ", on_error: :warn)
+      render_hook(view, "map_ready", %{})
+
+      # Empty callsign goes through the clear-tracking branch.
+      assert render_hook(view, "track_callsign", %{"callsign" => ""})
+    end
+
+    test "track_callsign with non-empty callsign that has a packet zooms and adds marker", %{conn: conn} do
+      # Seed a real packet so track_callsign exercises the latest_packet branch.
+      packet_fixture(%{
+        sender: "TRACKEV",
+        base_callsign: "TRACKEV",
+        ssid: "0",
+        received_at: DateTime.utc_now(),
+        lat: Decimal.new("45.0"),
+        lon: Decimal.new("-93.0"),
+        has_position: true,
+        path: "WIDE1-1"
+      })
+
+      {:ok, view, _html} = live(conn, "/", on_error: :warn)
+      render_hook(view, "map_ready", %{})
+      assert render_hook(view, "track_callsign", %{"callsign" => "TRACKEV"})
+    end
+
+    test "marker_hover_start with a path triggers RF path drawing", %{conn: conn} do
+      # Seed packets that match the path stations so RF path resolution finds them.
+      packet_fixture(%{
+        sender: "PATHST1",
+        base_callsign: "PATHST1",
+        ssid: "0",
+        received_at: DateTime.utc_now(),
+        lat: Decimal.new("33.5"),
+        lon: Decimal.new("-96.5"),
+        has_position: true,
+        path: ""
+      })
+
+      {:ok, view, _html} = live(conn, "/?lat=33.0&lng=-96.0&z=12", on_error: :warn)
+      render_hook(view, "map_ready", %{})
+
+      assert render_hook(view, "marker_hover_start", %{
+               "id" => "m-rf",
+               "path" => "PATHST1*,WIDE1-1",
+               "lat" => 33.0,
+               "lng" => -96.0
+             })
+    end
+
+    test "clear_tracking event clears the tracked callsign", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/?call=TRACK1", on_error: :warn)
+      render_hook(view, "map_ready", %{})
+      assert render_hook(view, "clear_tracking", %{})
+    end
+
+    test "update_callsign updates overlay_callsign", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/", on_error: :warn)
+      assert render_hook(view, "update_callsign", %{"callsign" => "AB1XYZ"})
+    end
+
+    test "update_trail_duration changes the threshold", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/?lat=33.0&lng=-96.0&z=10", on_error: :warn)
+      render_hook(view, "map_ready", %{})
+      assert render_hook(view, "update_trail_duration", %{"trail_duration" => "6"})
+    end
+
+    test "update_historical_hours changes assigns", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/?lat=33.0&lng=-96.0&z=10", on_error: :warn)
+      render_hook(view, "map_ready", %{})
+      assert render_hook(view, "update_historical_hours", %{"historical_hours" => "3"})
+    end
+
+    test "locate_me, set_location, clear_and_reload_markers events", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/", on_error: :warn)
+      assert render_hook(view, "locate_me", %{})
+      assert render_hook(view, "set_location", %{"lat" => 33.5, "lng" => -96.5})
+      assert render_hook(view, "clear_and_reload_markers", %{})
+    end
+
+    test "update_map_state with center and zoom", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/?lat=33.0&lng=-96.0&z=10", on_error: :warn)
+      render_hook(view, "map_ready", %{})
+
+      assert render_hook(view, "update_map_state", %{
+               "center" => %{"lat" => 34.0, "lng" => -95.0},
+               "zoom" => 11
+             })
+    end
+
+    test "marker_clicked event", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/", on_error: :warn)
+      assert render_hook(view, "marker_clicked", %{})
+    end
+
+    test "error_boundary_triggered event logs without crashing", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/", on_error: :warn)
+
+      assert render_hook(view, "error_boundary_triggered", %{
+               "message" => "test error",
+               "stack" => "test stack",
+               "component_id" => "comp-1"
+             })
+    end
+  end
+
+  describe "MapLive handle_info coverage" do
+    test ":cleanup_old_packets, :update_time_display, :reload_historical_packets do not crash", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/?lat=33.0&lng=-96.0&z=10", on_error: :warn)
+      render_hook(view, "map_ready", %{})
+
+      send(view.pid, :cleanup_old_packets)
+      send(view.pid, :update_time_display)
+      send(view.pid, :reload_historical_packets)
+      Process.sleep(50)
+      assert render(view) =~ "aprs-map"
+    end
+
+    test ":clear_rf_path message clears the RF path", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/?lat=33.0&lng=-96.0&z=10", on_error: :warn)
+      send(view.pid, :clear_rf_path)
+      Process.sleep(20)
+      assert render(view) =~ "aprs-map"
+    end
+
+    test "{:new_deployment, _} message updates deployed_at", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/", on_error: :warn)
+      send(view.pid, {:new_deployment, %{deployed_at: DateTime.utc_now()}})
+      Process.sleep(20)
+      assert render(view) =~ "aprs-map"
+    end
+
+    test "{:drain_connections, n} message processes drain", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/", on_error: :warn)
+      send(view.pid, {:drain_connections, 0})
+      Process.sleep(20)
+      assert render(view) =~ "aprs-map"
+    end
+
+    test ":initialize_replay message reaches handle_info_initialize_replay", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/?lat=33.0&lng=-96.0&z=10", on_error: :warn)
+      render_hook(view, "map_ready", %{})
+      send(view.pid, :initialize_replay)
+      Process.sleep(20)
+      assert render(view) =~ "aprs-map"
+    end
+
+    test "{:load_rf_path_station_packets, _} message hits get_latest_packets_for_callsigns", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/?lat=33.0&lng=-96.0&z=12", on_error: :warn)
+      send(view.pid, {:load_rf_path_station_packets, ["FOO", "BAR"]})
+      Process.sleep(20)
+      assert render(view) =~ "aprs-map"
+    end
+
+    test "{:show_error, message} message sets a flash", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/", on_error: :warn)
+      send(view.pid, {:show_error, "boom"})
+      Process.sleep(20)
+      assert render(view) =~ "aprs-map"
+    end
+
+    test "{:load_historical_batch, _} backward-compat message", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/?lat=33.0&lng=-96.0&z=10", on_error: :warn)
+      render_hook(view, "map_ready", %{})
+      send(view.pid, {:load_historical_batch, 0})
+      Process.sleep(20)
+      assert render(view) =~ "aprs-map"
+    end
+
+    test "{:load_historical_batch, _, generation} matching current generation", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/?lat=33.0&lng=-96.0&z=10", on_error: :warn)
+      render_hook(view, "map_ready", %{})
+
+      # Use the loading_generation that the LV currently has (default 0).
+      send(view.pid, {:load_historical_batch, 0, 0})
+      Process.sleep(20)
+      assert render(view) =~ "aprs-map"
+    end
+
+    test "{:load_historical_batch, _, stale_gen} with stale generation is ignored", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/?lat=33.0&lng=-96.0&z=10", on_error: :warn)
+      render_hook(view, "map_ready", %{})
+
+      # Use a wildly mismatched generation — the handler should ignore it.
+      send(view.pid, {:load_historical_batch, 0, 99_999})
+      Process.sleep(20)
+      assert render(view) =~ "aprs-map"
+    end
+
+    test "{:historical_loading_timeout, _} for stale generation no-ops", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/?lat=33.0&lng=-96.0&z=10", on_error: :warn)
+      send(view.pid, {:historical_loading_timeout, 99_999})
+      Process.sleep(20)
+      assert render(view) =~ "aprs-map"
+    end
+
+    test "{:process_pending_bounds} message", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/?lat=33.0&lng=-96.0&z=10", on_error: :warn)
+      send(view.pid, {:process_pending_bounds})
+      Process.sleep(20)
+      assert render(view) =~ "aprs-map"
+    end
+
+    test "Logger.debug-laden bounds-update path runs format strings at :debug level", %{conn: conn} do
+      original_level = Logger.level()
+      Logger.configure(level: :debug)
+
+      try do
+        {:ok, view, _html} = live(conn, "/?lat=33.0&lng=-96.0&z=10", on_error: :warn)
+        render_hook(view, "map_ready", %{})
+
+        # Send bounds events so that Logger.debug calls in process_bounds_update,
+        # handle_info_process_bounds_update, and process_bounds_from_params get
+        # their string interpolations evaluated.
+        send(view.pid, {:process_bounds_update, %{north: 34.0, south: 32.0, east: -95.0, west: -97.0}})
+
+        render_hook(view, "bounds_changed", %{
+          "bounds" => %{"north" => 34.0, "south" => 32.0, "east" => -95.0, "west" => -97.0}
+        })
+
+        render_hook(view, "update_map_state", %{
+          "center" => %{"lat" => 33.5, "lng" => -96.5},
+          "zoom" => 11,
+          "bounds" => %{"north" => 34.0, "south" => 32.0, "east" => -95.0, "west" => -97.0}
+        })
+
+        Process.sleep(50)
+      after
+        Logger.configure(level: original_level)
+      end
+    end
+
+    test "postgres_packet via PubSub", %{conn: conn} do
+      {:ok, view, _html} = live(conn, "/?lat=33.0&lng=-96.0&z=12", on_error: :warn)
+      render_hook(view, "map_ready", %{})
+
+      packet = %{
+        id: "pg-1",
+        sender: "PG-1",
+        base_callsign: "PG",
+        ssid: "1",
+        lat: 33.0,
+        lon: -96.0,
+        has_position: true,
+        symbol_table_id: "/",
+        symbol_code: ">",
+        received_at: DateTime.utc_now(),
+        path: "WIDE1-1"
+      }
+
+      send(view.pid, {:postgres_packet, packet})
+      Process.sleep(50)
+      assert render(view) =~ "aprs-map"
+    end
+  end
 end
diff --git a/test/aprsme_web/live/status_live/index_test.exs b/test/aprsme_web/live/status_live/index_test.exs
index a88f50e..953f161 100644
--- a/test/aprsme_web/live/status_live/index_test.exs
+++ b/test/aprsme_web/live/status_live/index_test.exs
@@ -248,4 +248,35 @@ defmodule AprsmeWeb.StatusLive.IndexTest do
       Aprsme.Cache.del(:query_cache, "aprs_status")
     end
   end
+
+  describe "{:status_updated, status} update path" do
+    test "applies a fresh status map and updates loading=false" do
+      socket = %Socket{
+        assigns: %{
+          aprs_status: %{},
+          current_time: DateTime.utc_now(),
+          health_score: 1,
+          loading: true,
+          __changed__: %{}
+        }
+      }
+
+      status = %{
+        connected: true,
+        server: "test",
+        port: 14_580,
+        connected_at: DateTime.utc_now(),
+        uptime_seconds: 100,
+        login_id: "N0CALL",
+        filter: "r/33/-96/100",
+        packet_stats: %{total_packets: 1, packets_per_second: 0, last_packet_at: DateTime.utc_now()},
+        stored_packet_count: 0,
+        oldest_packet_timestamp: nil
+      }
+
+      assert {:noreply, new_socket} = Index.handle_info({:status_updated, status}, socket)
+      assert new_socket.assigns.loading == false
+      assert new_socket.assigns.aprs_status == status
+    end
+  end
 end