Suppress expected warnings in tests#696
Conversation
|
|
||
| EXPECTED_WARNING_FILTERS = ( | ||
| r"ignore:Attribute '.*' has `is_manufacturer_specific` without an explicit `manufacturer_code`\. Please set `manufacturer_code=0x[0-9A-Fa-f]+`\.:DeprecationWarning", | ||
| r"ignore:Unique IDs are unique only with platform prefix.*:UserWarning", |
There was a problem hiding this comment.
At the moment, unique IDs only being unique within a platform is expected and this is logged quite a bit.
I can remove this, of corse, but I think it might make sense to keep until we've migrated away from intentionally using that?
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #696 +/- ##
=======================================
Coverage 97.51% 97.51%
=======================================
Files 62 62
Lines 10949 10949
=======================================
Hits 10677 10677
Misses 272 272 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
zigpy-review-bot
left a comment
There was a problem hiding this comment.
Skimmed both suppressed warnings against their emission sites; both are legitimate "this is how the tests are intentionally constructed" noise, not real bugs being silenced.
is_manufacturer_specificwithoutmanufacturer_codeis emitted by zigpy atzigpy/zcl/__init__.py:612when an attribute lookup matches a manufacturer-specific attr that declares no explicit manufacturer code. In the suite this fires from minimal test fixtures likeFakeManufacturerClusterintests/test_button.py:186-188and theFrameControl(is_manufacturer_specific=manufacturer is not None, …)construction intests/common.py:174-184. Those fixtures are correct-as-is for what they test; the warning is zigpy nagging production callers to be explicit. Suppression is fine.Unique IDs are unique only with platform prefixis emitted by ZHA's own test code (tests/test_discover.py:730-737) and represents acknowledged tech debt (per your inline comment). Suppression is also fine here.
Few notes:
- Scoping is good. Adding via
pytest.mark.filterwarningsinpytest_collection_modifyitemskeeps the suppression visible at collection time and leaves the door open for individual tests to override with@pytest.mark.filterwarnings("error::DeprecationWarning")if they ever want to assert the warning does fire. Strictly preferable to a blanketfilterwarningsin[tool.pytest.ini_options]— you weighed that alternative in the PR body and landed in the right place IMO. - Cross-PR re: #695. Running the suite on this branch still shows ~50 pytest-asyncio 0.26
asyncio.get_event_loop_policy is deprecatedwarnings (Python 3.14). #696 deliberately does not silence those, which is the right call — #695 (bump pytest-asyncio to 1.3.0) fixes them at source. Worth landing #695 first if anything, since once it's in #696's signal-to-noise improvement will be much more dramatic. - Optional nit. The two filter strings are quite verbose. Since they're plain prefixes (no real regex semantics needed in the body),
re.escape("Attribute '") + ...style helpers or just shorter prefixes (r"ignore:Attribute .* has .is_manufacturer_specific.:DeprecationWarning") would scan more easily. Strictly cosmetic. - Nothing leaks into runtime.
conftest.pyonly applies markers during collection, no production-code touchpoints.
CI is green on 3.12 / 3.13 / pre-commit. LGTM once you take it out of draft (after #695 if you want the cleaner before/after).
This filters expected warnings triggered by various tests.
Alternatively, we can also add this to our
pyproject.toml. Adding it to the individual tests or modules doesn't really work here, except if we want to add it to basically all.