From ed21c86214a56cd41c3e307ba822e4b4cf6070c8 Mon Sep 17 00:00:00 2001 From: Mauricio Camayo Date: Sun, 30 Aug 2026 11:31:19 -0500 Subject: [PATCH] test: assert exact history window instead of len() >= 1 CodeRabbit follow-up on PR #1765 (https://github.com/netalertx/NetAlertX/pull/1765#discussion_r3888374014): test_main_history_length_never_produces_empty_or_growing_unbounded only asserted len(history) >= 1, which a mis-clamped history_length (e.g. keeping 4 items instead of 1) would still pass unnoticed. Replaced with test_main_history_length_clamps_and_trims_exactly, seeding distinct ordered values and asserting the exact retained history against each PIHOLEMON_HISTORY_LENGTH boundary. Verified it actually catches a broken clamp: temporarily reverted the max(1, ...) fix in pihole_monitor.py, confirmed this test fails ([] == [40]) while the rest of the suite still passes, then restored the fix. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01CHJAArRiet4GmXUsxnNLdW --- test/plugins/test_pihole_monitor.py | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/test/plugins/test_pihole_monitor.py b/test/plugins/test_pihole_monitor.py index 11181ea98..22ef34433 100644 --- a/test/plugins/test_pihole_monitor.py +++ b/test/plugins/test_pihole_monitor.py @@ -469,21 +469,30 @@ def test_main_records_anomaly_when_stats_are_complete(isolated_state, settings): assert state_after["aa:bb:cc:dd:ee:01"] == [10, 10, 10, 50] -@pytest.mark.parametrize("configured_length", [-5, 0, 1, 28]) -def test_main_history_length_never_produces_empty_or_growing_unbounded(isolated_state, settings, configured_length): - """0 already falls back to 28 via `or`; negative values must clamp to - at least 1 rather than reach a nonsensical slice.""" +@pytest.mark.parametrize( + ("configured_length", "expected_history"), + [ + (-5, [40]), # negative - must clamp to 1, keeping only the newest sample + (0, [10, 20, 30, 40]), # falsy - already falls back to 28 via `or`, nothing trimmed + (1, [40]), # explicit 1 - only the newest sample survives + (28, [10, 20, 30, 40]), # the documented default - well under the cap, nothing trimmed + ], +) +def test_main_history_length_clamps_and_trims_exactly(isolated_state, settings, configured_length, expected_history): + """Distinct, ordered seed values (not len() alone) so a wrong slice + window - e.g. a length-1 clamp that actually kept 4 items, which a + bare `len(history) >= 1` check would miss - shows up as a mismatch.""" settings["PIHOLEMON_HISTORY_LENGTH"] = configured_length device = [_device_payload("aa:bb:cc:dd:ee:01", "10.0.0.5")] with patch.object(pihole_monitor.PiholeSource, "fetch_devices", return_value=device), \ - patch.object(pihole_monitor.PiholeSource, "fetch_top_blocked_clients", return_value={"10.0.0.5": 1}), \ + patch.object(pihole_monitor.PiholeSource, "fetch_top_blocked_clients", return_value={"10.0.0.5": 40}), \ patch.object(pihole_monitor, "Plugin_Objects"): - pihole_monitor.save_state({"aa:bb:cc:dd:ee:01": [1, 1, 1]}) + pihole_monitor.save_state({"aa:bb:cc:dd:ee:01": [10, 20, 30]}) assert pihole_monitor.main() == 0 history = pihole_monitor.load_state()["aa:bb:cc:dd:ee:01"] - assert len(history) >= 1 # never empty - a 0/negative slice length would be a bug + assert history == expected_history def test_main_returns_1_when_no_source_is_configured(isolated_state, settings):