From 4c5c730ee747c3dd5feba2520e674a59441966e5 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Sat, 9 May 2026 10:56:03 -0500 Subject: [PATCH] Speed up the slowest tests - packet_replay: stop_replay test reduces fake-task receive timeout from 5s to 200ms - broadcast_task_supervisor: scheduler_usage takes configurable sample seconds; test uses sample-pair API (instant) instead of blocking 1-second sample - health_check: ask_if_alive timeout is now configurable; unresponsive-pid test uses 50ms timeout instead of waiting full 5-second default - mix_unused/analyzer_test: cache analyze() result in setup_all so 4 tests share one analyze pass instead of running 4 separate scans - mix/tasks/compile/unused_test: consolidate three slow severity tests into one - log_sanitizer + packet_field_whitelist: cap property-test runs at 25 (was 100) - historical_loading: trim 200ms post-event sleeps to 50ms - movement: refute_push_event timeout from 200ms to 50ms Total suite time: ~45s -> 40.6s; 2488 tests, 0 failures. --- lib/aprsme/broadcast_task_supervisor.ex | 29 ++++++++++++---- lib/aprsme_web/plugs/health_check.ex | 3 +- .../aprsme/broadcast_task_supervisor_test.exs | 14 +++++--- test/aprsme/log_sanitizer_test.exs | 2 +- test/aprsme/packet_field_whitelist_test.exs | 6 ++-- test/aprsme/packet_replay_test.exs | 2 +- .../live/map_live/historical_loading_test.exs | 4 +-- .../live/map_live/movement_test.exs | 3 +- test/aprsme_web/plugs/health_check_test.exs | 7 ++-- test/mix/tasks/compile/unused_test.exs | 33 +++++++++---------- test/mix_unused/analyzer_test.exs | 26 ++++++++------- 11 files changed, 77 insertions(+), 52 deletions(-) diff --git a/lib/aprsme/broadcast_task_supervisor.ex b/lib/aprsme/broadcast_task_supervisor.ex index 83d9362..12fbaf3 100644 --- a/lib/aprsme/broadcast_task_supervisor.ex +++ b/lib/aprsme/broadcast_task_supervisor.ex @@ -97,17 +97,34 @@ defmodule Aprsme.BroadcastTaskSupervisor do if Application.get_env(:aprsme, :env) == :test do 0.0 else - 1 - |> :scheduler.utilization() - |> Enum.map(fn {_, usage, _} -> usage end) - |> Enum.sum() - |> Kernel./(System.schedulers_online()) - |> Float.round(2) + sample_seconds = Application.get_env(:aprsme, :scheduler_sample_seconds, 1) + compute_scheduler_usage(sample_seconds) end rescue _ -> 0.0 end + # When sample_seconds is 0, use sample-pair API to avoid blocking. + defp compute_scheduler_usage(0) do + sample = :scheduler.sample_all() + + sample + |> :scheduler.utilization() + |> Enum.map(fn {_, usage, _} -> usage end) + |> Enum.sum() + |> Kernel./(System.schedulers_online()) + |> Float.round(2) + end + + defp compute_scheduler_usage(seconds) when is_integer(seconds) and seconds > 0 do + seconds + |> :scheduler.utilization() + |> Enum.map(fn {_, usage, _} -> usage end) + |> Enum.sum() + |> Kernel./(System.schedulers_online()) + |> Float.round(2) + end + # Private helper to broadcast to multiple topics defp broadcast_to_topics(topics, message, pubsub) do Enum.each(topics, fn topic -> diff --git a/lib/aprsme_web/plugs/health_check.ex b/lib/aprsme_web/plugs/health_check.ex index 23d8059..791438d 100644 --- a/lib/aprsme_web/plugs/health_check.ex +++ b/lib/aprsme_web/plugs/health_check.ex @@ -105,7 +105,8 @@ defmodule AprsmeWeb.Plugs.HealthCheck do defp ask_if_alive(false, _pid), do: false defp ask_if_alive(true, pid) do - GenServer.call(pid, :shutting_down?, 5000) + timeout = Application.get_env(:aprsme, :health_check_call_timeout_ms, 5000) + GenServer.call(pid, :shutting_down?, timeout) catch _kind, _reason -> false end diff --git a/test/aprsme/broadcast_task_supervisor_test.exs b/test/aprsme/broadcast_task_supervisor_test.exs index 88b99af..8519adf 100644 --- a/test/aprsme/broadcast_task_supervisor_test.exs +++ b/test/aprsme/broadcast_task_supervisor_test.exs @@ -148,17 +148,23 @@ defmodule Aprsme.BroadcastTaskSupervisorTest do 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) + # live :scheduler.utilization branch. Sample seconds=0 uses the + # instant sample-pair API rather than blocking for a full second. + original_env = Application.get_env(:aprsme, :env) + original_seconds = Application.get_env(:aprsme, :scheduler_sample_seconds) Application.put_env(:aprsme, :env, :prod) + Application.put_env(:aprsme, :scheduler_sample_seconds, 0) 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) + Application.put_env(:aprsme, :env, original_env) + + if original_seconds, + do: Application.put_env(:aprsme, :scheduler_sample_seconds, original_seconds), + else: Application.delete_env(:aprsme, :scheduler_sample_seconds) end end end diff --git a/test/aprsme/log_sanitizer_test.exs b/test/aprsme/log_sanitizer_test.exs index 040b813..746f79c 100644 --- a/test/aprsme/log_sanitizer_test.exs +++ b/test/aprsme/log_sanitizer_test.exs @@ -235,7 +235,7 @@ defmodule Aprsme.LogSanitizerTest do describe "property: sanitize/1 is idempotent" do property "running sanitize twice equals running it once on maps" do - check all(map <- map_of(atom(:alphanumeric), term())) do + check all(map <- map_of(atom(:alphanumeric), term()), max_runs: 25) do once = LogSanitizer.sanitize(map) twice = LogSanitizer.sanitize(once) assert once == twice diff --git a/test/aprsme/packet_field_whitelist_test.exs b/test/aprsme/packet_field_whitelist_test.exs index 0676ec5..00ecfa3 100644 --- a/test/aprsme/packet_field_whitelist_test.exs +++ b/test/aprsme/packet_field_whitelist_test.exs @@ -104,7 +104,7 @@ defmodule Aprsme.PacketFieldWhitelistTest do describe "property: filter_fields/1 is idempotent" do property "running filter twice equals running once" do - check all(map <- map_of(atom(:alphanumeric), term())) do + check all(map <- map_of(atom(:alphanumeric), term()), max_runs: 25) do once = PacketFieldWhitelist.filter_fields(map) twice = PacketFieldWhitelist.filter_fields(once) assert once == twice @@ -112,14 +112,14 @@ defmodule Aprsme.PacketFieldWhitelistTest do end property "every key in the output satisfies allowed?/1" do - check all(map <- map_of(atom(:alphanumeric), term())) do + check all(map <- map_of(atom(:alphanumeric), term()), max_runs: 25) do result = PacketFieldWhitelist.filter_fields(map) assert Enum.all?(Map.keys(result), &PacketFieldWhitelist.allowed?/1) end end property "invalid_fields and filter_fields partition the input" do - check all(map <- map_of(atom(:alphanumeric), term())) do + check all(map <- map_of(atom(:alphanumeric), term()), max_runs: 25) do kept = map |> PacketFieldWhitelist.filter_fields() |> Map.keys() |> Enum.map(&to_string/1) dropped = PacketFieldWhitelist.invalid_fields(map) all_keys = map |> Map.keys() |> Enum.map(&to_string/1) diff --git a/test/aprsme/packet_replay_test.exs b/test/aprsme/packet_replay_test.exs index 0666225..ecacae3 100644 --- a/test/aprsme/packet_replay_test.exs +++ b/test/aprsme/packet_replay_test.exs @@ -154,7 +154,7 @@ defmodule Aprsme.PacketReplayTest do receive do :stop -> :ok after - 5_000 -> :ok + 200 -> :ok end 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 4f92130..bb8f461 100644 --- a/test/aprsme_web/live/map_live/historical_loading_test.exs +++ b/test/aprsme_web/live/map_live/historical_loading_test.exs @@ -222,7 +222,7 @@ defmodule AprsmeWeb.MapLive.HistoricalLoadingTest do assert render_hook(view, "bounds_changed", %{"bounds" => bounds}) # Wait for historical loading - Process.sleep(200) + Process.sleep(50) # At zoom level 5, the limit should be 100 packets # This is defined in HistoricalLoader's @zoom_packet_limits @@ -272,7 +272,7 @@ defmodule AprsmeWeb.MapLive.HistoricalLoadingTest do assert render_hook(view, "bounds_changed", %{"bounds" => bounds}) # Wait for historical loading - Process.sleep(200) + Process.sleep(50) # Verify all packets would be included in the query recent_packets = diff --git a/test/aprsme_web/live/map_live/movement_test.exs b/test/aprsme_web/live/map_live/movement_test.exs index 2f96682..c1a616d 100644 --- a/test/aprsme_web/live/map_live/movement_test.exs +++ b/test/aprsme_web/live/map_live/movement_test.exs @@ -71,8 +71,7 @@ defmodule AprsmeWeb.MapLive.MovementTest do send(view.pid, {:postgres_packet, drift_packet}) # The view should not push a new_packet event for GPS drift - # Using a longer timeout to avoid flaky tests on slower systems - refute_push_event(view, "new_packet", %{id: "TEST-1"}, 200) + refute_push_event(view, "new_packet", %{id: "TEST-1"}, 50) end test "updates marker for significant movement", %{conn: conn} do diff --git a/test/aprsme_web/plugs/health_check_test.exs b/test/aprsme_web/plugs/health_check_test.exs index cd46c1c..7c2640a 100644 --- a/test/aprsme_web/plugs/health_check_test.exs +++ b/test/aprsme_web/plugs/health_check_test.exs @@ -151,16 +151,17 @@ defmodule AprsmeWeb.Plugs.HealthCheckTest do test "returns 200 OK even when registered ShutdownHandler-named pid is unresponsive", %{conn: conn} do Application.put_env(:aprsme, :health_status, :healthy) + Application.put_env(:aprsme, :health_check_call_timeout_ms, 50) original_pid = Process.whereis(Aprsme.ShutdownHandler) # Register a dummy task pid under the name. GenServer.call to it will - # exit; the catch clause in ask_if_alive returns false → readiness OK. + # time out (50ms); the catch clause in ask_if_alive returns false → readiness OK. if original_pid do :erlang.unregister(Aprsme.ShutdownHandler) end - task = Task.async(fn -> Process.sleep(2_000) end) + task = Task.async(fn -> Process.sleep(500) end) Process.register(task.pid, Aprsme.ShutdownHandler) try do @@ -169,6 +170,8 @@ defmodule AprsmeWeb.Plugs.HealthCheckTest do # The dummy pid is alive but won't respond to :shutting_down? → catch → false → OK assert result.status == 200 after + Application.delete_env(:aprsme, :health_check_call_timeout_ms) + if Process.whereis(Aprsme.ShutdownHandler) == task.pid do :erlang.unregister(Aprsme.ShutdownHandler) end diff --git a/test/mix/tasks/compile/unused_test.exs b/test/mix/tasks/compile/unused_test.exs index c13e3ed..3b7e367 100644 --- a/test/mix/tasks/compile/unused_test.exs +++ b/test/mix/tasks/compile/unused_test.exs @@ -22,27 +22,24 @@ defmodule Mix.Tasks.Compile.UnusedTest do assert Unused.clean() == :ok end - test "runs against the project and returns either :ok or :error tuple" do - ExUnit.CaptureIO.capture_io(fn -> - result = Unused.run([]) - assert match?({:ok, _}, result) or match?({:error, _}, result) - end) - end + test "exercises run/1 across all severity arg variants" do + # Each Unused.run/1 call invokes the analyzer (which scans all BEAMs + # and is the slow part). We exercise every severity branch in a + # single test so we only pay analyze cost a small number of times. + arg_lists = [ + [], + ["--severity", "error"], + ["--severity", "warning"], + ["--severity", "info"], + ["--severity", "garbage"] + ] - test "honors --severity option override" do ExUnit.CaptureIO.capture_io(fn -> - result = Unused.run(["--severity", "error"]) - assert match?({:ok, _}, result) or match?({:error, _}, result) - end) - end - - test "accepts --severity warning, info, and unknown values" do - for severity <- ["warning", "info", "garbage"] do - ExUnit.CaptureIO.capture_io(fn -> - result = Unused.run(["--severity", severity]) + for args <- arg_lists do + result = Unused.run(args) assert match?({:ok, _}, result) or match?({:error, _}, result) - end) - end + end + end) end end end diff --git a/test/mix_unused/analyzer_test.exs b/test/mix_unused/analyzer_test.exs index 0574eab..553bbf0 100644 --- a/test/mix_unused/analyzer_test.exs +++ b/test/mix_unused/analyzer_test.exs @@ -3,6 +3,13 @@ defmodule MixUnused.AnalyzerTest do alias MixUnused.Analyzer + setup_all do + # Cache one analyze() result and reuse across tests. analyze() walks + # every BEAM file and is the slow part — running it once per module + # avoids paying that cost per test. + {:ok, baseline: Analyzer.analyze()} + end + describe "format/1" do test "renders a friendly message when there are no entries" do assert Analyzer.format([]) =~ "No unused public functions" @@ -47,9 +54,7 @@ defmodule MixUnused.AnalyzerTest do end describe "analyze/1 against the running project" do - test "returns a list of unused-entry maps" do - result = Analyzer.analyze() - + test "returns a list of unused-entry maps", %{baseline: result} do assert is_list(result) # Every entry has the expected shape. @@ -62,9 +67,7 @@ defmodule MixUnused.AnalyzerTest do end) end - test "result is sorted by module then name then arity" do - result = Analyzer.analyze() - + test "result is sorted by module then name then arity", %{baseline: result} do sorted = Enum.sort_by(result, fn e -> {Atom.to_string(e.module), e.name, e.arity} end) @@ -75,8 +78,8 @@ defmodule MixUnused.AnalyzerTest do Enum.map(sorted, & &1.module) end - test "exclude option removes whole modules from the output" do - [first | _] = Analyzer.analyze() + test "exclude option removes whole modules from the output", %{baseline: result} do + [first | _] = result assert first @@ -84,8 +87,7 @@ defmodule MixUnused.AnalyzerTest do refute Enum.any?(excluded, &(&1.module == first.module)) end - test "exclude option removes specific MFA triples" do - result = Analyzer.analyze() + test "exclude option removes specific MFA triples", %{baseline: result} do [first | _] = result assert first @@ -136,8 +138,8 @@ defmodule MixUnused.AnalyzerTest do assert result == [] end - test "exclude with both module atoms and MFA tuples filters both kinds" do - [first, second | _] = Analyzer.analyze() + test "exclude with both module atoms and MFA tuples filters both kinds", %{baseline: result} do + [first, second | _] = result excluded = Analyzer.analyze(exclude: [first.module, {second.module, second.name, second.arity}])