Cover DevicesController lifecycle end-to-end by bdraco · Pull Request #195 · esphome/device-builder

bdraco · 2026-05-03T14:09:20Z

Summary

The handler-level tests in `tests/controllers/devices/` all bypass `init` via `new` and stub the inner controllers individually. That left the wiring code itself uncovered: `init` (lines 81-129) constructing the scanner / state monitor / MQTT coordinator and threading their nine callbacks back to the controller, plus `start` / `stop` / `poll` (lines 142-164).

6 tests in `tests/controllers/devices/test_lifecycle_e2e.py` instantiate a real `DevicesController` against a `tmp_path` config dir and a thin stub `DeviceBuilder`:

`init` constructs all three inner controllers, sets the documented default attrs, wires the scanner against `tmp_path`.
State-monitor callbacks point back at `self.on*_change` — pin the seven distinct wires (regression class from PR Stop dropping mDNS state on Device rebuilds; speed up apply_* lookups #75).
`start` resolves esphome cmd, loads ignored, scans, starts the state monitor, reconciles MQTT, registers the JOB_COMPLETED bus listener.
`stop` unsubscribes the bus listener and stops both monitors.
`stop` is idempotent without a started listener (cold process restart).
`poll` rescans + reconciles MQTT.

Inner monitor lifecycle methods are patched as `AsyncMock` so `start` / `stop` don't try to open a zeroconf browser or connect to MQTT — those have their own dedicated test files.

Test plan

`uv run pytest tests/controllers/devices/test_lifecycle_e2e.py` — 6 passed
`uv run pre-commit run` clean

codspeed-hq · 2026-05-03T14:10:07Z

Merging this PR will not alter performance

✅ 5 untouched benchmarks

_{Comparing cover-devices-lifecycle-e2e (2b1ce82) with main (b6a5e37)}

codecov-commenter · 2026-05-03T14:11:46Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.89%. Comparing base (b6a5e37) to head (2b1ce82).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #195      +/-   ##
==========================================
+ Coverage   92.85%   92.89%   +0.04%     
==========================================
  Files          43       43              
  Lines        4924     4924              
==========================================
+ Hits         4572     4574       +2     
+ Misses        352      350       -2

Flag	Coverage Δ
py3.12	`92.83% <ø> (+0.04%)`	⬆️
py3.14	`92.89% <ø> (+0.04%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 1 file with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

Adds end-to-end tests to cover the real DevicesController lifecycle wiring (__init__, start, stop, poll) that existing handler-level tests miss due to constructing controllers via __new__.

Changes:

Introduces test_lifecycle_e2e.py to instantiate a real DevicesController against a tmp_path config dir using a thin DeviceBuilder-shaped stub.
Pins DeviceStateMonitor callback wiring back to controller methods to prevent regressions in controller ↔ monitor glue.
Verifies start/stop/poll invoke the expected scan/monitor/MQTT and event-bus subscription teardown behavior (with inner lifecycles patched via AsyncMock).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Docstring miscount: the state monitor wires *seven* callbacks back to controller methods (state / ip / version / config_hash / api_encryption / importable_added / importable_removed), not nine. Update the docstring to reflect the actual count and list them so a refactor that adds an eighth surfaces here too.

The handler-level tests in ``tests/controllers/devices/`` all bypass ``__init__`` via ``__new__`` and stub the inner controllers individually. That left the wiring code itself uncovered: ``__init__`` (lines 81-129) constructing the scanner / state monitor / MQTT coordinator and threading their nine callbacks back to the controller, plus ``start`` / ``stop`` / ``poll`` (lines 142-164). 6 tests instantiate a real ``DevicesController`` against a ``tmp_path`` config dir and a thin stub ``DeviceBuilder``: - ``__init__`` constructs all three inner controllers, sets the documented default attrs, wires the scanner against ``tmp_path``. - State-monitor callbacks point back at ``self._on_*_change`` — pin the seven distinct wires (regression class from PR #75). - ``start`` resolves esphome cmd, loads ignored, scans, starts the state monitor, reconciles MQTT, registers the JOB_COMPLETED bus listener. - ``stop`` unsubscribes the bus listener and stops both monitors. - ``stop`` is idempotent without a started listener (cold process restart). - ``poll`` rescans + reconciles MQTT. Inner monitor lifecycle methods are patched as ``AsyncMock`` so ``start`` / ``stop`` don't try to open a zeroconf browser or connect to MQTT — those have their own dedicated test files.

Docstring miscount: the state monitor wires *seven* callbacks back to controller methods (state / ip / version / config_hash / api_encryption / importable_added / importable_removed), not nine. Update the docstring to reflect the actual count and list them so a refactor that adds an eighth surfaces here too.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

bdraco · 2026-05-03T15:13:44Z

+- ``__init__`` (lines 81-129) — constructs the scanner, state
+  monitor, and MQTT coordinator and threads their callbacks
+  back to the controller.
+- ``start()`` (lines 142-149) — resolves the esphome cmd, loads
+  ignored devices, kicks the scanner, starts the state monitor,
+  reconciles MQTT, and registers the JOB_COMPLETED listener.
+- ``stop()`` (lines 155-159) — unsubscribes the bus listener and
+  stops the two background monitors.
+- ``poll()`` (lines 163-164) — re-scans and reconciles MQTT.


Dropped the hard-coded line ranges from the module docstring in commit 86176aa — describes the methods/behaviour being covered instead so the docstring doesn't drift as the controller evolves.

bdraco · 2026-05-03T15:13:53Z

+    elsewhere, but the *order* and *fact-of-call* live here. A
+    refactor that reordered (e.g. ``state_monitor.start`` before
+    ``scanner.scan``) could cause cold-start ordering bugs the
+    individual tests wouldn't catch.


Good catch — added a parent MagicMock with attach_mock for scan / state_monitor.start / reconcile, then assert observed_order == ["scan", "state_monitor_start", "reconcile"] so the relative order is now actually pinned. The state monitor reads self._scanner.devices for its first sweep, so swapping those would have it iterate over an empty list at cold-start.

- Drop the hard-coded line-range references from the module docstring (e.g. "__init__ (lines 81-129)"); those drift as the controller evolves and turn into stale lies. Describe the methods/behaviour being covered instead. - ``test_start_runs_full_initialisation_chain`` now actually asserts the call order via a parent ``MagicMock`` with ``attach_mock``. Production order is ``scan → state_monitor.start → reconcile``; swapping state-monitor start ahead of scan would have the monitor iterate over an empty device list at cold-start. The previous shape only verified each call happened, not its relative position.

The test mostly asserted that `__init__` set the defaults `__init__` obviously sets — `_db is db`, `_esphome_cmd == []`, empty sets, etc. The two meaningful claims it made (the inner controllers exist + are wired against the right config dir) are already covered: - `test_init_threads_state_monitor_callbacks_to_controller_methods` reads `controller._state_monitor._on_state_change` etc., so it crashes if the state monitor was dropped from `__init__`. - The `start` / `stop` / `poll` tests below all call into `_scanner`, `_state_monitor`, and `_mqtt_coordinator` and would crash on `AttributeError` if any were missing. Removing the redundant pin reduces noise without losing any regression coverage.

Both lived as file-local helpers in test_lifecycle_e2e.py but fit the same "shared shape future tests will want" rationale as the existing `make_controller` and `seed_device` factories. Move them into `conftest.py` as `StubBus` and the `make_db` fixture so the next test that needs to construct a real `DevicesController` (i.e. doesn't bypass-init via `make_controller`) can request `make_db` instead of copying the stub shape.

bdraco added the enhancement Improvement to an existing feature label May 3, 2026

Copilot AI review requested due to automatic review settings May 3, 2026 14:09

bdraco added the enhancement Improvement to an existing feature label May 3, 2026

bdraco force-pushed the cover-devices-lifecycle-e2e branch from 8af5aea to 720ba48 Compare May 3, 2026 14:09

Copilot started reviewing on behalf of bdraco May 3, 2026 14:10 View session

Copilot AI reviewed May 3, 2026

View reviewed changes

Comment thread tests/controllers/devices/test_lifecycle_e2e.py Outdated

Comment thread tests/controllers/devices/test_lifecycle_e2e.py Outdated

bdraco force-pushed the cover-devices-lifecycle-e2e branch from a8a3868 to 4d75b9e Compare May 3, 2026 14:42

bdraco force-pushed the cover-devices-lifecycle-e2e branch from 4d75b9e to f2e1ae2 Compare May 3, 2026 14:46

bdraco force-pushed the cover-devices-lifecycle-e2e branch from f2e1ae2 to 6ee0814 Compare May 3, 2026 14:59

bdraco requested a review from Copilot May 3, 2026 15:01

Copilot started reviewing on behalf of bdraco May 3, 2026 15:01 View session

bdraco added 2 commits May 3, 2026 10:02

bdraco force-pushed the cover-devices-lifecycle-e2e branch from 6ee0814 to 694e898 Compare May 3, 2026 15:02

Copilot AI reviewed May 3, 2026

View reviewed changes

bdraco added 3 commits May 3, 2026 10:06

bdraco merged commit cc8e29e into main May 3, 2026
14 checks passed

bdraco deleted the cover-devices-lifecycle-e2e branch May 3, 2026 15:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Cover DevicesController lifecycle end-to-end#195

Cover DevicesController lifecycle end-to-end#195
bdraco merged 5 commits into
mainfrom
cover-devices-lifecycle-e2e

bdraco commented May 3, 2026

Uh oh!

codspeed-hq Bot commented May 3, 2026 •

edited

Loading

Uh oh!

codecov-commenter commented May 3, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

bdraco May 3, 2026

Uh oh!

bdraco May 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

bdraco commented May 3, 2026

Summary

Test plan

Uh oh!

codspeed-hq Bot commented May 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Uh oh!

codecov-commenter commented May 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

bdraco May 3, 2026

Choose a reason for hiding this comment

Uh oh!

bdraco May 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codspeed-hq Bot commented May 3, 2026 •

edited

Loading

codecov-commenter commented May 3, 2026 •

edited

Loading