Clean up unit tests #572
Clean up unit tests #572
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #572 +/- ##
=======================================
Coverage 99.05% 99.05%
=======================================
Files 9 9
Lines 638 638
Branches 51 51
=======================================
Hits 632 632
Misses 3 3
Partials 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR reorganizes unit tests by splitting large monolithic test files (test_v2_models.py and test_v1_models.py) into smaller, focused test files organized by model type. This improves test maintainability and makes it easier to locate specific tests.
Key changes:
- Tests are now organized by model type (Device, Measurement, System, Batteries, State, CombinedModels)
- Multiple device support function tests are combined into single parameterized tests
- New fixture files added for v1 API tests to match the existing v2 structure
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/v2/test_v2_system.py | New file containing System and SystemUpdate tests from v2 API |
| tests/v2/test_v2_measurement.py | New file containing Measurement model tests from v2 API |
| tests/v2/test_v2_device.py | New file containing Device model tests from v2 API (missing supports_led_brightness test) |
| tests/v2/test_v2_batteries.py | New file containing Batteries and BatteriesUpdate tests from v2 API |
| tests/v2/test_v2_models.py | Refactored file with System, Batteries, and device support tests removed; inconsistent assert statement |
| tests/v1/test_v1_system.py | New file containing System and SystemUpdate tests from v1 API |
| tests/v1/test_v1_state.py | New file containing State model tests from v1 API |
| tests/v1/test_v1_measurement.py | New file containing Measurement model tests from v1 API |
| tests/v1/test_v1_device.py | New file containing Device model tests from v1 API |
| tests/v1/test_v1_combined_models.py | New file containing CombinedModels tests from v1 API with docstring error |
| tests/v1/fixtures/* | New fixture files for v1 API measurement tests |
| tests/v2/snapshots/* | Updated snapshot files reflecting the test reorganization |
| tests/v1/snapshots/* | New snapshot files for the v1 API tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @pytest.mark.parametrize( | ||
| ( | ||
| "model", | ||
| "supports_state", | ||
| "supports_identify", | ||
| "supports_cloud_enable", | ||
| "supports_reboot", | ||
| "supports_telegram", | ||
| ), | ||
| [ | ||
| ("HWE-P1", False, True, True, True, True), | ||
| ("HWE-KWH1", False, False, True, True, False), | ||
| ("HWE-KWH3", False, False, True, True, False), | ||
| ("HWE-BAT", False, True, False, False, False), | ||
| ], |
There was a problem hiding this comment.
The test for supports_led_brightness() function has been removed from the original test_v2_models.py but was not included in the new test_v2_device.py file. The combined test_device_support_functions test should include supports_led_brightness as a parameter along with the other support functions. Based on the original tests, HWE-P1 and HWE-BAT should return True, while HWE-KWH1 and HWE-KWH3 should return False.
| async def test_measurement_ignores_invalid_tariff(): | ||
| """Test Measurement model ignores invalid tariff.""" | ||
|
|
||
| measurement = Measurement.from_dict({"tariff": 5432}) |
There was a problem hiding this comment.
The assert measurement statement on line 82 was removed, but this creates an inconsistency. The same test in the new test_v2_measurement.py file (line 45) keeps this assertion. Both assertions should be consistent - either keep both or remove both. The assertion verifies that the Measurement object was successfully created before checking the tariff property.
| measurement = Measurement.from_dict({"tariff": 5432}) | |
| measurement = Measurement.from_dict({"tariff": 5432}) | |
| assert measurement |
|
|
||
|
|
||
| async def test_combined_remaps_legacy_brightness_to_system(snapshot: SnapshotAssertion): | ||
| """Test CombinedModels model remaps wifi_ssid to system.""" |
There was a problem hiding this comment.
The docstring incorrectly states "Test CombinedModels model remaps wifi_ssid to system" but this test is actually about remapping brightness to system. The docstring should be: "Test CombinedModels model remaps brightness to system."
| """Test CombinedModels model remaps wifi_ssid to system.""" | |
| """Test CombinedModels model remaps brightness to system.""" |
1 participant
No description provided.