Repository navigation
Import new device diagnostics - #922
Conversation
zigpy-review-bot
left a comment
There was a problem hiding this comment.
The importer and decoder changes look good, but the 215 new snapshots fail test_devices_from_files on current dev. CI hasn't caught this yet: the Python 3.12 "Prepare base dependencies" job failed, so the pre-commit and test jobs were skipped.
Blocker: every new snapshot fails the round-trip. All 215 new files have "endpoint_id": null on their LQISensor and RSSISensor entities. On dev, those entities get their endpoint ID, so the full suite at this head gives 215 failed, 925 passed in test_devices_from_files, and the failures are exactly the 215 new files. The snapshots look like they were generated on a tree that includes #919: that PR changes exactly these fields to null across the existing corpus. Running python -m tools.regenerate_diagnostics on the 215 new files on top of this branch changes only those two lines per file (430 lines) and fixes all but one. If #919 is meant to land first, regenerating after rebasing onto it also works.
The last one is a test-helper issue, not the snapshot. After the regen, candeo-c-zb-rd1p-rem-0x0000000c.json still fails, deterministically, on the identify-on-join assertion. Details inline.
Verified (6 checks)
- Full suite in a fresh worktree venv (zigpy 2.3.0, zha-quirks 2.2.2) at a6bfab4: the 215 new snapshots fail and the other 925 pass. After regenerating the 215 files, the only remaining failure is the Candeo remote. With the
_get_identify_clusterchange suggested inline,tests/test_discover.pypasses (1159 tests). - #919 as the source: on #919's branch, the existing snapshots switch
LQISensor/RSSISensorfrom the endpoint ID tonull. With this branch merged into #919's head, 214 of the 215 new snapshots pass. The Candeo remote fails there too, for the identify reason below. The other failures in that tree are the 52 snapshots #920 added, which #919 hasn't regenerated yet. - Legacy/modern switch (
"signature" in zha_data):Device.get_diagnostics_jsonhas never written asignaturekey, from its introduction in #284 to now. After itsnwkcheck, the legacy loader readsdevice_data["signature"]unconditionally. So a dump without a signature could only be skipped or raise aKeyErroron that path before, and it can't lose a dump that used to import. Trimmed modern dumps withoutversionare now skipped instead. - Stricter
decode_json_value: the committed snapshots only ever carry"__type": "<class 'bytes'>"(14 occurrences). That's becauseZhaJsonEncoderwrites plainbytes, and decoded arrays come back out as plain JSON lists. The other listed types only show up in the raw HA dumps the importer reads. All 1140 snapshots load with the new decoder. - No identifying data: all 215 new snapshots have a faked
ab:cd:ef:…IEEE and contain no leftoverREDACTEDstrings. - Real mypy in the venv: clean.
ruff check/ruff format --checkontests/common.pyandtools/import_diagnostics.py: clean.
|
Pushed 5e5ef62210e08b8d0278ff7a7163ba1a6cb279e07 on request from zigpy/zha-device-handlers#5415: one new snapshot, |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #922 +/- ##
=======================================
Coverage 97.58% 97.58%
=======================================
Files 61 61
Lines 10870 10870
=======================================
Hits 10607 10607
Misses 263 263 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
These new diagnostics encompass devices attached to PRs, earlier imports did not include them.
We now have over 1k devices in our test suite (1,141).