Apply PR #144 review round 3: lifecycle tests for malformed-option fallback, comment/test hygiene

Add LIFECYCLE tests that wire an entry via make_entry(options=...) and
call async_setup_entry, covering the from_options_resilient fallback
production actually uses (P2): a fully malformed options dict
(horizon_days=True, day_rollover_time='99:99',
morning_summary_time='25:00') succeeds with the day-rollover listener
at the default 00:00 and the backfill over DEFAULT_HORIZON_DAYS; and a
bad sibling (horizon_days=5, morning_summary_time='25:00') keeps
horizon_days == 5. Swapping from_options_resilient for from_options in
setup now fails these tests.

Reword comments/tests describing review history rather than the code:
__init__.py setup comment, test_core_settings P2-3/P2-1/P2-5 markers,
the moved from_options horizon test, and the AST scanner docstrings.

Test hygiene: move freeze_nestquest_clock into tests/conftest.py and
use it from test_core_settings and test_timezones; simplify CORE_PKG to
a plain import plus Path(mod.__file__).parent; move
test_from_options_horizon_validates_and_defaults into test_core_settings.

1575 passed.

custom_components/nestquest/__init__.py

@@ -467,10 +467,9 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
         # horizon materialization and the daily-rollover listener both
         # consume it, so core never reads HA config itself.  A malformed
         # stored option falls back PER FIELD (via from_options_resilient)
-        # so a single bad sibling option cannot silently reset the user's
-        # horizon_days or day_rollover_time — the two fields core consumes
-        # — rather than the all-or-nothing reset a single try/except would
-        # cause (a malformed morning_summary_time used to wipe horizon_days).
+        # so a single bad sibling option cannot reset the user's
+        # horizon_days or day_rollover_time — the two fields core
+        # consumes — to their defaults.
         settings = NestQuestSettings.from_options_resilient(entry.options)
         remove_update_listener = entry.add_update_listener(_async_update_listener)
         remove_time_change_listener = _register_day_rollover_listener(

tests/conftest.py

@@ -22,6 +22,7 @@ pytest -q``); it is never meant to run against a real HA install.
 from __future__ import annotations
 
 import asyncio
+import datetime
 import importlib.util
 import inspect
 import sys
@@ -916,4 +917,40 @@ def make_config_flow_entry():
     def _factory(domain: str, entry_id: str = "existing"):
         return SimpleNamespace(domain=domain, entry_id=entry_id)
 
-    return _factory
+    return _factory
+
+
+def freeze_nestquest_clock(
+    monkeypatch, instant: datetime.datetime
+) -> tuple[datetime.datetime, list]:
+    """Freeze the integration's ``datetime`` clock to a fixed aware instant.
+
+    Only ``custom_components.nestquest``'s ``datetime`` binding is patched,
+    so the generation path's ``datetime.datetime.now`` reads this instant
+    (localized to whatever time zone it asks for) while every other module
+    and the test harness keep the real clock.  Returns ``(frozen, requested)``
+    where ``frozen`` is the instant normalized to UTC and ``requested`` is
+    the list of ``tzinfo`` objects the code under test passed to ``now`` —
+    so a caller can prove the generation path actually requested the
+    configured HA time zone rather than silently reading UTC.
+    """
+    import custom_components.nestquest as nestquest
+
+    frozen = instant.astimezone(datetime.timezone.utc)
+    requested: list = []
+
+    class _FrozenDatetime(datetime.datetime):
+        @classmethod
+        def now(cls, tz=None):
+            requested.append(tz)
+            if tz is None:
+                return frozen.replace(tzinfo=None)
+            return frozen.astimezone(tz)
+
+    fake_module = SimpleNamespace(
+        datetime=_FrozenDatetime,
+        timedelta=datetime.timedelta,
+        date=datetime.date,
+    )
+    monkeypatch.setattr(nestquest, "datetime", fake_module)
+    return frozen, requested

tests/test_core_settings.py

@@ -24,11 +24,13 @@ import ast
 import datetime
 from dataclasses import FrozenInstanceError
 from pathlib import Path
-from types import SimpleNamespace
 from typing import Any
 
 import pytest
 
