Repository navigation
Make test_database persist last_seen it checks - #1901
Draft
TheJulianJES wants to merge 1 commit into
Draft
TheJulianJES wants to merge 1 commit into
TheJulianJES wants to merge 1 commit into
Conversation
`_save_device_last_seen` skips updates within `MIN_UPDATE_DELTA` (30 s) of the stored value, so setting `last_seen` to the current time was never written. The reloaded value was the device's earlier save time, and the 10 ms tolerance only measured how fast the test ran, which failed on a slow CI runner. Set it past the throttle so the update is actually persisted and checked.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #1901 +/- ##
=======================================
Coverage 99.51% 99.51%
=======================================
Files 61 61
Lines 12773 12773
=======================================
Hits 12711 12711
Misses 62 62 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused test correction accurately accounts for the persistence throttle and prevents timing-dependent results.
Review effort: Balanced
Findings: None
What changed in this PR
Updates test_database to reliably exercise persistence of throttled last_seen updates.
Changes:
- Advances the test timestamp beyond
MIN_UPDATE_DELTA. - Documents why the future timestamp is necessary.
| File | Description |
|---|---|
tests/test_appdb.py |
Ensures the database write path for last_seen is tested reliably. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This was referenced Oct 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes test failure seen in https://github.com/zigpy/zigpy/actions/runs/37516303325/job/112450383118?pr=1900 and actually makes the test work and test
last_seenbeing written.Changes
test_databasefailed on a CI runner for #1900 withassert abs(dev.last_seen - dev_last_seen) < 0.01(10.8 ms), although nothing in that PR touches this path.The cause is in the test:
_save_device_last_seenonly writes a newlast_seenwhen it is more thanMIN_UPDATE_DELTA(30 s) newer than the stored value. The test setslast_seento the current time, so the update is never written, and the reloaded value is the device's earlier save time. The 10 ms tolerance therefore measured how long that stretch of the test took. Locally it is about 1 ms on bothdevand #1900; a slow runner can exceed 10 ms.The test now sets
last_seenpast the throttle, so the update is persisted and the reloaded value matches exactly (0 ms difference). It also covers the write path now: withdevice_last_seen_updated()disabled, the old test still passed, while the new one fails.