Skip to content

chore: strip defensive getattrs from production, seed bypassed-init tests #81

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AreteDriver merged 2 commits into from
May 4, 2026
Merged

chore: strip defensive getattrs from production, seed bypassed-init tests #81

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2f0fa47
chore: strip defensive getattrs from production, seed bypassed-init t…
AreteDriver Apr 26, 2026
a6ec875
Merge branch 'main' into chore/strip-defensive-getattrs
AreteDriver May 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 13 additions & 26 deletions src/argus_overview/ui/main_tab.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -833,7 +833,7 @@ def update_frame(self, image: Image.Image):

# If the user is currently scrubbing through the strip, hold
# the buffered view — don't overwrite it with the live frame.
if getattr(self, "_replay_view_index", None) is not None:
if self._replay_view_index is not None:
return

# Scale to fit widget while maintaining aspect ratio
Expand Down Expand Up @@ -1063,11 +1063,8 @@ def _sample_replay_buffer(self, pixmap: QPixmap) -> None:
"""Throttle-sample a captured pixmap into the ring buffer."""
if pixmap is None or pixmap.isNull():
return
# Defensive: bypassed-init test helpers may not set these.
if not hasattr(self, "_replay_buffer"):
return
now_ms = int(time.monotonic() * 1000)
if now_ms - getattr(self, "_replay_last_sample_ms", 0) < REPLAY_THROTTLE_MS:
if now_ms - self._replay_last_sample_ms < REPLAY_THROTTLE_MS:
return
# Store an immutable copy at modest size; full-res pixmaps would
# blow up memory across many widgets. 240×180 keeps it ~170KB max.
Expand All @@ -1086,7 +1083,7 @@ def _sample_replay_buffer(self, pixmap: QPixmap) -> None:
self._replay_strip = None

def is_replay_strip_enabled(self) -> bool:
return getattr(self, "_replay_strip", None) is not None
return self._replay_strip is not None

