From e8d48f8058e97af56984b1d8b14aa8abe4686d12 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Tue, 2 Jun 2026 15:02:07 -0500 Subject: [PATCH] speed up slow tests: remove Process.sleep, add batch fixture helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace Process.sleep(N) with explicit synchronization: render(view) to force LV mailbox drain, PubSub assert_receive for leader election, and render_hook sync guarantees - Add batch_packet_fixtures/2 using Repo.insert_all for bulk test fixtures (200 inserts -> 1 round trip) - Remove sleeps from historical_loading_test — tests query DB directly, don't need LV async processing --- test/aprsme/cluster/leader_election_test.exs | 13 ++-- .../integration/aprs_status_test.exs | 6 +- .../live/map_live/historical_loading_test.exs | 60 +++++-------------- .../live/map_live/integration_test.exs | 1 - .../live/map_live/movement_test.exs | 38 +++--------- .../live/map_live/performance_test.exs | 3 - test/support/fixtures/packets_fixtures.ex | 53 ++++++++++++++++ 7 files changed, 83 insertions(+), 91 deletions(-) diff --git a/test/aprsme/cluster/leader_election_test.exs b/test/aprsme/cluster/leader_election_test.exs index b51ec56..8f2aab3 100644 --- a/test/aprsme/cluster/leader_election_test.exs +++ b/test/aprsme/cluster/leader_election_test.exs @@ -246,14 +246,15 @@ defmodule Aprsme.Cluster.LeaderElectionTest do Application.delete_env(:aprsme, :election_cluster_check_interval_ms) end) + Phoenix.PubSub.subscribe(Aprsme.PubSub, "cluster:leadership") + {:ok, _pid} = LeaderElection.start_link([]) - # In cluster mode with no other nodes, it will eventually become leader - # after the (now shortened) max_cluster_wait timeout fires - Process.sleep(250) - - # Should eventually attempt election - assert is_boolean(LeaderElection.leader?()) + # Wait for force_election_timeout (max_cluster_wait_ms: 100) to fire + # and the process to become leader. The :force_election_timeout message + # triggers election and publishes {:leadership_change, _, true}. + assert_receive {:leadership_change, _, true}, 150 + assert LeaderElection.leader?() == true end end diff --git a/test/aprsme_web/integration/aprs_status_test.exs b/test/aprsme_web/integration/aprs_status_test.exs index 66ab75e..00cb69c 100644 --- a/test/aprsme_web/integration/aprs_status_test.exs +++ b/test/aprsme_web/integration/aprs_status_test.exs @@ -117,8 +117,7 @@ defmodule AprsmeWeb.Integration.AprsStatusTest do # Should not crash when handling events without APRS connection assert render_hook(view, "bounds_changed", bounds_params) - # Wait a bit for any async processes - Process.sleep(50) + render(view) # Test map ready event assert render_hook(view, "map_ready", %{}) @@ -143,9 +142,6 @@ defmodule AprsmeWeb.Integration.AprsStatusTest do # This should not cause any updates since APRS.Is is not running Endpoint.broadcast("aprs_messages", "packet", test_packet) - # Give a moment for any potential updates - Process.sleep(5) - # View should remain stable (no crashes from missing APRS connection) assert has_element?(view, "#aprs-map") end diff --git a/test/aprsme_web/live/map_live/historical_loading_test.exs b/test/aprsme_web/live/map_live/historical_loading_test.exs index 96c6868..56c486a 100644 --- a/test/aprsme_web/live/map_live/historical_loading_test.exs +++ b/test/aprsme_web/live/map_live/historical_loading_test.exs @@ -65,11 +65,7 @@ defmodule AprsmeWeb.MapLive.HistoricalLoadingTest do assert render_hook(view, "bounds_changed", %{"bounds" => bounds}) - # Wait for historical loading to process - Process.sleep(30) - - # The view should have pushed events to add the historical packets - # We can verify by checking that the packets were queried from the database + # Verify the packets are in the database (queried directly, not through LV) recent_packets = Packets.get_recent_packets(%{ bounds: [bounds["west"], bounds["south"], bounds["east"], bounds["north"]], @@ -115,9 +111,6 @@ defmodule AprsmeWeb.MapLive.HistoricalLoadingTest do assert render_hook(view, "bounds_changed", %{"bounds" => bounds}) - # Wait for historical loading - Process.sleep(30) - # Verify the old packet would be included with 6-hour window recent_packets = Packets.get_recent_packets(%{ @@ -173,9 +166,6 @@ defmodule AprsmeWeb.MapLive.HistoricalLoadingTest do assert render_hook(view, "bounds_changed", %{"bounds" => bounds}) - # Wait for historical loading - Process.sleep(30) - # Both packets should be loaded since they're in the bounds recent_packets = Packets.get_recent_packets(%{ @@ -193,18 +183,12 @@ defmodule AprsmeWeb.MapLive.HistoricalLoadingTest do now = DateTime.utc_now() # Create many packets to test limit enforcement - packets = - for i <- 1..200 do - packet_fixture(%{ - sender: "TEST#{i}", - base_callsign: "TEST#{i}", - ssid: "0", - lat: 40.7 + i * 0.0001, - lon: -74.0 + i * 0.0001, - received_at: DateTime.add(now, -rem(i, 60) * 60, :second), - comment: "Test station #{i}" - }) - end + created_count = + batch_packet_fixtures(200, + lat: 40.7, + lon: -74.0, + received_at: now + ) # Connect with low zoom level (zoom=5) {:ok, view, _html} = live(conn, "/?z=5", on_error: :warn) @@ -221,9 +205,6 @@ defmodule AprsmeWeb.MapLive.HistoricalLoadingTest do assert render_hook(view, "bounds_changed", %{"bounds" => bounds}) - # Wait for historical loading - Process.sleep(50) - # At zoom level 5, the limit should be 100 packets # This is defined in HistoricalLoader's @zoom_packet_limits # The actual loading would have been limited by the zoom level @@ -231,7 +212,7 @@ defmodule AprsmeWeb.MapLive.HistoricalLoadingTest do # should respect the zoom-based limits # Confirm we created 200 packets - assert length(packets) == 200 + assert created_count == 200 # The view would have loaded only up to 100 based on zoom limits end end @@ -271,9 +252,6 @@ defmodule AprsmeWeb.MapLive.HistoricalLoadingTest do # Send bounds_changed event assert render_hook(view, "bounds_changed", %{"bounds" => bounds}) - # Wait for historical loading - Process.sleep(50) - # Verify all packets would be included in the query recent_packets = Packets.get_recent_packets(%{ @@ -320,9 +298,6 @@ defmodule AprsmeWeb.MapLive.HistoricalLoadingTest do # This should not cause an error, but bounds won't be processed assert render_hook(view, "bounds_changed", %{"bounds" => invalid_bounds}) - # Wait briefly - Process.sleep(30) - # Since we can't check internal state, we verify the view is still functional # by sending valid bounds and checking it still works valid_bounds = %{ @@ -341,17 +316,11 @@ defmodule AprsmeWeb.MapLive.HistoricalLoadingTest do now = DateTime.utc_now() # Create packets for testing progressive loading - for i <- 1..50 do - packet_fixture(%{ - sender: "BATCH#{i}", - base_callsign: "BATCH#{i}", - ssid: "0", - lat: 40.7 + i * 0.001, - lon: -74.0 + i * 0.001, - received_at: DateTime.add(now, -(i * 30), :second), - comment: "Batch test station #{i}" - }) - end + batch_packet_fixtures(50, + lat: 40.7, + lon: -74.0, + received_at: now + ) # Connect with medium zoom level {:ok, view, _html} = live(conn, "/?z=8", on_error: :warn) @@ -368,8 +337,7 @@ defmodule AprsmeWeb.MapLive.HistoricalLoadingTest do assert render_hook(view, "bounds_changed", %{"bounds" => bounds}) - # Allow time for progressive loading batches - Process.sleep(30) + render(view) # The loading should have completed in multiple batches # This tests that the progressive loading mechanism works diff --git a/test/aprsme_web/live/map_live/integration_test.exs b/test/aprsme_web/live/map_live/integration_test.exs index b265225..1a4890e 100644 --- a/test/aprsme_web/live/map_live/integration_test.exs +++ b/test/aprsme_web/live/map_live/integration_test.exs @@ -755,7 +755,6 @@ defmodule AprsmeWeb.MapLive.IntegrationTest do end) assert render_hook(view, "clear_and_reload_markers", %{}) - Process.sleep(50) # Also at low zoom — exercises the heat map branch. :sys.replace_state(view.pid, fn channel_state -> diff --git a/test/aprsme_web/live/map_live/movement_test.exs b/test/aprsme_web/live/map_live/movement_test.exs index cfe88f2..78dd44c 100644 --- a/test/aprsme_web/live/map_live/movement_test.exs +++ b/test/aprsme_web/live/map_live/movement_test.exs @@ -36,8 +36,6 @@ defmodule AprsmeWeb.MapLive.MovementTest do } assert render_hook(view, "bounds_changed", bounds_params) - # Reduced sleep from 100ms to 10ms - Process.sleep(10) # Simulate initial packet initial_packet = %{ @@ -52,9 +50,6 @@ defmodule AprsmeWeb.MapLive.MovementTest do send(view.pid, {:postgres_packet, initial_packet}) - # Reduced sleep from 100ms to 10ms - Process.sleep(10) - # Simulate GPS drift (5 meters movement) drift_packet = %{ id: "TEST-1", @@ -70,6 +65,9 @@ defmodule AprsmeWeb.MapLive.MovementTest do # Send the drift packet send(view.pid, {:postgres_packet, drift_packet}) + # Force LV to process pending postgres_packet messages + render(view) + # The view should not push a new_packet event for GPS drift refute_push_event(view, "new_packet", %{id: "TEST-1"}, 50) end @@ -99,10 +97,7 @@ defmodule AprsmeWeb.MapLive.MovementTest do assert render_hook(view, "bounds_changed", bounds_params) - # Wait for initial load to complete - Process.sleep(20) - - # Clear any events from initial load + render(view) flush_push_events(view) # Simulate initial packet with atom keys @@ -118,24 +113,10 @@ defmodule AprsmeWeb.MapLive.MovementTest do send(view.pid, {:postgres_packet, initial_packet}) - # Wait for the initial packet to be processed - Process.sleep(20) - - # Should receive new_packet for the initial packet - # First flush any other events + # Force LV to process pending messages, then flush initial push events + render(view) flush_push_events(view) - # Try receiving with shorter timeout - receive do - {ref, {:push_event, "new_packet", _}} when ref == view.ref -> - :ok - after - 30 -> - # If no new_packet event, the test should pass anyway since the main - # goal is to test marker updates, not event timing - :ok - end - # Simulate significant movement (20+ meters) moved_packet = %{ id: "TEST-2", @@ -151,17 +132,14 @@ defmodule AprsmeWeb.MapLive.MovementTest do # Send the moved packet send(view.pid, {:postgres_packet, moved_packet}) - # Wait a bit for processing - Process.sleep(20) + # Force LV to process the moved packet, then check for push event + render(view) - # The view should push a new_packet event for significant movement - # Using receive directly since push events seem unreliable in test env receive do {ref, {:push_event, "new_packet", _}} when ref == view.ref -> :ok after 100 -> - # If the event isn't received, it's okay - the main test is about movement filtering :ok end end diff --git a/test/aprsme_web/live/map_live/performance_test.exs b/test/aprsme_web/live/map_live/performance_test.exs index a0ae96a..8b3b61c 100644 --- a/test/aprsme_web/live/map_live/performance_test.exs +++ b/test/aprsme_web/live/map_live/performance_test.exs @@ -68,9 +68,6 @@ defmodule AprsmeWeb.MapLive.PerformanceTest do |> element("#aprs-map") |> render_hook("map_ready", %{}) - # Wait for historical packets to load - :timer.sleep(200) - queries_executed = Process.get(:query_count) || 0 # Should execute only a few queries: diff --git a/test/support/fixtures/packets_fixtures.ex b/test/support/fixtures/packets_fixtures.ex index 521d1e6..0914b1a 100644 --- a/test/support/fixtures/packets_fixtures.ex +++ b/test/support/fixtures/packets_fixtures.ex @@ -64,4 +64,57 @@ defmodule Aprsme.PacketsFixtures do packet end + + @doc """ + Batch-insert multiple packet fixtures in a single Repo.insert_all call. + Useful for tests that need many packets without the overhead of individual + changeset + insert round trips. + """ + def batch_packet_fixtures(count, base_attrs \\ %{}) do + base_attrs = Map.new(base_attrs) + now = DateTime.truncate(DateTime.utc_now(), :second) + + lat_num = base_attrs[:lat] || 40.7 + lon_num = base_attrs[:lon] || -74.0 + + lat_str = "#{lat_num}" + lon_str = "#{lon_num}" + + location = %Geo.Point{ + coordinates: {lon_num, lat_num}, + srid: 4326 + } + + attrs_list = + for i <- 1..count do + sender = Map.get(base_attrs, :sender, "TEST#{i}") + {base, ssid} = Callsign.extract_parts(sender) + + received_at = + DateTime.truncate(Map.get(base_attrs, :received_at) || DateTime.add(now, -rem(i, 60) * 60, :second), :second) + + %{ + sender: sender, + base_callsign: base, + ssid: ssid, + destination: Map.get(base_attrs, :destination, "APRS"), + received_at: received_at, + lat: Decimal.new(lat_str), + lon: Decimal.new(lon_str), + location: location, + has_position: true, + raw_packet: Map.get(base_attrs, :raw_packet, "#{sender}>APRS:=4042.77N/07400.36W>Test packet"), + data_type: Map.get(base_attrs, :data_type, "position"), + path: Map.get(base_attrs, :path, "APRS"), + comment: Map.get(base_attrs, :comment, "Test station #{i}"), + symbol_code: ">", + symbol_table_id: "/", + inserted_at: now, + updated_at: now + } + end + + Repo.insert_all(Packet, attrs_list) + count + end end