chore: strip defensive getattrs from production, seed bypassed-init tests#81
Conversation
…ests Production code accumulated 8 sites of getattr(self, '_x', default) just to keep bypassed-__init__ unit tests passing — patterns like test_helpers that call WindowPreviewWidget.__new__(WindowPreviewWidget) to test methods in isolation. The defensive code in production was the wrong place to handle that gap. This is debt item #2 from the post-v3.2.0 audit. Production changes: - main_tab.py: drop getattr around _replay_view_index, _replay_last_sample_ms, _replay_strip, _accent_color, _character_systems, _jump_calculator, _jump_max, _focus_window_id, status_dock. Production __init__ always sets these; tests are responsible for their own setup. - main_tab.py: also fix a stale 'calculator' local variable reference that the strip exposed (pre-existing latent bug — code path was reachable only when alpha < 1.0 from a no-longer-defined local). Test changes: - New tests/_test_helpers.py exposes seed_preview_widget, seed_main_tab, seed_window_manager that initialize the v3.2.0 attributes a bypassed-init instance needs. - 9 bypassed-init test sites updated to call the relevant seeder. - TestWindowManagerApplyThreatStateSmartFanout._make_manager already seeds char_systems; added _jump_calculator/_jump_max here too. Suite: 2398 passed, 5 skipped, 0 failures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the MainTab UI components to use direct attribute access, removing previous getattr and hasattr defensive guards. To support this change in tests where __init__ is bypassed, a new _test_helpers.py file was added to provide attribute seeding. Review feedback identifies that the seeding helpers are missing several required attributes, such as _accent_color and preview_frames, which will result in AttributeError during test execution.
| 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 |
There was a problem hiding this comment.
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_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 |
There was a problem hiding this comment.
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.
| 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 |
1 participant
Summary
Production code accumulated 8 sites of `getattr(self, '_x', default)` just to keep bypassed-`init` unit tests passing. This is debt item #2 from the post-v3.2.0 audit — the defensive code in production was the wrong place to handle the test-fixture gap.
Production changes
`src/argus_overview/ui/main_tab.py`:
Test infrastructure
New `tests/_test_helpers.py` exposes three seeders:
9 bypassed-init test sites updated to call the relevant seeder.
Test plan
Future-proofing
Anyone adding new state to `WindowPreviewWidget` / `MainTab` / `WindowManager` should add it to the appropriate seeder, not slap `getattr` on production reads.
🤖 Generated with Claude Code