Files
LEDMatrix/test/test_wifi_manager_ap.py
T
ChuckandClaude Opus 5.5 84afa9d64f refactor: delete dead Python code in the core (and stop storing Wi-Fi passwords) (#608)
* refactor(plugins): remove the no-op PluginHealthMonitor

Its monitor loop did nothing (`if callbacks: pass`), register_health_check
had no callers and api_v3.health_monitor was never read by any route. The
live health data comes from PluginHealthTracker, which is untouched.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* refactor(store): drop the never-set uninstall tombstones

Nothing in production called mark_recently_uninstalled, so the
reconciler's was_recently_uninstalled check was always False. The
persistent uninstall registry is what actually stops resurrection; the
reconciler test now exercises that gate instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* refactor(common): delete unused config/display/game helpers, utils and error_handler

Nothing in core, the web UI, scripts or the plugin monorepo imports
config_helper, display_helper, game_helper, utils or error_handler; only
their own tests did. The error_handler re-exports leave src.common's
__all__; APIHelper, TextHelper, ScrollHelper, LogoHelper and the adaptive
layout exports are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* refactor(config): drop ConfigService's unused versioning and save API

ConfigVersion, get_version/get_version_history/get_version_config,
rollback, save_config, reload, get_plugin_config and the backward-compat
load_config/get_config_path/get_secrets_path had no callers. The display
controller only uses get_config, subscribe, unsubscribe and shutdown,
plus the file watcher. Change detection now compares against the
current checksum instead of the last history entry.

The subscriber tests asserted `callback.called or True`; they now
reload the way the watcher does and assert the notification.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* refactor(plugins): drop unread plugin state history and callbacks

plugin_state.PluginStateManager kept a bounded per-plugin transition
history that only get_state_history (tests only) read; get_state_info
reports a separate lifetime count, which stays. set_error_info and
record_display had no callers, and set_state_with_error's `error`
argument only fed the history.

The web-side state_manager.PluginStateManager loses
subscribe_to_state_changes, _notify_callbacks, set_plugin_error and
get_state_version, none of which had callers; with no subscribers the
old-state copy in update_plugin_state went with them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* refactor(plugins): remove unused PluginManager methods and attribute guards

update_all_plugins was only called by a test (the display loop uses
run_scheduled_updates); get_plugin_health_metrics,
get_plugin_resource_metrics and get_plugin_state had no callers; and
plugin_modules was written but never read. plugin_directories is now
initialised in __init__, so the hasattr() guards around it go.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* refactor(plugins): remove unused executor, loader, store and package helpers

- PluginExecutor.execute_safe: no callers.
- PluginLoader._parse_semver: only its own tests; compatibility.parse_semver
  is the live copy and test_compatibility.py already covers it.
- PluginStoreManager.get_installed_plugin_info: no callers.
- PluginResourceMonitor._local: never read.
- src.plugin_system.get_store_manager and __api_version__: no importers in
  core, scripts or the plugin monorepo.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(wifi): stop storing Wi-Fi passwords in wifi_config.json

WiFiManager appended every joined network's SSID and password, in
plaintext, to saved_networks in config/wifi_config.json, and nothing
(web UI, backup restore, scripts) ever read them back: NetworkManager
keeps its own credentials. The writes are gone, and loading the config
now drops any saved_networks key and rewrites the file, so passwords
already on disk are scrubbed.

Also removes _check_dnsmasq_conflict (never called) and _detect_trixie,
whose result only reached one log line, along with the
NM_CONNECTIONS_PATHS constant only it used.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* refactor(display): remove unreachable and unused DisplayController code

- _follower_rebuild_scroll_image: never called.
- mode_duration (never read) and last_mode_change (write-only).
- The `chosen_cap <= 0` branch: chosen_cap is either the minimum of
  caps already filtered to > 0 or DEFAULT_DYNAMIC_DURATION_CAP (180).
- The `max_duration < min_duration` branch directly after
  `max_duration = max(min_duration, max_duration)`.
- The circuit-breaker branch's `display_result = False` and
  `manager_to_display = None`: the first is overwritten a few lines
  later, the second is already None there.
- The bool-to-bool conversion of execute_display's result, which is
  always a bool.
- The `loaded_plugins` lookup in _update_modules: PluginManager has no
  such attribute, so it always fell through to `plugins`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* refactor(vegas): remove unused config update, boundary finder and refresh

VegasModeConfig.update had no callers outside its own tests (the
coordinator rebuilds the config with from_config on a change);
geometry.find_item_boundary and StreamManager._refresh_plugin_content
had no callers at all.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* refactor(run): drop the debug block that pretended to import the plugin system

In debug mode run.py put src/plugin_system itself on sys.path and printed
"Plugin system import successful" without importing anything. Nothing
imports plugin_system modules by bare name, so the path entry did
nothing either.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test: delete tests that test nothing

- test/plugins/test_{basketball_scoreboard,calendar,clock_simple,
  odds_ticker,soccer_scoreboard,text_display}.py skip everywhere the named
  plugins are not installed, including CI (LEDMATRIX_PLUGINS_DIR holds only
  the fixture plugin); test_plugin_matrix.py already covers every
  discovered plugin. Their PluginTestBase and the fixtures only it used
  (plugins_dir, mock_display_manager, mock_cache_manager,
  mock_plugin_manager, base_plugin_config in test/plugins/conftest.py) go
  with them.
- test_plugin_system.py: test_discover_plugins (body was `pass`) and
  test_dependency_check (a comment), plus the test_plugin_manager fixture
  only the former requested.
- test_display_manager.py: test_draw_image asserted that an image it had
  just assigned was not None.
- test_display_controller.py: the rotation and schedule-override tests
  re-implemented the run-loop arithmetic inline and asserted on their own
  result without calling the controller.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test: expect one plugin_last_update success stamp after update_all_plugins

EveryStampRecordsACompletion required at least two success-path stamps;
the second was update_all_plugins, removed as test-only. The worker and
synchronous paths share the remaining stamp in _execute_update_now, and
the check that every stamp calls _note_update_completed is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-09-23 12:36:26 -04:00

392 lines
16 KiB
Python

"""
Unit tests for WiFi AP mode — src.wifi_manager.
Each test exercises logic that can be verified through the subprocess calls the
manager emits, without requiring root access, hardware, or a running Pi.
Scenarios covered:
1. nmcli AP profile is created with no security parameters (open/passwordless).
2. iptables PREROUTING and INPUT rules are added when the nmcli AP starts.
3. iptables rules and ip_forward are reverted when the AP is torn down.
4. LED matrix message includes the SSID, 'No password', and the setup URL.
5. Known AP profile names are deleted before the new profile is created.
6. Wi-Fi passwords are not written to wifi_config.json, and old ones are scrubbed.
"""
from __future__ import annotations
import json
from pathlib import Path
from unittest.mock import MagicMock, patch
import pytest
from src.wifi_manager import WiFiManager
# ---------------------------------------------------------------------------
# Helpers
# ---------------------------------------------------------------------------
def _ok(stdout: str = "", stderr: str = "") -> MagicMock:
r = MagicMock()
r.returncode = 0
r.stdout = stdout
r.stderr = stderr
return r
def _fail(stdout: str = "", stderr: str = "error") -> MagicMock:
r = MagicMock()
r.returncode = 1
r.stdout = stdout
r.stderr = stderr
return r
def _find_path_side_effect(name: str) -> str:
"""Deterministic fake for _find_command_path."""
return {"iptables": "/usr/sbin/iptables", "sysctl": "/usr/sbin/sysctl"}.get(
name, f"/usr/bin/{name}"
)
# ---------------------------------------------------------------------------
# Fixtures
# ---------------------------------------------------------------------------
@pytest.fixture()
def wifi_config(tmp_path: Path) -> Path:
"""Minimal wifi_config.json in a temporary directory."""
cfg_dir = tmp_path / "config"
cfg_dir.mkdir()
cfg = {
"ap_ssid": "LEDMatrix-Setup",
"ap_channel": 7,
"auto_enable_ap_mode": True,
}
p = cfg_dir / "wifi_config.json"
p.write_text(json.dumps(cfg))
return p
@pytest.fixture()
def manager(wifi_config: Path, tmp_path: Path) -> WiFiManager:
"""
WiFiManager with all system calls stubbed out during construction and the
ip_forward save file redirected to a per-test temporary path.
"""
with patch("src.wifi_manager.subprocess.run", return_value=_ok(stdout="wlan0\n")):
mgr = WiFiManager(config_path=wifi_config)
# Force clean, deterministic state regardless of what __init__ inferred
mgr._wifi_interface = "wlan0"
mgr.has_nmcli = True
mgr.has_hostapd = False
mgr.has_dnsmasq = False
mgr.has_iwlist = False
# Redirect the ip_forward save file to tmp so tests never share state
mgr._IP_FORWARD_SAVE_PATH = tmp_path / "ip_fwd_saved"
return mgr
# ---------------------------------------------------------------------------
# 1. AP profile is open (no password)
# ---------------------------------------------------------------------------
@pytest.mark.unit
def test_nmcli_ap_profile_has_no_security_params(manager: WiFiManager) -> None:
"""
The 'nmcli connection add' command must not include key-mgmt, psk, or any
WPA-related parameter. On Bookworm/Trixie, NM creates a WPA2-protected
hotspot even when those values are set to 'none'/empty via a later
'connection modify', so the profile must be created without a security
section from the start.
"""
captured: list[list[str]] = []
def _run(cmd, **kw):
captured.append(list(cmd))
return _ok()
with patch("src.wifi_manager.subprocess.run", side_effect=_run), \
patch.object(manager, "disconnect_from_network", return_value=(True, "ok")), \
patch.object(manager, "_setup_iptables_redirect", return_value=True), \
patch.object(manager, "_get_ap_status_nmcli",
return_value={"active": True, "ip": "192.168.4.1"}), \
patch.object(manager, "_show_led_message"):
success, _ = manager._enable_ap_mode_nmcli_hotspot()
assert success, "AP enable should report success"
add_calls = [c for c in captured if "nmcli" in c and "connection" in c and "add" in c]
assert add_calls, "Expected at least one 'nmcli connection add' invocation"
add_str = " ".join(add_calls[0])
assert "key-mgmt" not in add_str, "AP profile must not set key-mgmt"
assert "psk" not in add_str, "AP profile must not include a PSK/password"
assert "wpa" not in add_str.lower(), "AP profile must not reference WPA"
assert "802-11-wireless.mode" in add_str, "AP profile must declare wireless mode"
# Verify the value for 802-11-wireless.mode is exactly "ap" — check the element
# that immediately follows the key in the command list, not a loose substring match.
cmd = add_calls[0]
try:
mode_idx = cmd.index("802-11-wireless.mode")
assert cmd[mode_idx + 1] == "ap", \
f"802-11-wireless.mode value must be exactly 'ap', got {cmd[mode_idx + 1]!r}"
except ValueError:
pytest.fail("802-11-wireless.mode not found as a list element in nmcli command")
# ---------------------------------------------------------------------------
# 2. iptables NAT rules are added when the AP starts
# ---------------------------------------------------------------------------
@pytest.mark.unit
def test_iptables_nat_rules_added_on_ap_start(manager: WiFiManager) -> None:
"""
_setup_iptables_redirect must add:
- a PREROUTING REDIRECT rule that maps incoming TCP port 80 to port 5000, and
- an INPUT ACCEPT rule for port 5000 (the post-redirect destination port,
NOT port 80 which never hits the INPUT chain after PREROUTING rewrites it).
"""
captured: list[list[str]] = []
def _run(cmd, **kw):
captured.append(list(cmd))
# iptables -C (check) → rc=1 so the -A (add) branch executes
if "iptables" in " ".join(str(x) for x in cmd) and "-C" in cmd:
return _fail()
return _ok()
# Patch Path.read_text so /proc/sys/net/ipv4/ip_forward is readable on any OS
with patch("src.wifi_manager.subprocess.run", side_effect=_run), \
patch.object(manager, "_find_command_path", side_effect=_find_path_side_effect), \
patch("pathlib.Path.read_text", return_value="0\n"):
result = manager._setup_iptables_redirect()
assert result, "_setup_iptables_redirect must return True on success"
prerouting_adds = [c for c in captured if "iptables" in " ".join(c) and "-A" in c and "PREROUTING" in c]
assert prerouting_adds, "Expected 'iptables -A PREROUTING' invocation"
pr_str = " ".join(prerouting_adds[0])
assert "--dport" in pr_str and "80" in pr_str, "PREROUTING rule must match dport 80"
assert "5000" in pr_str, "PREROUTING rule must redirect to port 5000"
assert "REDIRECT" in pr_str, "PREROUTING rule must use REDIRECT target"
input_adds = [c for c in captured if "iptables" in " ".join(c) and "-A" in c and "INPUT" in c]
assert input_adds, "Expected 'iptables -A INPUT' invocation"
in_str = " ".join(input_adds[0])
assert "5000" in in_str, "INPUT rule must accept port 5000 (post-PREROUTING destination)"
assert "ACCEPT" in in_str, "INPUT rule must use ACCEPT target"
# Port 80 must NOT be used in the INPUT rule (it is already redirected by PREROUTING)
input_80 = [c for c in captured if "iptables" in " ".join(c) and "INPUT" in c and "--dport" in c and "80" in c]
assert not input_80, "INPUT rule must target port 5000, not port 80"
# ---------------------------------------------------------------------------
# 3a. iptables rules and ip_forward reverted on teardown
# ---------------------------------------------------------------------------
@pytest.mark.unit
def test_iptables_rules_and_ip_forward_reverted_on_teardown(manager: WiFiManager) -> None:
"""
_teardown_iptables_redirect must:
- remove the PREROUTING and INPUT iptables rules, and
- restore ip_forward to the exact value recorded in the save file.
"""
original_fwd = "0"
manager._IP_FORWARD_SAVE_PATH.write_text(original_fwd)
# Teardown dispatches on the backend recorded during setup
manager._redirect_backend = "iptables"
captured: list[list[str]] = []
with patch("src.wifi_manager.subprocess.run",
side_effect=lambda cmd, **kw: (captured.append(list(cmd)) or _ok())), \
patch.object(manager, "_find_command_path", side_effect=_find_path_side_effect):
manager._teardown_iptables_redirect()
assert [c for c in captured if "iptables" in " ".join(c) and "-D" in c and "PREROUTING" in c], \
"Expected 'iptables -D PREROUTING' invocation"
assert [c for c in captured if "iptables" in " ".join(c) and "-D" in c and "INPUT" in c], \
"Expected 'iptables -D INPUT' invocation"
restore_calls = [
c for c in captured
if "sysctl" in " ".join(c) and f"ip_forward={original_fwd}" in " ".join(c)
]
assert restore_calls, f"Expected sysctl to restore ip_forward to {original_fwd!r}"
assert not manager._IP_FORWARD_SAVE_PATH.exists(), \
"Save file must be removed after successful teardown"
# ---------------------------------------------------------------------------
# 3b. ip_forward untouched when no save file exists
# ---------------------------------------------------------------------------
@pytest.mark.unit
def test_ip_forward_not_restored_when_save_file_absent(manager: WiFiManager) -> None:
"""
When the save file is missing (setup never wrote it, e.g. because /proc was
unreadable or the write failed), teardown must NOT call sysctl so it does not
accidentally clobber ip_forward state owned by a VPN or NetworkManager.
"""
assert not manager._IP_FORWARD_SAVE_PATH.exists()
captured: list[list[str]] = []
with patch("src.wifi_manager.subprocess.run",
side_effect=lambda cmd, **kw: (captured.append(list(cmd)) or _ok())), \
patch.object(manager, "_find_command_path", side_effect=_find_path_side_effect):
manager._teardown_iptables_redirect()
sysctl_calls = [
c for c in captured
if "sysctl" in " ".join(c) and "ip_forward" in " ".join(c)
]
assert not sysctl_calls, \
"sysctl must not be called when no ip_forward save file exists"
# ---------------------------------------------------------------------------
# 4. LED message content
# ---------------------------------------------------------------------------
@pytest.mark.unit
def test_led_message_shows_ssid_no_password_and_url(manager: WiFiManager) -> None:
"""
When the nmcli AP activates, the LED message must include:
- the AP SSID ('LEDMatrix-Setup')
- the string 'No password'
- the AP IP address (192.168.4.1) and Flask port (5000)
"""
led_messages: list[str] = []
with patch("src.wifi_manager.subprocess.run", return_value=_ok()), \
patch.object(manager, "disconnect_from_network", return_value=(True, "ok")), \
patch.object(manager, "_setup_iptables_redirect", return_value=True), \
patch.object(manager, "_get_ap_status_nmcli",
return_value={"active": True, "ip": "192.168.4.1"}), \
patch.object(manager, "_show_led_message",
side_effect=lambda msg, **kw: led_messages.append(msg)):
success, _ = manager._enable_ap_mode_nmcli_hotspot()
assert success, "AP enable should report success"
assert led_messages, "Expected at least one _show_led_message call"
combined = "\n".join(led_messages)
assert "No password" in combined, "LED message must say 'No password'"
assert "LEDMatrix-Setup" in combined, "LED message must include the AP SSID"
assert "192.168.4.1" in combined, "LED message must include the AP IP address"
assert "5000" in combined, "LED message must include the Flask port"
# ---------------------------------------------------------------------------
# 5. Stale AP profiles deleted before the new one is created
# ---------------------------------------------------------------------------
@pytest.mark.unit
def test_existing_ap_profiles_deleted_before_new_profile_created(manager: WiFiManager) -> None:
"""
Before 'nmcli connection add', the manager must issue
'nmcli connection down/delete' for every known AP profile name so stale
profiles (from a previous crash or partial setup) cannot block the new one.
"""
captured: list[list[str]] = []
def _run(cmd, **kw):
captured.append(list(cmd))
return _ok()
with patch("src.wifi_manager.subprocess.run", side_effect=_run), \
patch.object(manager, "disconnect_from_network", return_value=(True, "ok")), \
patch.object(manager, "_setup_iptables_redirect", return_value=True), \
patch.object(manager, "_get_ap_status_nmcli",
return_value={"active": True, "ip": "192.168.4.1"}), \
patch.object(manager, "_show_led_message"):
success, _ = manager._enable_ap_mode_nmcli_hotspot()
assert success
cmd_strs = [" ".join(c) for c in captured]
for profile in ("LEDMatrix-Setup-AP", "Hotspot", "TickerSetup-AP"):
assert any("connection delete" in s and profile in s for s in cmd_strs), \
f"Expected 'nmcli connection delete {profile}' before creating the new profile"
add_indices = [i for i, s in enumerate(cmd_strs) if "connection add" in s]
del_indices = [i for i, s in enumerate(cmd_strs) if "connection delete" in s]
assert add_indices, "Expected 'nmcli connection add' call"
assert del_indices, "Expected 'nmcli connection delete' calls"
assert max(del_indices) < min(add_indices), \
"All connection deletions must complete before the new profile is created"
# ---------------------------------------------------------------------------
# 6. Wi-Fi passwords are not kept in wifi_config.json
# ---------------------------------------------------------------------------
@pytest.mark.unit
def test_loading_scrubs_plaintext_saved_networks(wifi_config: Path) -> None:
"""Older versions wrote every joined network's password to the config in
plaintext and never read it back. Loading must remove it from disk."""
cfg = json.loads(wifi_config.read_text())
cfg["saved_networks"] = [
{"ssid": "HomeNet", "password": "hunter22", "saved_at": 0},
]
wifi_config.write_text(json.dumps(cfg))
with patch("src.wifi_manager.subprocess.run", return_value=_ok(stdout="wlan0\n")):
mgr = WiFiManager(config_path=wifi_config)
assert "saved_networks" not in mgr.config
assert "hunter22" not in wifi_config.read_text()
on_disk = json.loads(wifi_config.read_text())
assert "saved_networks" not in on_disk
# Everything else survives the scrub.
assert on_disk["ap_ssid"] == "LEDMatrix-Setup"
assert on_disk["auto_enable_ap_mode"] is True
@pytest.mark.unit
def test_default_config_has_no_saved_networks(tmp_path: Path) -> None:
config_path = tmp_path / "config" / "wifi_config.json"
config_path.parent.mkdir()
with patch("src.wifi_manager.subprocess.run", return_value=_ok(stdout="wlan0\n")):
WiFiManager(config_path=config_path)
assert "saved_networks" not in json.loads(config_path.read_text())
@pytest.mark.unit
def test_connecting_does_not_store_the_password(manager: WiFiManager) -> None:
commands = []
def fake_run(cmd, *args, **kwargs):
commands.append(cmd)
# No existing profile for the SSID, so a new connection is created.
if cmd[:3] == ["nmcli", "connection", "show"] and "HomeNet" in cmd:
return _fail()
return _ok(stdout="")
with patch("src.wifi_manager.subprocess.run", side_effect=fake_run), \
patch("src.wifi_manager.time.sleep"), \
patch.object(manager, "_show_led_message"):
manager._connect_nmcli("HomeNet", "hunter22")
assert ["nmcli", "device", "wifi", "connect", "HomeNet", "password", "hunter22"] in commands, \
"the new-connection path was not reached"
assert "hunter22" not in json.dumps(manager.config)
assert "hunter22" not in manager.config_path.read_text()