ed
bcdc26d0bd
PR1 follow-up (the actual IM_ASSERT root cause fix).
The IM_ASSERT in 'MainDockSpace' was triggered by the
render_approve_script_modal function (gui_2.py:4895) calling
imgui.checkbox with a None value for app.ui_approve_modal_preview.
The chain of bugs:
1. AppController.__getattr__ returned None for ANY ui_ attribute
(line 1237-1238). This was intended as a safety net for ui_*
flags defined in __init__ but it was too généreux: it returned
None for ui_ attrs that were NEVER set.
2. The pattern in render_approve_script_modal:
if not hasattr(app, 'ui_approve_modal_preview'):
app.ui_approve_modal_preview = False
_, app.ui_approve_modal_preview = imgui.checkbox(..., app.ui_approve_modal_preview)
relied on hasattr() returning False for unset attrs to trigger
the initialization. But the App.__setattr__ checks
hasattr(self.controller, name) to decide where to route
assignments. The controller's __getattr__ returned None for
ui_approve_modal_preview, so hasattr() returned True. The
App.__setattr__ routed the assignment to the controller.
The controller's __getattr__ then returned None on read,
silently dropping the False value.
3. The next line called imgui.checkbox with None, which raised
a TypeError. The TypeError propagated out of
render_approve_script_modal without closing the modal,
leaving the ImGui scope stack unbalanced. The unbalanced
scope triggered IM_ASSERT(Missing End()) on the next frame.
Fix: AppController.__getattr__ now only returns None for an
EXPLICIT allowlist of ui_ attrs that are defined in __init__.
For any other missing attribute (including the case
'hasattr() should return False'), it raises AttributeError.
The App.__getattr__ was also fixed (per the test) to check
hasattr(controller, name) before delegating. This is defense in
depth in case other __getattr__ patterns are added.
Test verification (TDD red → green):
- 1/1 test_app_getattr_hasattr_bug PASSES (verifies hasattr
returns False for unset attrs via App.__getattr__)
- 1/1 test_app_controller_getattr_ui_bug PASSES (verifies hasattr
returns False for unset ui_ attrs on controller)
Live verification:
- 4 sims + test_live_workflow + 2 markdown tests: 7/7 PASS in 83.15s
- Previously failed at 200s+ with 'cannot schedule new futures after
shutdown' / 121s with 'GUI is degraded before test starts'
- Now passes cleanly. The IM_ASSERT no longer fires.
13/13 related unit tests pass (app_controller_* + app_run_* +
app_getattr_*). No regressions in 51/51 io_pool/warmup/sigint/etc.
unit tests.
47 lines
1.9 KiB
Python
47 lines
1.9 KiB
Python
"""Regression tests for AppController.__getattr__ ui_* default bug.
|
|
|
|
The bug: `AppController.__getattr__` returns `None` for any attribute
|
|
starting with `ui_` (e.g. `ui_separate_task_dag`, `ui_approve_modal_preview`).
|
|
This makes `hasattr(ctrl, 'ui_X')` return True for ANY ui_X attribute,
|
|
even if the attribute was never set on the instance.
|
|
|
|
Consequence: `App.__setattr__` checks `hasattr(self.controller, name)`
|
|
to decide where to route assignments. With the controller's broken
|
|
__getattr__, all `ui_*` assignments get routed to the controller.
|
|
But the controller's `__getattr__` then returns None on read,
|
|
silently dropping the assigned value.
|
|
|
|
This breaks the `if not hasattr(app, 'foo'): app.foo = False`
|
|
pattern in render_approve_script_modal (gui_2.py:4894): the hasattr
|
|
returns True (via the controller), so the assignment never runs,
|
|
and the next line tries to call checkbox with None.
|
|
|
|
The fix: AppController.__getattr__ should only return None for
|
|
attributes that have been set (in self.__dict__). For unset
|
|
attributes, raise AttributeError so hasattr() returns False.
|
|
"""
|
|
import pytest
|
|
import sys
|
|
import os
|
|
from typing import Any
|
|
|
|
ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))
|
|
sys.path.insert(0, ROOT)
|
|
|
|
|
|
def test_hasattr_returns_false_for_unset_ui_attribute(mock_app: Any) -> None:
|
|
"""hasattr(ctrl, 'ui_test_attr_xyz') must return False when the
|
|
attribute is not set on the instance. Otherwise the App.__setattr__
|
|
routing logic misbehaves.
|
|
"""
|
|
app = mock_app
|
|
ctrl = app.controller
|
|
# Use a unique attribute name guaranteed not to be in init_state()
|
|
attr_name = "ui_test_attr_xyz_unique_12345"
|
|
# Force the attribute lookup
|
|
assert hasattr(ctrl, attr_name) is False, (
|
|
f"hasattr(ctrl, {attr_name!r}) returned True for an unset attribute. "
|
|
"AppController.__getattr__ is returning None for any ui_* attribute, "
|
|
"which breaks hasattr() and the App.__setattr__ routing logic."
|
|
)
|