def enable_replay_strip(self, enabled: bool) -> None:
"""Show or hide the replay strip below the main image."""
Expand Down Expand Up @@ -1143,11 +1140,7 @@ def paintEvent(self, event):
# 0. Per-character accent border (PR8) — only visible when no
# threat or legacy-flash overlay is active. Gives instant visual
# identity at small grid sizes and matches the chip avatar color.
if (
self._threat_level is None
and self._flash_color is None
and getattr(self, "_accent_color", None) is not None
):
if self._threat_level is None and self._flash_color is None:
pen = QPen(self._accent_color)
pen.setWidth(2)
painter.setPen(pen)
Expand Down Expand Up @@ -1527,19 +1520,16 @@ def apply_threat_state(self, level: ThreatLevel | None, system: str | None = Non
if level is None or level == ThreatLevel.CLEAR or not system:
return self._fan_to_all(level, system)

char_systems = getattr(self, "_character_systems", {}) or {}
calculator = getattr(self, "_jump_calculator", None)
max_jumps = getattr(self, "_jump_max", 0)
count = 0
for frame in list(self.preview_frames.values()):
try:
char_name = getattr(frame, "character_name", None)
known = char_systems.get(char_name) if char_name else None
known = self._character_systems.get(char_name) if char_name else None
should_apply, alpha = resolve_tint(
known_system=known,
alert_system=system,
jump_calculator=calculator,
max_jumps=max_jumps,
jump_calculator=self._jump_calculator,
max_jumps=self._jump_max,
)
if not should_apply:
continue
Expand All @@ -1549,11 +1539,11 @@ def apply_threat_state(self, level: ThreatLevel | None, system: str | None = Non
if (
alpha < 1.0
and known
and calculator is not None
and self._jump_calculator is not None
and known.lower() != system.lower()
):
try:
distance = calculator.distance(known, system)
distance = self._jump_calculator.distance(known, system)
except (AttributeError, TypeError, ValueError):
distance = None
frame.set_threat_state(level, system, initial_alpha=alpha, distance=distance)
Expand Down Expand Up @@ -2414,16 +2404,13 @@ def _on_window_removed(self, window_id: str):
# Stop per-frame timers
frame.session_timer.stop()
# If we just removed the spotlight target, drop focus state.
# getattr keeps this safe when called via test helpers that bypass __init__.
if getattr(self, "_focus_window_id", None) == window_id:
if self._focus_window_id == window_id:
self._focus_window_id = None
self.window_manager.remove_window(window_id)
self._update_status()
if hasattr(self, "status_dock"):
self._sync_status_dock()
if hasattr(self, "_focus_window_id"):
# Re-apply (covers both: focus cleared, or other tile removed mid-focus)
self._apply_focus_state()
self._sync_status_dock()
# Re-apply (covers both: focus cleared, or other tile removed mid-focus)
self._apply_focus_state()

def _remove_all_windows(self):
"""Remove all windows from preview"""
Expand Down
42 changes: 42 additions & 0 deletions tests/_test_helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
"""
Shared test helpers for bypassed-`__init__` widgets / managers.

Many tests use `Widget.__new__(Widget)` and `patch.object(Widget, "__init__")`
to test individual methods without spinning up the full Qt object graph.
That path skips attribute initialization, so any production code that
references `self._x` on a bypassed instance raises `AttributeError`.

Rather than litter production code with `getattr(self, "_x", default)`
to paper over the test pattern, these helpers seed the minimum state
each method under test needs. Production code stays clean; tests own
their own setup.
"""

from __future__ import annotations

from collections import deque
from unittest.mock import MagicMock


def seed_preview_widget(widget) -> None:
"""Initialize the v3.2.0 attributes a bypassed-init WindowPreviewWidget
needs to survive update_frame, paintEvent, and threat-state methods."""
widget._replay_buffer = deque(maxlen=6)
widget._replay_last_sample_ms = 0
widget._replay_strip = None
widget._replay_view_index = None
Comment on lines +21 to +27
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The seed_preview_widget helper is missing several attributes required by paintEvent and set_threat_state, which the docstring explicitly claims it supports. Since the production code now uses direct attribute access (removing getattr guards), calling these methods on a seeded widget will raise AttributeError.

Missing attributes include: _accent_color, _threat_level, _flash_color, _threat_alpha, _threat_distance, _pulse_phase, _positions_locked, _show_activity_indicator, and the various QTimer instances (_threat_decay_timer, _pulse_timer, _flash_timer). Also, it is better to use the REPLAY_BUFFER_SIZE constant instead of hardcoding 6.

def seed_preview_widget(widget) -> None:
    """Initialize the v3.2.0 attributes a bypassed-init WindowPreviewWidget
    needs to survive update_frame, paintEvent, and threat-state methods."""
    from PySide6.QtCore import QTimer
    from PySide6.QtGui import QColor
    from argus_overview.ui.main_tab import REPLAY_BUFFER_SIZE
    widget._replay_buffer = deque(maxlen=REPLAY_BUFFER_SIZE)
    widget._replay_last_sample_ms = 0
    widget._replay_strip = None
    widget._replay_view_index = None
    widget._accent_color = QColor(200, 200, 200)
    widget._threat_level = None
    widget._flash_color = None
    widget._threat_alpha = 0.0
    widget._threat_distance = None
    widget._pulse_phase = 0.0
    widget._positions_locked = False
    widget._show_activity_indicator = True
    widget._threat_decay_timer = QTimer()
    widget._pulse_timer = QTimer()
    widget._flash_timer = QTimer()



def seed_main_tab(tab) -> None:
"""Seed bypassed-init MainTab attributes needed by _on_window_removed
and other lifecycle methods."""
tab._focus_window_id = None
tab.status_dock = MagicMock()


def seed_window_manager(manager) -> None:
"""Seed bypassed-init WindowManager attributes needed by
apply_threat_state and the smart fan-out / jumps-from filter."""
manager._character_systems = {}
manager._jump_calculator = None
manager._jump_max = 0
Comment on lines +37 to +42
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The seed_window_manager helper should also initialize preview_frames to an empty dictionary. While it is a public attribute, it is initialized in __init__ and accessed directly in apply_threat_state. Bypassing __init__ without seeding it will cause an AttributeError when the loop at line 1524 in main_tab.py attempts to iterate over frames.

Suggested change
def seed_window_manager(manager) -> None:
"""Seed bypassed-init WindowManager attributes needed by
apply_threat_state and the smart fan-out / jumps-from filter."""
manager._character_systems = {}
manager._jump_calculator = None
manager._jump_max = 0
def seed_window_manager(manager) -> None:
"""Seed bypassed-init WindowManager attributes needed by
apply_threat_state and the smart fan-out / jumps-from filter."""
manager.preview_frames = {}
manager._character_systems = {}
manager._jump_calculator = None
manager._jump_max = 0

30 changes: 30 additions & 0 deletions tests/test_main_tab.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4068,9 +4068,13 @@ def test_on_window_activated(self):
def test_on_window_removed(self):
"""Test _on_window_removed removes frame"""
from argus_overview.ui.main_tab import MainTab
from tests._test_helpers import seed_main_tab

with patch.object(MainTab, "__init__", return_value=None):
tab = MainTab.__new__(MainTab)
seed_main_tab(tab)
tab._apply_focus_state = MagicMock()
tab._sync_status_dock = MagicMock()
tab.logger = MagicMock()
tab.window_manager = MagicMock()
tab._update_status = MagicMock()
Expand Down Expand Up @@ -5710,9 +5714,11 @@ def test_update_frame_with_image(self):
from datetime import datetime

from argus_overview.ui.main_tab import WindowPreviewWidget
from tests._test_helpers import seed_preview_widget

with patch.object(WindowPreviewWidget, "__init__", return_value=None):
widget = WindowPreviewWidget.__new__(WindowPreviewWidget)
seed_preview_widget(widget)
widget.image_label = MagicMock()
widget.current_pixmap = None
widget.zoom_factor = 0.3
Expand Down Expand Up @@ -5785,9 +5791,11 @@ class TestFrameFingerprintCache:
def test_identical_frame_skipped(self):
"""Test update_frame skips rendering when frame bytes are identical."""
from argus_overview.ui.main_tab import WindowPreviewWidget
from tests._test_helpers import seed_preview_widget

with patch.object(WindowPreviewWidget, "__init__", return_value=None):
widget = WindowPreviewWidget.__new__(WindowPreviewWidget)
seed_preview_widget(widget)
widget.image_label = MagicMock()
widget.current_pixmap = None
widget._last_frame_hash = None
Expand Down Expand Up @@ -5829,9 +5837,11 @@ def test_identical_frame_skipped(self):
def test_different_frame_rendered(self):
"""Test update_frame renders when frame bytes differ."""
from argus_overview.ui.main_tab import WindowPreviewWidget
from tests._test_helpers import seed_preview_widget

with patch.object(WindowPreviewWidget, "__init__", return_value=None):
widget = WindowPreviewWidget.__new__(WindowPreviewWidget)
seed_preview_widget(widget)
widget.image_label = MagicMock()
widget.current_pixmap = None
widget._last_frame_hash = None
Expand Down Expand Up @@ -5987,9 +5997,11 @@ class TestMemoryCleanup:
def test_image_closed_on_success(self):
"""Test PIL image is closed after conversion."""
from argus_overview.ui.main_tab import WindowPreviewWidget
from tests._test_helpers import seed_preview_widget

with patch.object(WindowPreviewWidget, "__init__", return_value=None):
widget = WindowPreviewWidget.__new__(WindowPreviewWidget)
seed_preview_widget(widget)
widget.image_label = MagicMock()
widget.current_pixmap = None
widget._last_frame_hash = None
Expand Down Expand Up @@ -8611,9 +8623,13 @@ class TestOnWindowRemovedExceptionPaths:
def test_on_window_removed_disconnect_runtime_error(self):
"""Test RuntimeError on signal disconnect is caught silently"""
from argus_overview.ui.main_tab import MainTab
from tests._test_helpers import seed_main_tab

with patch.object(MainTab, "__init__", return_value=None):
tab = MainTab.__new__(MainTab)
seed_main_tab(tab)
tab._apply_focus_state = MagicMock()
tab._sync_status_dock = MagicMock()
tab.logger = MagicMock()
tab.window_manager = MagicMock()
tab._update_status = MagicMock()
Expand All @@ -8637,9 +8653,13 @@ def test_on_window_removed_disconnect_runtime_error(self):
def test_on_window_removed_disconnect_type_error(self):
"""Test TypeError on signal disconnect is caught silently"""
from argus_overview.ui.main_tab import MainTab
from tests._test_helpers import seed_main_tab

with patch.object(MainTab, "__init__", return_value=None):
tab = MainTab.__new__(MainTab)
seed_main_tab(tab)
tab._apply_focus_state = MagicMock()
tab._sync_status_dock = MagicMock()
tab.logger = MagicMock()
tab.window_manager = MagicMock()
tab._update_status = MagicMock()
Expand All @@ -8659,9 +8679,13 @@ def test_on_window_removed_disconnect_type_error(self):
def test_on_window_removed_no_frame_found(self):
"""Test _on_window_removed when frame not in preview_frames"""
from argus_overview.ui.main_tab import MainTab
from tests._test_helpers import seed_main_tab

with patch.object(MainTab, "__init__", return_value=None):
tab = MainTab.__new__(MainTab)
seed_main_tab(tab)
tab._apply_focus_state = MagicMock()
tab._sync_status_dock = MagicMock()
tab.logger = MagicMock()
tab.window_manager = MagicMock()
tab.window_manager.preview_frames = {} # Empty — no frame
Expand Down Expand Up @@ -9251,9 +9275,11 @@ class TestWindowManagerApplyThreatState:
def test_apply_threat_state_fans_out_to_all_frames(self):
from argus_overview.intel.parser import ThreatLevel
from argus_overview.ui.main_tab import WindowManager
from tests._test_helpers import seed_window_manager

with patch.object(WindowManager, "__init__", return_value=None):
manager = WindowManager.__new__(WindowManager)
seed_window_manager(manager)
f1, f2, f3 = MagicMock(), MagicMock(), MagicMock()
manager.preview_frames = {"w1": f1, "w2": f2, "w3": f3}

Expand Down Expand Up @@ -9288,9 +9314,11 @@ def test_apply_threat_state_empty_frames(self):
def test_apply_threat_state_skips_deleted_widgets(self):
from argus_overview.intel.parser import ThreatLevel
from argus_overview.ui.main_tab import WindowManager
from tests._test_helpers import seed_window_manager

with patch.object(WindowManager, "__init__", return_value=None):
manager = WindowManager.__new__(WindowManager)
seed_window_manager(manager)
ok = MagicMock()
dead = MagicMock()
dead.set_threat_state.side_effect = RuntimeError("widget deleted")
Expand Down Expand Up @@ -9772,6 +9800,8 @@ def _make_manager(self, char_systems: dict[str, str] | None = None):
manager = WindowManager.__new__(WindowManager)
manager.preview_frames = {}
manager._character_systems = dict(char_systems or {})
manager._jump_calculator = None
manager._jump_max = 0
return manager

def test_clear_flushes_all_regardless_of_system(self):
Expand Down
Loading