+import custom_components.nestquest.core as _core_mod
+from conftest import freeze_nestquest_clock
+
 from custom_components.nestquest.const import (
     CONF_AFTERNOON_REMINDER_ENABLED,
     CONF_AFTERNOON_REMINDER_TIME,
@@ -139,6 +141,29 @@ def test_from_options_horizon_absent_falls_back_to_default() -> None:
     )
 
 
+def test_from_options_horizon_validates_and_defaults() -> None:
+    """from_options returns the configured horizon, defaulting when absent/None.
+
+    A valid horizon is read; an absent or None key falls back to the
+    default; and a malformed value RAISES (the integration's
+    ``from_options_resilient`` handles the per-field fall-back at setup,
+    not this strict path).
+    """
+    assert NestQuestSettings.from_options({CONF_HORIZON_DAYS: 5}).horizon_days == 5
+    assert (
+        NestQuestSettings.from_options({}).horizon_days == DEFAULT_HORIZON_DAYS
+    )
+    assert (
+        NestQuestSettings.from_options(
+            {CONF_HORIZON_DAYS: None}
+        ).horizon_days
+        == DEFAULT_HORIZON_DAYS
+    )
+    for bad in (True, "5", 0):
+        with pytest.raises(ValueError):
+            NestQuestSettings.from_options({CONF_HORIZON_DAYS: bad})
+
+
 @pytest.mark.parametrize(
     "value",
     ["25:00", "00:60", "24:00", "9:5", "abc", "07:00:00", 700, ""],
@@ -212,7 +237,7 @@ def test_from_options_strips_notify_target() -> None:
 
 
 # ---------------------------------------------------------------------------
-# from_options_resilient: per-field fallback (P2-1).
+# from_options_resilient: per-field fallback.
 # ---------------------------------------------------------------------------
 
 
@@ -335,7 +360,7 @@ def test_direct_construction_validates() -> None:
 def test_direct_construction_rejects_none(field: str, kwargs: dict[str, Any]) -> None:
     """__post_init__ is STRICT: None is rejected for every field.
 
-    P2-5: the None->default mapping lives ONLY in from_options (via
+    The None->default mapping lives ONLY in from_options (via
     _option_value).  A hand-built NestQuestSettings(field=None) must raise
     in __post_init__ rather than constructing and later blowing up
     horizon_window (timedelta(days=None) -> TypeError) or the rollover
@@ -419,11 +444,7 @@ def test_horizon_window_non_default_horizon() -> None:
 # AST: no HA config reads under core/.
 # ---------------------------------------------------------------------------
 
-CORE_PKG = Path(
-    __import__(
-        "custom_components.nestquest.core", fromlist=["__file__"]
-    ).__file__
-).parent
+CORE_PKG = Path(_core_mod.__file__).parent
 
 
 def _is_homeassistant(module: str) -> bool:
@@ -446,8 +467,8 @@ _FORBIDDEN_ATTRS = frozenset({"hass", "entry", "config_entry"})
 #: Config-entry attribute reads — the shapes ``hass.config``,
 #: ``config_entry.options``, ``self.entry.options``, ``runtime.options``,
 #: ``entry.data``.  Flagging the attribute ANYWHERE (regardless of root)
-#: is what catches ``runtime.options``, which the old root-only check
-#: missed.  No core module currently reads any of these attributes.
+#: catches ``runtime.options``, whose root name is not a forbidden
+#: identifier.  No core module currently reads any of these attributes.
 _CONFIG_ATTRS = frozenset({"config", "options", "data"})
 
 #: Dynamic config-entry reads via getattr/hasattr with a string argument.
@@ -475,8 +496,8 @@ def _scan_source_for_ha_config(
 ) -> list[str]:
     """Return offender strings for HA-coupling in ``source``.
 
-    Flags (each closes a bypass the old root-only ``hass``/``entry``
-    chain check missed):
+    Flags (each closes a bypass a root-only ``hass``/``entry`` chain
+    check would not catch):
 
     - any ``import homeassistant`` / ``from homeassistant ...``;
     - any ``importlib.import_module('homeassistant...')`` (a string-based
@@ -489,14 +510,14 @@ def _scan_source_for_ha_config(
       bare-Name check misses keyword arguments);
     - any attribute access whose ``attr`` is ``hass``/``entry``/
       ``config_entry`` (catches ``self.hass``, ``self.entry``,
-      ``obj.config_entry`` — root names the old check could not see);
+      ``obj.config_entry`` — root names a root-only check could not see);
     - any attribute access whose ``attr`` is ``config``/``options``/
       ``data`` — these are Home Assistant config-entry attributes with no
       legitimate core use (``.config`` reads ``hass.config``, ``.options``
       reads ``config_entry.options``, ``.data`` reads ``config_entry.data``);
       flagging the attribute ANYWHERE (regardless of root) is what catches
-      ``runtime.options``, which the old root-only check missed.  No core
-      module currently reads any of these attributes;
+      ``runtime.options``, whose root name is not a forbidden identifier.
+      No core module currently reads any of these attributes;
     - any ``getattr``/``hasattr`` call whose string argument is
       ``'config'``/``'options'``/``'data'`` (catches the dynamic read
       ``getattr(hass, 'config')`` the static attribute check cannot see).
@@ -604,13 +625,13 @@ def test_core_modules_have_no_ha_config_reads() -> None:
     the HA-object attributes ``.hass``/``.entry``/``.config_entry``, the
     config-entry attribute reads ``.config``/``.options``/``.data``, and
     dynamic ``getattr``/``hasattr`` reads of them — so a chain rooted at
-    ``self`` or ``runtime`` (which the old root-only check missed) is now
-    caught.  ``.config``/``.options``/``.data`` are banned because they are
-    Home Assistant config-entry attributes with no legitimate core use:
-    core receives the resolved values via the explicit
-    :class:`NestQuestSettings` object the integration builds from
-    ``entry.options``, so any read of those attributes in ``core/`` is a
-    config-entry coupling the settings object exists to remove.
+    ``self`` or ``runtime`` (whose root name is not a forbidden
+    identifier) is caught.  ``.config``/``.options``/``.data`` are banned
+    because they are Home Assistant config-entry attributes with no
+    legitimate core use: core receives the resolved values via the
+    explicit :class:`NestQuestSettings` object the integration builds
+    from ``entry.options``, so any read of those attributes in ``core/``
+    is a config-entry coupling the settings object exists to remove.
     Docstring mentions pass: AST only sees code.
     """
     py_files = sorted(CORE_PKG.rglob("*.py"))
@@ -625,11 +646,11 @@ def test_core_modules_have_no_ha_config_reads() -> None:
 
 
 def test_ast_scan_flags_each_bypass_shape() -> None:
-    """Negative control: the scanner flags each bypass shape the old check missed.
+    """Negative control: the scanner flags each bypass a root-only check misses.
 
-    The old scan only flagged chains rooted at a literal name ``hass``/
-    ``entry``.  Each snippet below is a shape it missed; the new scanner
-    must flag every one.
+    A root-only scan flags chains rooted at a literal name ``hass``/
+    ``entry``.  Each snippet below is a shape such a scan would miss;
+    this scanner must flag every one.
     """
     snippets: dict[str, str] = {
         "self.hass.config": "x = self.hass.config\n",
@@ -809,49 +830,18 @@ async def test_materialize_over_non_default_horizon(hass, tmp_path) -> None:
         await database.close()
 
 
-def _freeze_nestquest_clock(
-    monkeypatch, instant: datetime.datetime
-) -> None:
-    """Freeze the integration's ``datetime`` clock to a fixed aware instant.
-
-    Only ``custom_components.nestquest``'s ``datetime`` binding is patched,
-    so ``_run_horizon_materialization``'s ``datetime.datetime.now`` reads
-    this instant (localized to ``hass.config.time_zone``) while the test
-    harness and core keep the real clock.  Mirrors the helper in
-    test_timezones.py.
-    """
-    import custom_components.nestquest as nestquest
-
-    frozen = instant.astimezone(datetime.timezone.utc)
-
-    class _FrozenDatetime(datetime.datetime):
-        @classmethod
-        def now(cls, tz=None):
-            if tz is None:
-                return frozen.replace(tzinfo=None)
-            return frozen.astimezone(tz)
-
-    fake_module = SimpleNamespace(
-        datetime=_FrozenDatetime,
-        timedelta=datetime.timedelta,
-        date=datetime.date,
-    )
-    monkeypatch.setattr(nestquest, "datetime", fake_module)
-
-
 async def test_integration_path_threads_settings(
     hass, tmp_path, monkeypatch
 ) -> None:
     """The integration path threads ``settings`` through horizon_window + rollover.
 
-    P2-3: ``_run_horizon_materialization`` now calls
+    ``_run_horizon_materialization`` calls
     ``settings.horizon_window(today)`` (so the helper is real and shared,
     not recomputed inline), and ``_register_day_rollover_listener``
     registers at ``settings.day_rollover_hour_minute``.  This runs the
     INTEGRATION path with ``horizon_days=3`` and
     ``day_rollover_time='03:30'`` and asserts the resulting instance
-    window matches [today, today+3] AND the registered hour/minute are 3/30
-    — a test the requirement-level core-only check could not fail.
+    window matches [today, today+3] AND the registered hour/minute are 3/30.
     """
     import custom_components.nestquest as nestquest
     from custom_components.nestquest.core.dao_children import ChildrenDao
@@ -869,7 +859,7 @@ async def test_integration_path_threads_settings(
     # Freeze the integration clock so "today" is deterministic (the
     # hass fixture's config.time_zone is UTC, so 2026-03-01 12:00 UTC
     # localizes to 2026-03-01).
-    _freeze_nestquest_clock(
+    freeze_nestquest_clock(
         monkeypatch, datetime.datetime(2026, 3, 1, 12, 0, tzinfo=datetime.timezone.utc)
     )
 

tests/test_lifecycle.py

@@ -12,6 +12,7 @@ from custom_components.nestquest.const import (
     CONF_ADMIN_USER_IDS,
     CONF_DAY_ROLLOVER_TIME,
     CONF_HORIZON_DAYS,
+    CONF_MORNING_SUMMARY_TIME,
     DEFAULT_HORIZON_DAYS,
     SERVICE_REGENERATE,
     DOMAIN as DOMAIN_CONST,
@@ -526,29 +527,78 @@ async def test_setup_backfills_configured_horizon_days(hass, make_entry) -> None
     ).list_by_date_range(child_id, beyond, beyond_end) == []
 
 
-def test_from_options_horizon_validates_and_defaults() -> None:
-    """from_options returns the configured horizon, defaulting when absent/None.
+async def test_setup_succeeds_with_fully_malformed_options(
+    hass, make_entry
+) -> None:
+    """A fully malformed options dict falls back per field and setup succeeds.
 
-    Repointed from the removed ``_configured_horizon_days`` shim: a valid
-    horizon is read, an absent or None key falls back to the default, and
-    a malformed value RAISES (the integration's ``from_options_resilient``
-    handles the per-field fall-back at setup, not this strict path).
+    ``horizon_days=True``, ``day_rollover_time='99:99'`` and
+    ``morning_summary_time='25:00'`` are all invalid; the integration's
+    ``from_options_resilient`` falls each back to its default, so setup
+    SUCCEEDS, the day-rollover listener registers at the default hour 0
+    minute 0, and the startup backfill materializes the default horizon
+    window.
     """
-    from custom_components.nestquest.core.settings import NestQuestSettings
+    import datetime
 
-    assert NestQuestSettings.from_options({CONF_HORIZON_DAYS: 5}).horizon_days == 5
-    assert (
-        NestQuestSettings.from_options({}).horizon_days == DEFAULT_HORIZON_DAYS
+    from custom_components.nestquest.dao_instances import QuestInstancesDao
+    from custom_components.nestquest.db import NestQuestDatabase
+    from custom_components.nestquest.migrations import apply_migrations
+    from custom_components.nestquest.store import async_get_db_path
+
+    db_path = await async_get_db_path(hass)
+    pre = NestQuestDatabase(hass.async_add_executor_job)
+    await pre.open(db_path)
+    await apply_migrations(pre)
+    child_id = await _seed_daily_child_and_definition(pre, _local_today(hass))
+    await pre.close()
+
+    entry = _wire(
+        make_entry(
+            options={
+                CONF_HORIZON_DAYS: True,
+                CONF_DAY_ROLLOVER_TIME: "99:99",
+                CONF_MORNING_SUMMARY_TIME: "25:00",
+            }
+        ),
+        hass.registry,
     )
-    assert (
-        NestQuestSettings.from_options(
-            {CONF_HORIZON_DAYS: None}
-        ).horizon_days
-        == DEFAULT_HORIZON_DAYS
+    assert await async_setup_entry(hass, entry) is True
+
+    # The day-rollover listener fell back to the default 00:00.
+    registration = hass.time_change.registrations[0]
+    assert registration["hour"] == 0
+    assert registration["minute"] == 0
+
+    # The startup backfill used the default horizon window.
+    today = _local_today(hass)
+    end = (today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS)).isoformat()
+    records = await QuestInstancesDao(
+        entry.runtime_data.database
+    ).list_by_date_range(child_id, today.isoformat(), end)
+    assert len(records) == DEFAULT_HORIZON_DAYS + 1
+
+
+async def test_setup_keeps_horizon_when_sibling_option_is_bad(
+    hass, make_entry
+) -> None:
+    """A malformed sibling option does not reset the configured horizon.
+
+    ``horizon_days=5`` is valid while ``morning_summary_time='25:00'`` is
+    invalid; ``from_options_resilient`` falls back ONLY the bad sibling,
+    so the entry's resolved settings keep ``horizon_days == 5``.
+    """
+    entry = _wire(
+        make_entry(
+            options={
+                CONF_HORIZON_DAYS: 5,
+                CONF_MORNING_SUMMARY_TIME: "25:00",
+            }
+        ),
+        hass.registry,
     )
-    for bad in (True, "5", 0):
-        with pytest.raises(ValueError):
-            NestQuestSettings.from_options({CONF_HORIZON_DAYS: bad})
+    assert await async_setup_entry(hass, entry) is True
+    assert entry.runtime_data.settings.horizon_days == 5
 
 
 async def test_regenerate_service_registered_and_materializes(

tests/test_timezones.py

@@ -13,6 +13,7 @@ import datetime
 from types import SimpleNamespace
 from zoneinfo import ZoneInfo
 
+from conftest import freeze_nestquest_clock
 from custom_components.nestquest.const import DEFAULT_HORIZON_DAYS
 from custom_components.nestquest.core.settings import NestQuestSettings
 from custom_components.nestquest.dao_children import ChildrenDao
@@ -54,35 +55,10 @@ def _freeze_clock(
 ) -> tuple[datetime.datetime, list]:
     """Freeze the integration's ``datetime`` clock to a fixed aware instant.
 
-    Only ``custom_components.nestquest``'s ``datetime`` binding is patched, so
-    the generation path's "today" reads this instant (localized to whatever
-    time zone it asks for) while every other module and the test harness keep
-    the real clock.  Returns ``(frozen, requested)`` where ``frozen`` is the
-    instant normalized to UTC and ``requested`` is the list of ``tzinfo``
-    objects the code under test passed to ``now`` — so a test can prove the
-    generation path actually requested the configured HA time zone rather
-    than silently reading UTC.
+    Thin wrapper over :func:`conftest.freeze_nestquest_clock` kept for the
+    ``(frozen, requested)`` return shape the timezone assertions read.
     """
-    import custom_components.nestquest as nestquest
-
-    frozen = instant.astimezone(datetime.timezone.utc)
-    requested: list = []
-
-    class _FrozenDatetime(datetime.datetime):
-        @classmethod
-        def now(cls, tz=None):
-            requested.append(tz)
-            if tz is None:
-                return frozen.replace(tzinfo=None)
-            return frozen.astimezone(tz)
-
-    fake_module = SimpleNamespace(
-        datetime=_FrozenDatetime,
-        timedelta=datetime.timedelta,
-        date=datetime.date,
-    )
-    monkeypatch.setattr(nestquest, "datetime", fake_module)
-    return frozen, requested
+    return freeze_nestquest_clock(monkeypatch, instant)
 
 
 def _assert_requested_ha_timezone(requested, time_zone: str) -> None: