Implement snapshot testing #906
Conversation
WalkthroughAdds a two-step pytest flow in Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Script as scripts/core-testing.sh
participant Py as pytest
Dev->>Script: Run core-testing.sh
Script->>Py: eval "$PYTEST_COMMAND"
alt First run succeeds
Py-->>Script: Exit 0
Script-->>Dev: Print success (no update)
else First run fails
Py-->>Script: Exit !=0
Script->>Py: eval "$PYTEST_COMMAND --snapshot-update"
alt Re-run succeeds
Py-->>Script: Exit 0
Script->>Script: SNAPSHOT_UPDATED=1
Script-->>Dev: Print note about updated snapshots (non-CI)
else Re-run fails
Py-->>Script: Exit !=0
Script-->>Dev: Exit with error
end
end
sequenceDiagram
autonumber
participant Test as Test
participant HA as HomeAssistant
participant CE as MockConfigEntry
participant PW as PlugwiseIntegration
Test->>CE: Create mock config entry
Test->>HA: Add CE to hass
Test->>PW: Patch PLATFORMS to `platforms` fixture
Test->>HA: async_setup_entry(CE)
HA->>PW: Initialize selected platforms
HA-->>Test: Setup complete
Test->>Test: Run snapshot_platform
Test-->>Test: Assert snapshot matches saved .ambr
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
tests/components/plugwise/test_sensor.py (1)
30-185: Complete snapshot testing migration in test_sensor.pyWe’ve confirmed that
tests/components/plugwise/snapshots/test_sensor.ambrexists and covers the three snapshot-based tests:• test_anna_sensor_states
• test_p1_dsmr_sensor_entities
• test_stretch_sensor_entitiesHowever, the following functions still use explicit assertions:
• test_adam_climate_sensor_entities (Lines 30–41)
• test_adam_climate_sensor_entity_2 (Lines 43–51)
• test_unique_id_migration_humidity (Lines 53–82)
• test_p1_3ph_dsmr_sensor_entities (Lines 95–128)To keep the file consistent and maintainable, either:
- Migrate these remaining tests to snapshot tests—capturing expected states and registry entries in
test_sensor.ambr.- Or document why explicit assertions are needed (e.g., unique-ID migrations or specialized value checks that don’t map cleanly to snapshots).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
custom_components/plugwise/coordinator.py(1 hunks)scripts/core-testing.sh(2 hunks)tests/components/plugwise/conftest.py(2 hunks)tests/components/plugwise/snapshots/test_binary_sensor.ambr(1 hunks)tests/components/plugwise/snapshots/test_climate.ambr(1 hunks)tests/components/plugwise/snapshots/test_switch.ambr(1 hunks)tests/components/plugwise/test_binary_sensor.py(1 hunks)tests/components/plugwise/test_climate.py(4 hunks)tests/components/plugwise/test_config_flow.py(1 hunks)tests/components/plugwise/test_diagnostics.py(1 hunks)tests/components/plugwise/test_init.py(1 hunks)tests/components/plugwise/test_sensor.py(3 hunks)tests/components/plugwise/test_switch.py(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: bouwew
PR: plugwise/plugwise-beta#793
File: custom_components/plugwise/manifest.json:10-11
Timestamp: 2024-12-12T19:48:34.652Z
Learning: In the Plugwise integration, temporary use of alpha versions from test repositories is acceptable for testing purposes.
Learnt from: bouwew
PR: plugwise/plugwise-beta#671
File: custom_components/plugwise/switch.py:82-83
Timestamp: 2024-06-22T08:05:56.791Z
Learning: User bouwew prefers to ignore exception handling suggestions for the `pw-beta` branch in the `custom_components/plugwise/switch.py` file.
Learnt from: bouwew
PR: plugwise/plugwise-beta#671
File: custom_components/plugwise/switch.py:82-83
Timestamp: 2024-10-08T15:35:25.180Z
Learning: User bouwew prefers to ignore exception handling suggestions for the `pw-beta` branch in the `custom_components/plugwise/switch.py` file.
Learnt from: bouwew
PR: plugwise/plugwise-beta#734
File: tests/components/plugwise/test_config_flow.py:58-58
Timestamp: 2024-09-27T09:48:30.890Z
Learning: In `tests/components/plugwise/test_config_flow.py`, the versions are mocked firmware versions for testing purposes and do not need to match the software version in `manifest.json`.
Learnt from: bouwew
PR: plugwise/plugwise-beta#734
File: tests/components/plugwise/test_config_flow.py:58-58
Timestamp: 2024-10-08T15:35:25.180Z
Learning: In `tests/components/plugwise/test_config_flow.py`, the versions are mocked firmware versions for testing purposes and do not need to match the software version in `manifest.json`.
Learnt from: CoMPaTech
PR: plugwise/plugwise-beta#0
File: :0-0
Timestamp: 2025-01-25T20:42:24.862Z
Learning: The linting rules in plugwise-beta have been updated to use TC001/TC002/TC003 instead of TCH001/TCH002/TCH003 for type-checking related rules.
Learnt from: CoMPaTech
PR: plugwise/plugwise-beta#0
File: :0-0
Timestamp: 2025-01-25T20:42:24.862Z
Learning: In the plugwise-beta project, `uv` package manager was removed from core-testing.sh in favor of pip due to issues it was causing.
📚 Learning: in `tests/components/plugwise/test_config_flow.py`, the versions are mocked firmware versions for te...
Learnt from: bouwew
PR: plugwise/plugwise-beta#734
File: tests/components/plugwise/test_config_flow.py:58-58
Timestamp: 2024-09-27T09:48:30.890Z
Learning: In `tests/components/plugwise/test_config_flow.py`, the versions are mocked firmware versions for testing purposes and do not need to match the software version in `manifest.json`.
Applied to files:
custom_components/plugwise/coordinator.pytests/components/plugwise/test_config_flow.pytests/components/plugwise/test_diagnostics.pytests/components/plugwise/test_init.pytests/components/plugwise/test_climate.pytests/components/plugwise/conftest.pytests/components/plugwise/test_switch.pytests/components/plugwise/test_sensor.py
📚 Learning: in the plugwise-beta project, `uv` package manager was removed from core-testing.sh in favor of pip ...
Learnt from: CoMPaTech
PR: plugwise/plugwise-beta#0
File: :0-0
Timestamp: 2025-01-25T20:42:24.862Z
Learning: In the plugwise-beta project, `uv` package manager was removed from core-testing.sh in favor of pip due to issues it was causing.
Applied to files:
tests/components/plugwise/test_config_flow.py
📚 Learning: in the plugwise integration, temporary use of alpha versions from test repositories is acceptable fo...
Learnt from: bouwew
PR: plugwise/plugwise-beta#793
File: custom_components/plugwise/manifest.json:10-11
Timestamp: 2024-12-12T19:48:34.652Z
Learning: In the Plugwise integration, temporary use of alpha versions from test repositories is acceptable for testing purposes.
Applied to files:
tests/components/plugwise/conftest.py
🧬 Code Graph Analysis (4)
tests/components/plugwise/test_binary_sensor.py (2)
tests/components/plugwise/test_climate.py (1)
test_binary_sensor_states(36-44)tests/components/plugwise/conftest.py (2)
mock_smile_anna(240-263)setup_platform(135-147)
tests/components/plugwise/test_climate.py (2)
tests/components/plugwise/test_binary_sensor.py (1)
test_binary_sensor_states(23-31)tests/components/plugwise/conftest.py (2)
mock_smile_anna(240-263)setup_platform(135-147)
tests/components/plugwise/test_switch.py (1)
tests/components/plugwise/conftest.py (3)
mock_smile_adam(151-181)setup_platform(135-147)mock_stretch(320-343)
tests/components/plugwise/test_sensor.py (1)
tests/components/plugwise/conftest.py (5)
mock_smile_adam(151-181)setup_platform(135-147)mock_smile_anna(240-263)mock_smile_p1(267-289)mock_stretch(320-343)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Prepare
🔇 Additional comments (27)
custom_components/plugwise/coordinator.py (1)
5-5: LGTM! Import reorganization improves consistency.Moving the
Versionimport earlier in the import block aligns with Python import conventions and maintains consistency across the codebase.tests/components/plugwise/test_config_flow.py (1)
6-6: LGTM! Import reorganization maintains consistency.Moving the
Versionimport earlier matches the same organizational improvement made incoordinator.py, ensuring consistent import ordering across the Plugwise integration.tests/components/plugwise/conftest.py (4)
4-4: LGTM! Good addition for fixture typing.Adding
AsyncGeneratorandGeneratorimports fromcollections.abcproperly supports the type hints for the async fixtures.
9-10: LGTM! Imports support existing functionality.The
MunchandVersionimports are already used elsewhere in the file but were missing from the imports section.
128-131: LGTM! Clean default fixture implementation.The
platformsfixture provides a sensible default (empty list) that can be overridden via parametrization for selective platform testing.
134-147: LGTM! Well-designed setup fixture for snapshot testing.The
setup_platformfixture properly:
- Adds the config entry to Home Assistant
- Patches the PLATFORMS attribute to control which platforms are loaded
- Sets up the config entry and waits for completion
- Yields the config entry for test use
This enables the snapshot testing approach being implemented across the test suite.
tests/components/plugwise/snapshots/test_binary_sensor.ambr (1)
1-386: LGTM! Well-structured snapshot baseline.The snapshot file properly captures:
- Entity registry entries with appropriate metadata
- State snapshots with current values
- Dynamic value placeholders using
<ANY>for IDs and timestamps- Comprehensive coverage of binary sensor entities for the test environment
This provides a solid baseline for snapshot-based testing of the binary sensor platform.
tests/components/plugwise/test_binary_sensor.py (4)
6-6: LGTM! Proper import for freezegun type hints.Adding
FrozenDateTimeFactoryimport improves type safety for the existing test functions.
8-8: LGTM! Essential import for snapshot testing.Adding
SnapshotAssertionfrom syrupy enables the snapshot-based testing approach.
10-10: LGTM! Necessary imports for the refactored test.The additions of:
BINARY_SENSOR_DOMAINfor parametrizationentity_registryfor registry assertionssnapshot_platformhelper functionAll support the new snapshot testing approach being implemented.
Also applies to: 13-13, 16-16
21-31: LGTM! Clean snapshot-based test implementation.The refactored test properly:
- Uses parametrization to target only the binary sensor domain
- Leverages the new
setup_platformfixture for selective platform setup- Implements snapshot validation via
snapshot_platformhelper- Follows the consistent pattern being applied across the test suite
This is much cleaner than the previous approach with individual entity assertions.
tests/components/plugwise/test_climate.py (2)
7-7: LGTM! Proper imports for snapshot testing.The import additions properly support the new snapshot-based testing approach:
FrozenDateTimeFactoryfor type hintsSnapshotAssertionfor snapshot assertionsentity_registryfor registry validationsnapshot_platformhelper functionAlso applies to: 10-10, 21-21, 23-23
432-432: LGTM! Good fix for async completion.Adding the extra
await hass.async_block_till_done()call ensures all asynchronous operations complete before the assertions, which is a good practice in async tests.tests/components/plugwise/test_switch.py (3)
19-20: LGTM! Essential imports for snapshot testing.The additions of:
entity_registryfor registry validationSnapshotAssertionfor snapshot assertionssnapshot_platformhelper functionAll properly support the new snapshot-based testing approach.
Also applies to: 22-22
25-35: LGTM! Proper snapshot test for Adam switches.The test correctly:
- Parametrizes for the switch domain only
- Uses the
entity_registry_enabled_by_defaultfixture- Leverages the new
setup_platformfixture- Implements snapshot validation via
snapshot_platformThis follows the consistent refactoring pattern being applied across the test suite.
111-121: LGTM! Proper snapshot test for Stretch switches.The test correctly follows the same pattern as the Adam test:
- Proper parametrization for switch domain
- Uses appropriate fixtures including
mock_stretch- Implements snapshot validation consistently
Both switch snapshot tests maintain the same high-quality structure.
tests/components/plugwise/test_sensor.py (7)
6-6: Import added for snapshot testing.The addition of
SnapshotAssertionimport supports the new snapshot-based testing approach.
11-11: Import updated for entity registry access.The change from importing the entity_registry helper to importing it as
erprovides cleaner access to entity registry functionality in the snapshot tests.
14-14: Import added for snapshot platform helper.The
snapshot_platformimport enables the new snapshot-based testing functionality.
17-27: Snapshot-based test implementation looks good.The conversion from explicit state assertions to snapshot testing using
snapshot_platformis well-implemented. The parametrized approach with platform filtering and entity registry fixtures provides comprehensive validation.
111-121: Anna sensor test converted to snapshot testing.The refactoring maintains the existing parametrization for environment and cooling settings while adding snapshot functionality. The implementation is consistent with the other converted tests.
126-136: P1 DSMR sensor test converted appropriately.The test maintains the specific environment and gateway ID parametrization while adopting the snapshot testing approach. This ensures the same test coverage with improved maintainability.
175-185: Stretch sensor test converted to snapshot testing.The conversion follows the same pattern as other snapshot-based tests, providing consistent validation across different device types.
tests/components/plugwise/snapshots/test_switch.ambr (1)
1-1276: Snapshot content acceptedThe
.ambrfile looks like a standard Syrupy snapshot dump containing only hashed/opaque identifiers and generic meta-data—nothing obviously sensitive is exposed. No further action required.tests/components/plugwise/test_diagnostics.py (1)
6-7: Import reorder is harmlessMoving the
HomeAssistantimport below external-library imports keeps third-party and HA-specific modules grouped consistently; no issues spotted.tests/components/plugwise/test_init.py (1)
6-6: Import position change is fineRelocating
FrozenDateTimeFactoryso that all stdlib/third-party imports are grouped makes the section slightly tidier—good to keep.tests/components/plugwise/snapshots/test_climate.ambr (1)
1-84: Climate snapshot LGTMThe snapshot captures a full entity-state baseline for
climate.anna; structure and values align with current schema. No concerns.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/components/plugwise/test_sensor.py(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: bouwew
PR: plugwise/plugwise-beta#793
File: custom_components/plugwise/manifest.json:10-11
Timestamp: 2024-12-12T19:48:34.652Z
Learning: In the Plugwise integration, temporary use of alpha versions from test repositories is acceptable for testing purposes.
Learnt from: bouwew
PR: plugwise/plugwise-beta#671
File: custom_components/plugwise/switch.py:82-83
Timestamp: 2024-10-08T15:35:25.180Z
Learning: User bouwew prefers to ignore exception handling suggestions for the `pw-beta` branch in the `custom_components/plugwise/switch.py` file.
Learnt from: bouwew
PR: plugwise/plugwise-beta#671
File: custom_components/plugwise/switch.py:82-83
Timestamp: 2024-06-22T08:05:56.791Z
Learning: User bouwew prefers to ignore exception handling suggestions for the `pw-beta` branch in the `custom_components/plugwise/switch.py` file.
Learnt from: bouwew
PR: plugwise/plugwise-beta#734
File: tests/components/plugwise/test_config_flow.py:58-58
Timestamp: 2024-09-27T09:48:30.890Z
Learning: In `tests/components/plugwise/test_config_flow.py`, the versions are mocked firmware versions for testing purposes and do not need to match the software version in `manifest.json`.
Learnt from: bouwew
PR: plugwise/plugwise-beta#734
File: tests/components/plugwise/test_config_flow.py:58-58
Timestamp: 2024-10-08T15:35:25.180Z
Learning: In `tests/components/plugwise/test_config_flow.py`, the versions are mocked firmware versions for testing purposes and do not need to match the software version in `manifest.json`.
Learnt from: CoMPaTech
PR: plugwise/plugwise-beta#0
File: :0-0
Timestamp: 2025-01-25T20:42:24.862Z
Learning: The linting rules in plugwise-beta have been updated to use TC001/TC002/TC003 instead of TCH001/TCH002/TCH003 for type-checking related rules.
Learnt from: CoMPaTech
PR: plugwise/plugwise-beta#0
File: :0-0
Timestamp: 2025-01-25T20:42:24.862Z
Learning: In the plugwise-beta project, `uv` package manager was removed from core-testing.sh in favor of pip due to issues it was causing.
📚 Learning: in `tests/components/plugwise/test_config_flow.py`, the versions are mocked firmware versions for te...
Learnt from: bouwew
PR: plugwise/plugwise-beta#734
File: tests/components/plugwise/test_config_flow.py:58-58
Timestamp: 2024-09-27T09:48:30.890Z
Learning: In `tests/components/plugwise/test_config_flow.py`, the versions are mocked firmware versions for testing purposes and do not need to match the software version in `manifest.json`.
Applied to files:
tests/components/plugwise/test_sensor.py
🧬 Code Graph Analysis (1)
tests/components/plugwise/test_sensor.py (1)
tests/components/plugwise/conftest.py (5)
mock_smile_adam(151-181)setup_platform(135-147)mock_smile_anna(240-263)mock_smile_p1(267-289)mock_stretch(320-343)
🪛 Ruff (0.12.2)
tests/components/plugwise/test_sensor.py
12-12: homeassistant.helpers.entity_component.async_update_entity imported but unused
Remove unused import: homeassistant.helpers.entity_component.async_update_entity
(F401)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Setup for HA-core (dev)
- GitHub Check: Setup for HA-core (release/master)
- GitHub Check: Prepare and validate pre-commit
🔇 Additional comments (7)
tests/components/plugwise/test_sensor.py (7)
6-6: Import added for snapshot testing.The
SnapshotAssertionimport is correctly added to support the new snapshot-based testing approach.
11-11: Import modified for entity registry usage.The entity registry import is correctly updated to match the new test function signatures that require the
EntityRegistrytype.
14-14: Import added for snapshot testing helper.The
snapshot_platformimport is correctly added to support the new snapshot-based testing pattern.
17-28: Excellent refactoring to snapshot testing.The
test_adam_sensor_entitiesfunction has been successfully converted from explicit state assertions to snapshot-based testing. The function signature correctly includes all required parameters:
snapshot: SnapshotAssertionfor snapshot assertionsentity_registry: er.EntityRegistryfor entity registry accesssetup_platform: MockConfigEntryfrom the conftest fixtureThe use of
@pytest.mark.parametrize("platforms", [(SENSOR_DOMAIN,)])and@pytest.mark.usefixtures("entity_registry_enabled_by_default")decorators is appropriate for platform-specific testing.
84-95: Consistent snapshot testing implementation.The
test_anna_sensor_statesfunction follows the same pattern as other converted test functions, maintaining consistency across the test suite. The parametrization forchosen_env,cooling_present, andplatformsis correctly preserved from the original implementation.
99-110: P1 DSMR sensor test properly converted.The
test_p1_dsmr_sensor_entitiesfunction correctly maintains the original parametrization forchosen_envandgateway_idwhile converting to snapshot testing. This ensures the same test scenarios are covered with the new approach.
148-159: Stretch sensor test successfully converted.The
test_stretch_sensor_entitiesfunction completes the snapshot testing conversion pattern consistently with the other test functions in this file.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/components/plugwise/test_select.py (1)
77-77: Fix typo in docstring.There's a typo: "chaging" should be "changing".
- """Test chaging of Adam regulation_mode select entity. + """Test changing of Adam regulation_mode select entity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
tests/components/plugwise/snapshots/test_select.ambr(1 hunks)tests/components/plugwise/test_select.py(4 hunks)tests/components/plugwise/test_sensor.py(3 hunks)tests/components/plugwise/test_switch.py(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/components/plugwise/snapshots/test_select.ambr
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/components/plugwise/test_switch.py
- tests/components/plugwise/test_sensor.py
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: bouwew
PR: plugwise/plugwise-beta#793
File: custom_components/plugwise/manifest.json:10-11
Timestamp: 2024-12-12T19:48:34.652Z
Learning: In the Plugwise integration, temporary use of alpha versions from test repositories is acceptable for testing purposes.
Learnt from: bouwew
PR: plugwise/plugwise-beta#671
File: custom_components/plugwise/switch.py:82-83
Timestamp: 2024-10-08T15:35:25.180Z
Learning: User bouwew prefers to ignore exception handling suggestions for the `pw-beta` branch in the `custom_components/plugwise/switch.py` file.
Learnt from: bouwew
PR: plugwise/plugwise-beta#671
File: custom_components/plugwise/switch.py:82-83
Timestamp: 2024-06-22T08:05:56.791Z
Learning: User bouwew prefers to ignore exception handling suggestions for the `pw-beta` branch in the `custom_components/plugwise/switch.py` file.
Learnt from: bouwew
PR: plugwise/plugwise-beta#734
File: tests/components/plugwise/test_config_flow.py:58-58
Timestamp: 2024-09-27T09:48:30.890Z
Learning: In `tests/components/plugwise/test_config_flow.py`, the versions are mocked firmware versions for testing purposes and do not need to match the software version in `manifest.json`.
Learnt from: bouwew
PR: plugwise/plugwise-beta#734
File: tests/components/plugwise/test_config_flow.py:58-58
Timestamp: 2024-10-08T15:35:25.180Z
Learning: In `tests/components/plugwise/test_config_flow.py`, the versions are mocked firmware versions for testing purposes and do not need to match the software version in `manifest.json`.
Learnt from: CoMPaTech
PR: plugwise/plugwise-beta#0
File: :0-0
Timestamp: 2025-01-25T20:42:24.862Z
Learning: The linting rules in plugwise-beta have been updated to use TC001/TC002/TC003 instead of TCH001/TCH002/TCH003 for type-checking related rules.
Learnt from: CoMPaTech
PR: plugwise/plugwise-beta#0
File: :0-0
Timestamp: 2025-01-25T20:42:24.862Z
Learning: In the plugwise-beta project, `uv` package manager was removed from core-testing.sh in favor of pip due to issues it was causing.
📚 Learning: in `tests/components/plugwise/test_config_flow.py`, the versions are mocked firmware versions for te...
Learnt from: bouwew
PR: plugwise/plugwise-beta#734
File: tests/components/plugwise/test_config_flow.py:58-58
Timestamp: 2024-10-08T15:35:25.180Z
Learning: In `tests/components/plugwise/test_config_flow.py`, the versions are mocked firmware versions for testing purposes and do not need to match the software version in `manifest.json`.
Applied to files:
tests/components/plugwise/test_select.py
🧬 Code Graph Analysis (1)
tests/components/plugwise/test_select.py (1)
tests/components/plugwise/conftest.py (4)
mock_smile_adam(151-181)setup_platform(135-147)init_integration(347-356)mock_smile_adam_heat_cool(185-208)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Setup for HA-core (dev)
- GitHub Check: Setup for HA-core (release/master)
- GitHub Check: Prepare and validate pre-commit
🔇 Additional comments (4)
tests/components/plugwise/test_select.py (4)
6-6: LGTM! Imports correctly support snapshot testing.The added imports for
SnapshotAssertion,entity_registry, andsnapshot_platformare necessary for the snapshot testing implementation and are properly organized.Also applies to: 16-16, 18-18
21-31: LGTM! Proper snapshot testing implementation.The refactoring correctly implements snapshot testing for Adam select entities:
- Appropriate pytest markers for platform selection and entity registry setup
- Correct fixture usage for snapshot testing components
- Simplified test logic using the
snapshot_platformhelper- Updated docstring reflects the new testing approach
57-70: LGTM! Well-structured additional snapshot test.The new test function properly implements snapshot testing for Adam with cooling capabilities:
- Correct parametrization for the cooling environment
- Appropriate fixture usage matching the established pattern
- Clear docstring indicating the specific test scenario
- Consistent implementation with the refactored test
111-111: LGTM! Improved docstring clarity.The change from "not available" to "non-available" improves the grammatical clarity of the docstring.
| PYTEST_COMMAND="pytest ${debug_params} ${subject} tests/components/${REPO_NAME}/${basedir} --cov=homeassistant/components/${REPO_NAME}/ --cov-report term-missing" | ||
| eval "${PYTEST_COMMAND}" || { | ||
| echo "" | ||
| echo -e "${CFAIL}Pytest / Snapshot validation failed, re-running to update snapshot ...${CNORM}" |
There was a problem hiding this comment.
yeah I skipped those 'CRAI wants handbrakes on everything' :) But good enough - mind you the script is flawed when translations fails (which you'll find when bringing the script to other parts of the universe)
CoMPaTech
left a comment
CoMPaTech
left a comment
There was a problem hiding this comment.
Looks good in general, and indeed we cannot get rid of all functionality tests with snapshots. Probably in some AI future :)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
tests/components/plugwise/test_binary_sensor.py (4)
34-45: Anna snapshot: consider adding a ‘cooling_present = False’ variant for broader coverageNice conversion to snapshot testing. To broaden coverage, consider parameterizing
cooling_presentfor both True/False, or adding a second dataset to capture variations in exposed binary sensors.
19-20: Optional: prefer Platform enum over DOMAIN strings for forward-compatibilityUsing
Platform.BINARY_SENSORkeeps type consistency with HA’sPLATFORMSconventions and avoids string/domain drift.Example minimal changes:
-@pytest.mark.parametrize("platforms", [(BINARY_SENSOR_DOMAIN,)]) +@pytest.mark.parametrize("platforms", [(Platform.BINARY_SENSOR,)])And ensure the import (adjust as needed if you consolidate imports):
-from homeassistant.components.binary_sensor import DOMAIN as BINARY_SENSOR_DOMAIN +from homeassistant.const import PlatformAlso applies to: 34-36, 69-71
81-81: Nit: encode “beta-only” context as a marker instead of a commentIf this is intended to control execution in certain pipelines, consider a pytest marker (e.g.,
@pytest.mark.beta_only) that CI can include/exclude, rather than relying on a comment.
90-90: Docstring grammar nitHyphenate “P1-related” and use “binary sensor” for readability.
-"""Test creation of a persistent notification for the corresponding P1 related binary_sensor.""" +"""Test creation of a persistent notification for the corresponding P1-related binary sensor."""
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/components/plugwise/snapshots/test_binary_sensor.ambr(1 hunks)tests/components/plugwise/test_binary_sensor.py(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/components/plugwise/snapshots/test_binary_sensor.ambr
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/components/plugwise/test_binary_sensor.py (1)
tests/components/plugwise/conftest.py (4)
mock_smile_adam(151-181)setup_platform(135-147)mock_smile_anna(240-263)mock_smile_p1(267-289)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Prepare
🔇 Additional comments (2)
tests/components/plugwise/test_binary_sensor.py (2)
19-29: Adam snapshot test: solid, concise, and uses isolated platform setupGood shift to a single snapshot assertion for the binary_sensor platform. Fixture usage looks correct and minimal.
67-79: P1 snapshot test: LGTMClean snapshot test for the P1 environment with proper platform isolation and registry-enabled fixture.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (14)
tests/components/plugwise/snapshots/test_climate.ambr (5)
1-1: Add an explicit “generated file — do not edit” noticeHelps reduce accidental manual edits to snapshots when reviewing locally.
Apply this header right after the serializer line:
# serializer version: 1 +# NOTE: This file is auto-generated by syrupy/amber during tests. Do not edit manually.
8-12: Normalize list ordering for hvac_modes/preset_modes to avoid churnThe order of hvac_modes and preset_modes may change upstream without functional impact, causing noise in diffs. Consider normalizing these lists before snapshotting.
As a follow-up, add a lightweight normalizer used by the climate tests:
# tests/components/plugwise/_snapshot_normalize.py from enum import Enum from typing import Any, Dict, Iterable def _enum_to_value(x: Any) -> Any: return getattr(x, "value", x) def _sorted_list(values: Iterable[Any]) -> list[Any]: return sorted((_enum_to_value(v) for v in values), key=str) def normalize_climate_payload(obj: Any) -> Any: if isinstance(obj, dict): out: Dict[str, Any] = {k: normalize_climate_payload(v) for k, v in obj.items()} for key in ("hvac_modes", "preset_modes"): if key in out and isinstance(out[key], (list, tuple)): out[key] = _sorted_list(out[key]) return out if isinstance(obj, (list, tuple)): return [normalize_climate_payload(v) for v in obj] return _enum_to_value(obj)Then in your climate test(s), wrap before asserting:
from ._snapshot_normalize import normalize_climate_payload snapshot.assert_match(normalize_climate_payload(state_dict_or_entry))Also applies to: 15-22, 143-147, 151-157, 176-179, 182-189
47-49: Avoid snapshotting numeric supported_features bitmasksValues like <ClimateEntityFeature: 401>, 17, and 18 are bitmasks that can shift when HA adds/renumbers flags, creating brittle snapshots. Prefer asserting named feature flags instead of the integer mask.
Option A (prefer): Transform bitmasks to sorted feature names in tests:
# extend normalize_climate_payload from homeassistant.components.climate.const import ClimateEntityFeature def _feature_names(mask: int) -> list[str]: names = [f.name for f in ClimateEntityFeature if (mask & int(f))] return sorted(names) # inside normalize_climate_payload dict block: if "supported_features" in out and isinstance(out["supported_features"], int): out["supported_features"] = _feature_names(out["supported_features"])Option B: If you don’t care about this field, exclude it with a snapshot matcher.
# conftest.py import re from syrupy.matchers import path_type def snapshot_matchers(): return { "supported_features": path_type(re.compile(r"(^|.*\.)supported_features$")), }Also applies to: 131-133, 214-216, 296-299, 377-379, 458-461, 540-543, 622-624, 705-708, 788-791
58-63: Snapshot enums as their string values to reduce representation riskEntries like <HVACAction.PREHEATING: 'preheating'> and <HVACMode.AUTO: 'auto'> are fine now, but enum repr changes upstream will cause needless snapshot diffs. Store ‘preheating’, ‘auto’, etc.
Use the same normalizer above; it already converts enums via .value when present.
Also applies to: 142-147, 225-229, 307-311, 388-391, 471-473, 551-555, 633-637, 716-720, 799-803
52-85: Consider asserting fewer, higher-signal fields to keep snapshots maintainableThese blocks snapshot a lot of stable metadata (e.g., platform, translation_key, None fields). It’s fine, but pruning low-signal fields can reduce diff noise when HA internals change.
Example: exclude always-None fields and static platform metadata via matchers:
# conftest.py from syrupy.matchers import path_type import re def snapshot_matchers(): ignored = [ r"(^|.*\.)area_id$", r"(^|.*\.)device_class$", r"(^|.*\.)entity_category$", r"(^|.*\.)icon$", r"(^|.*\.)labels$", r"(^|.*\.)name$", r"(^|.*\.)original_device_class$", r"(^|.*\.)original_icon$", r"(^|.*\.)original_name$", r"(^|.*\.)previous_unique_id$", r"(^|.*\.)suggested_object_id$", r"(^|.*\.)unit_of_measurement$", r"(^|.*\.)translation_key$", # if not business-critical r"(^|.*\.)platform$", # if not business-critical ] return { "ignore_misc_none": path_type(re.compile("|".join(ignored))) }Also applies to: 136-169, 219-333, 334-413, 414-495, 496-577, 578-660, 661-743, 744-826
tests/components/plugwise/snapshots/test_select.ambr (2)
8-13: Consider normalizing options ordering to avoid flaky diffs.Lists under capabilities.options and state attributes.options are order-sensitive. If future upstream changes reorder options (even semantically equivalent), snapshots will churn. Normalize by sorting these arrays in the snapshot helper before assertion.
A minimal normalization hook in tests/common (snapshot_platform) could sort any attribute named "options":
# example helper to run inside snapshot_platform before snapshotting def _normalize_options(attrs: dict) -> dict: for k, v in list(attrs.items()): if k == "options" and isinstance(v, list): attrs[k] = sorted(v) elif isinstance(v, dict): attrs[k] = _normalize_options(v) return attrsAlso applies to: 47-52, 130-137, 170-178, 193-200, 234-241
259-261: Double space in 'GF7 Woonkamer' — verify source data.There’s a double space in the option label. If this is intentional (matches device data), keep it. If not, it will cause avoidable churn if later normalized. Please confirm the dataset/string source.
Also applies to: 496-497
tests/components/plugwise/test_select.py (3)
77-80: Docstring over-promises behavior not asserted.The test claims to “also” validate climate previous mode changes, but there’s no assertion covering that side effect. Either add a targeted assertion or trim the docstring.
Suggested docstring-only tweak:
- """Test changing of Adam regulation_mode select entity. - - Also tests a change in climate _previous mode. - """ + """Test changing of Adam regulation_mode select entity."""
85-89: Use constants for payload keys for consistency.Other tests use ATTR_ENTITY_ID/ATTR_OPTION; align this call as well.
- { - "entity_id": "select.adam_regulation_mode", - "option": "heating", - }, + { + ATTR_ENTITY_ID: "select.adam_regulation_mode", + ATTR_OPTION: "heating", + },
111-121: Docstring/entity mismatch: regulation_mode vs thermostat_schedule.Docstring mentions regulation_mode, but the call targets anna_thermostat_schedule. Pick one and make both consistent. Easiest is updating the docstring:
- """Test an Anna regulation_mode non-available option.""" + """Test an Anna thermostat_schedule non-available option."""Alternatively, if the intention was regulation_mode, change the entity id:
- ATTR_ENTITY_ID: "select.anna_thermostat_schedule", + ATTR_ENTITY_ID: "select.anna_regulation_mode",tests/components/plugwise/test_switch.py (1)
79-109: Add a negative test for toggle path to complete coverage.You already cover turn_off/turn_on error propagation. Covering toggle maintains parity with the positive-path tests.
async def test_adam_switch_negative_testing( @@ mock_smile_adam.set_switch_state.assert_called_with( "a28f588dc4a049a483fd03a30361ad3a", None, "relay", STATE_ON ) + +async def test_adam_switch_negative_toggle( + hass: HomeAssistant, mock_smile_adam: MagicMock, init_integration: MockConfigEntry +) -> None: + """Test exceptions for Adam switch toggle.""" + mock_smile_adam.set_switch_state.side_effect = PlugwiseException + with pytest.raises(HomeAssistantError): + await hass.services.async_call( + SWITCH_DOMAIN, + SERVICE_TOGGLE, + {"entity_id": "switch.fibaro_hc2_relay"}, + blocking=True, + ) + mock_smile_adam.set_switch_state.assert_called_with( + "a28f588dc4a049a483fd03a30361ad3a", None, "relay", STATE_OFF + )tests/components/plugwise/test_number.py (1)
54-68: Assert no backend call on validation failure and add lower-bound case.Strengthen the out-of-range test by asserting the mock wasn’t called and add a symmetric below-min case.
async def test_adam_temperature_offset_out_of_bounds_change( @@ await hass.services.async_call( NUMBER_DOMAIN, SERVICE_SET_VALUE, { ATTR_ENTITY_ID: "number.zone_thermostat_jessie_temperature_offset", ATTR_VALUE: 3.0, }, blocking=True, ) + + mock_smile_adam.set_number.assert_not_called() + + +async def test_adam_temperature_offset_below_bounds_change( + hass: HomeAssistant, mock_smile_adam: MagicMock, init_integration: MockConfigEntry +) -> None: + """Test changing the temperature_offset number below limits.""" + with pytest.raises(ServiceValidationError, match="valid range"): + await hass.services.async_call( + NUMBER_DOMAIN, + SERVICE_SET_VALUE, + { + ATTR_ENTITY_ID: "number.zone_thermostat_jessie_temperature_offset", + ATTR_VALUE: -3.0, + }, + blocking=True, + ) + mock_smile_adam.set_number.assert_not_called()tests/components/plugwise/test_sensor.py (2)
29-36: Make this test async for consistency and fixture compatibility.init_integration is an async fixture; making the test async avoids any event loop edge cases and aligns with the rest of the suite.
-def test_adam_4_sensor_entity( +async def test_adam_4_sensor_entity( hass: HomeAssistant, mock_smile_adam_4: MagicMock, init_integration: MockConfigEntry ) -> None: """Test creation of specific Adam sensor entity.""" - state = hass.states.get("sensor.woonkamer_humidity") + # Ensure setup tasks settle + await hass.async_block_till_done() + state = hass.states.get("sensor.woonkamer_humidity")
128-134: Typo in test name: 'entitiy' → 'entity'.Purely cosmetic, but worth correcting for readability.
-async def test_p1_3ph_dsmr_disabled_sensor_entitiy( +async def test_p1_3ph_dsmr_disabled_sensor_entity(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (15)
scripts/core-testing.sh(2 hunks)tests/components/plugwise/conftest.py(2 hunks)tests/components/plugwise/snapshots/test_binary_sensor.ambr(1 hunks)tests/components/plugwise/snapshots/test_button.ambr(1 hunks)tests/components/plugwise/snapshots/test_climate.ambr(1 hunks)tests/components/plugwise/snapshots/test_number.ambr(1 hunks)tests/components/plugwise/snapshots/test_select.ambr(1 hunks)tests/components/plugwise/snapshots/test_switch.ambr(1 hunks)tests/components/plugwise/test_binary_sensor.py(3 hunks)tests/components/plugwise/test_button.py(1 hunks)tests/components/plugwise/test_climate.py(4 hunks)tests/components/plugwise/test_number.py(2 hunks)tests/components/plugwise/test_select.py(3 hunks)tests/components/plugwise/test_sensor.py(3 hunks)tests/components/plugwise/test_switch.py(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- tests/components/plugwise/snapshots/test_button.ambr
- tests/components/plugwise/snapshots/test_binary_sensor.ambr
🚧 Files skipped from review as they are similar to previous changes (7)
- tests/components/plugwise/snapshots/test_switch.ambr
- scripts/core-testing.sh
- tests/components/plugwise/snapshots/test_number.ambr
- tests/components/plugwise/conftest.py
- tests/components/plugwise/test_binary_sensor.py
- tests/components/plugwise/test_button.py
- tests/components/plugwise/test_climate.py
🧰 Additional context used
🧬 Code graph analysis (4)
tests/components/plugwise/test_switch.py (1)
tests/components/plugwise/conftest.py (4)
mock_smile_adam(151-181)setup_platform(135-147)init_integration(347-356)mock_stretch(320-343)
tests/components/plugwise/test_number.py (1)
tests/components/plugwise/conftest.py (4)
mock_smile_adam(151-181)setup_platform(135-147)init_integration(347-356)mock_smile_anna(240-263)
tests/components/plugwise/test_select.py (1)
tests/components/plugwise/conftest.py (4)
mock_smile_adam(151-181)setup_platform(135-147)init_integration(347-356)mock_smile_adam_heat_cool(185-208)
tests/components/plugwise/test_sensor.py (1)
tests/components/plugwise/conftest.py (7)
mock_smile_adam(151-181)setup_platform(135-147)mock_smile_adam_4(212-236)init_integration(347-356)mock_smile_anna(240-263)mock_smile_p1(267-289)mock_stretch(320-343)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Prepare
🔇 Additional comments (17)
tests/components/plugwise/snapshots/test_climate.ambr (6)
2-51: Snapshot structure and use of matchers look solidGood job masking volatile identifiers and timestamps. This should keep the snapshots deterministic across runs and machines.
649-652: Range targets on heat_cool look correctAnna showing target_temp_low/high and no single temperature is consistent with heat_cool behavior. Looks good.
Also applies to: 732-735, 816-818
139-148: Confirmed PREHEATING semanticsThe m_adam_heating fixture explicitly sets
"climate_mode": "heat"and"control_state": "preheating", and the snapshot for living_room ( lines 139–148 and 160–168) reflectsHVACAction.PREHEATINGas intended. No changes needed.
98-99: min_temp variability is expected per-deviceThe
min_tempattribute in climate.py is sourced directly from the device’s reported lower bound, so different hardware can legitimately expose different minimum temperatures:• File – custom_components/plugwise/climate.py
• Line 131 –self._attr_min_temp = self.device[THERMOSTAT][LOWER_BOUND]Snapshots for AMBR (1.0) and other devices (0.0) correctly mirror their respective device specs. No normalization is required unless you intend to enforce a uniform lower bound across all Plugwise devices.
584-587: Confirm support for HVACMode.OFF for the Anna climate deviceThe test snapshots for Anna’s
hvac_modesintentionally list only[HVACMode.AUTO, HVACMode.HEAT_COOL]which matches the current logic path when neither HomeKit emulation nor
REGULATION_MODESdata is present. Please verify against the Anna device specification or manufacturer API whether it truly supports an “off” command via Home Assistant.• If OFF is supported, update the code to append
HVACMode.OFFand adjust these snapshot tests accordingly:
– tests/components/plugwise/snapshots/test_climate.ambr, lines 584–587
– tests/components/plugwise/snapshots/test_climate.ambr, lines 635–637• If OFF is not supported, no changes are needed—the exclusion in the snapshots is correct.
340-343: No action needed: Garage correctly exposes only HEAT mode
The snapshot and tests confirm that climate.garage has no schedules, cooling support, or regulation modes enabled, so only HVACMode.HEAT is returned. This matches the intended behavior and requires no change. (resolve_review_comment)tests/components/plugwise/snapshots/test_select.ambr (2)
1-41: Solid snapshot structure and good use of dynamic placeholders.The serializer header and consistent use of for non-deterministic fields (ids, timestamps, contexts) look correct and will keep snapshots stable across runs.
38-41: Consistent translation_key usage for select entities.translation_key values for gateway_mode, regulation_mode, and select_schedule are consistent and align with platform expectations, which helps reduce brittleness across language changes.
Also applies to: 99-102, 162-165, 225-228, 484-487
tests/components/plugwise/test_select.py (1)
21-32: Good migration to snapshot-based select testing.Targeted platform setup via setup_platform with entity_registry_enabled_by_default keeps scope tight and snapshots deterministic.
tests/components/plugwise/test_switch.py (2)
25-36: Switch snapshot tests look good.Scoping to the SWITCH domain and snapshotting via snapshot_platform is clean and mirrors the pattern used elsewhere in the PR.
125-160: Positive-path change tests are precise and readable.Incremental call_count assertions combined with assert_called_with confirm the exact API surface to the Smile mock without overspecifying state details.
tests/components/plugwise/test_number.py (4)
21-32: Good snapshot migration for number entities.Focused NUMBER platform setup and snapshotting should keep these tests resilient.
34-51: Direct set_number call verification looks correct.Entity id, key, and value match the dataset and expected API surface.
92-105: Anna number snapshot test follows the established pattern.Looks consistent with other domain snapshot tests in this PR.
107-126: Max boiler temp test reads well and asserts the exact backend call.Good choice to assert against a float value to match internal normalization.
tests/components/plugwise/test_sensor.py (2)
16-27: Sensor snapshot tests are clean and scoped.The approach mirrors the other domains and should help reduce brittle per-entity assertions.
96-109: Snapshots for P1 (single and 3-phase) and Stretch look good.Comprehensive coverage that matches the new snapshot test strategy.
Also applies to: 111-124, 150-161
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/components/plugwise/test_sensor.py (1)
29-37: Verify whether this test should use snapshot testing.While most sensor tests in this file have been converted to snapshot-based validation, this test still uses manual state assertions. This creates inconsistency in the test approach.
Should this test also be converted to the snapshot pattern like
test_adam_sensor_snapshot, or is there a specific reason it needs to check the exact humidity value (56.2)?If conversion is appropriate, consider aligning with the snapshot pattern:
+@pytest.mark.parametrize("platforms", [(SENSOR_DOMAIN,)]) +@pytest.mark.usefixtures("entity_registry_enabled_by_default") async def test_adam_climate_sensor_humidity( hass: HomeAssistant, mock_smile_adam_jip: MagicMock, - init_integration: MockConfigEntry, + snapshot: SnapshotAssertion, + entity_registry: er.EntityRegistry, + setup_platform: MockConfigEntry, ) -> None: - """Test creation of climate related humidity sensor entity.""" - state = hass.states.get("sensor.woonkamer_humidity") - assert state - assert float(state.state) == 56.2 + """Test Adam climate sensor humidity snapshot.""" + await snapshot_platform(hass, entity_registry, snapshot, setup_platform.entry_id)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
custom_components/plugwise/coordinator.py(1 hunks)scripts/core-testing.sh(2 hunks)tests/components/plugwise/conftest.py(3 hunks)tests/components/plugwise/snapshots/test_binary_sensor.ambr(1 hunks)tests/components/plugwise/snapshots/test_button.ambr(1 hunks)tests/components/plugwise/snapshots/test_climate.ambr(1 hunks)tests/components/plugwise/snapshots/test_number.ambr(1 hunks)tests/components/plugwise/snapshots/test_select.ambr(1 hunks)tests/components/plugwise/snapshots/test_switch.ambr(1 hunks)tests/components/plugwise/test_binary_sensor.py(2 hunks)tests/components/plugwise/test_button.py(1 hunks)tests/components/plugwise/test_climate.py(10 hunks)tests/components/plugwise/test_config_flow.py(1 hunks)tests/components/plugwise/test_diagnostics.py(1 hunks)tests/components/plugwise/test_init.py(1 hunks)tests/components/plugwise/test_number.py(3 hunks)tests/components/plugwise/test_select.py(4 hunks)tests/components/plugwise/test_sensor.py(4 hunks)tests/components/plugwise/test_switch.py(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- scripts/core-testing.sh
- tests/components/plugwise/snapshots/test_button.ambr
- tests/components/plugwise/snapshots/test_climate.ambr
- tests/components/plugwise/snapshots/test_number.ambr
- tests/components/plugwise/snapshots/test_switch.ambr
- tests/components/plugwise/snapshots/test_select.ambr
- tests/components/plugwise/snapshots/test_binary_sensor.ambr
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: bouwew
PR: plugwise/plugwise-beta#906
File: tests/components/plugwise/test_binary_sensor.py:9-16
Timestamp: 2025-08-23T16:26:34.700Z
Learning: The Plugwise integration test files import utilities like `snapshot_platform`, `MockConfigEntry`, and `async_fire_time_changed` from `tests.common`, which is from the Home Assistant core repository at https://github.com/home-assistant/core/blob/master/tests/common.py, not from local test files.
📚 Learning: 2025-08-23T16:26:34.700Z
Learnt from: bouwew
PR: plugwise/plugwise-beta#906
File: tests/components/plugwise/test_binary_sensor.py:9-16
Timestamp: 2025-08-23T16:26:34.700Z
Learning: The Plugwise integration test files import utilities like `snapshot_platform`, `MockConfigEntry`, and `async_fire_time_changed` from `tests.common`, which is from the Home Assistant core repository at https://github.com/home-assistant/core/blob/master/tests/common.py, not from local test files.
Applied to files:
tests/components/plugwise/test_diagnostics.pytests/components/plugwise/test_binary_sensor.py
📚 Learning: 2024-10-08T15:35:25.180Z
Learnt from: bouwew
PR: plugwise/plugwise-beta#734
File: tests/components/plugwise/test_config_flow.py:58-58
Timestamp: 2024-10-08T15:35:25.180Z
Learning: In `tests/components/plugwise/test_config_flow.py`, the versions are mocked firmware versions for testing purposes and do not need to match the software version in `manifest.json`.
Applied to files:
tests/components/plugwise/test_config_flow.py
🧬 Code graph analysis (7)
tests/components/plugwise/test_select.py (1)
tests/components/plugwise/conftest.py (6)
mock_smile_adam(151-181)setup_platform(135-147)init_integration(347-356)mock_smile_adam_heat_cool(185-208)mock_smile_legacy_anna(293-316)mock_smile_anna(240-263)
tests/components/plugwise/test_climate.py (1)
tests/components/plugwise/conftest.py (6)
mock_smile_adam(151-181)setup_platform(135-147)init_integration(347-356)mock_smile_adam_heat_cool(185-208)mock_smile_adam_jip(212-236)mock_smile_anna(240-263)
tests/components/plugwise/test_number.py (1)
tests/components/plugwise/conftest.py (5)
mock_smile_adam(151-181)setup_platform(135-147)init_integration(347-356)mock_smile_adam_heat_cool(185-208)mock_smile_anna(240-263)
tests/components/plugwise/test_switch.py (1)
tests/components/plugwise/conftest.py (4)
mock_smile_adam(151-181)setup_platform(135-147)init_integration(347-356)mock_stretch(320-343)
tests/components/plugwise/test_button.py (1)
tests/components/plugwise/conftest.py (3)
mock_smile_adam(151-181)setup_platform(135-147)init_integration(347-356)
tests/components/plugwise/test_sensor.py (1)
tests/components/plugwise/conftest.py (7)
mock_smile_adam(151-181)setup_platform(135-147)mock_smile_adam_jip(212-236)init_integration(347-356)mock_smile_anna(240-263)mock_smile_p1(267-289)mock_stretch(320-343)
tests/components/plugwise/test_binary_sensor.py (1)
tests/components/plugwise/conftest.py (4)
mock_smile_adam(151-181)setup_platform(135-147)mock_smile_anna(240-263)mock_smile_p1(267-289)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Prepare
🔇 Additional comments (5)
custom_components/plugwise/coordinator.py (1)
5-5: LGTM!The import reorganization consolidates the
Versionimport at the top of the file with other third-party imports, following PEP 8 conventions. This improves code organization without changing any functionality.tests/components/plugwise/test_button.py (1)
16-27: Snapshot coverage looks solidNice reuse of the shared
setup_platformfixture withsnapshot_platform—this keeps the Adam button snapshot lean and consistent with the other platform tests.tests/components/plugwise/test_sensor.py (3)
6-13: LGTM! Import changes align with snapshot testing.The imports are correctly updated to support the snapshot testing refactor:
SnapshotAssertionfrom syrupy for type hintsentity_registry as erfollows Home Assistant conventionssnapshot_platformutility from tests.commonBased on learnings.
16-27: LGTM! Snapshot testing properly implemented.The test correctly uses the snapshot testing pattern with appropriate fixtures and the
snapshot_platformutility.
83-168: LGTM! Snapshot testing consistently applied.The sensor snapshot tests for Anna, P1 (1-phase and 3-phase), and Stretch are properly converted with:
- Correct parametrization for different environments and gateway IDs
- Consistent fixture usage (
snapshot,entity_registry,setup_platform)- Proper use of
snapshot_platformutilityThe
test_p1_3ph_dsmr_sensor_disabled_entitiestest appropriately remains as a behavior-specific test since it validates entity enabling/disabling logic rather than static state.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
tests/components/plugwise/conftest.py(3 hunks)tests/components/plugwise/test_binary_sensor.py(2 hunks)tests/components/plugwise/test_button.py(1 hunks)tests/components/plugwise/test_climate.py(10 hunks)tests/components/plugwise/test_number.py(2 hunks)tests/components/plugwise/test_select.py(3 hunks)tests/components/plugwise/test_sensor.py(4 hunks)tests/components/plugwise/test_switch.py(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/components/plugwise/test_select.py
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: bouwew
PR: plugwise/plugwise-beta#906
File: tests/components/plugwise/test_binary_sensor.py:9-16
Timestamp: 2025-08-23T16:26:34.700Z
Learning: The Plugwise integration test files import utilities like `snapshot_platform`, `MockConfigEntry`, and `async_fire_time_changed` from `tests.common`, which is from the Home Assistant core repository at https://github.com/home-assistant/core/blob/master/tests/common.py, not from local test files.
📚 Learning: 2025-08-23T16:26:34.700Z
Learnt from: bouwew
PR: plugwise/plugwise-beta#906
File: tests/components/plugwise/test_binary_sensor.py:9-16
Timestamp: 2025-08-23T16:26:34.700Z
Learning: The Plugwise integration test files import utilities like `snapshot_platform`, `MockConfigEntry`, and `async_fire_time_changed` from `tests.common`, which is from the Home Assistant core repository at https://github.com/home-assistant/core/blob/master/tests/common.py, not from local test files.
Applied to files:
tests/components/plugwise/test_binary_sensor.py
🧬 Code graph analysis (6)
tests/components/plugwise/test_button.py (1)
tests/components/plugwise/conftest.py (3)
mock_smile_adam(151-181)setup_platform(135-147)init_integration(347-356)
tests/components/plugwise/test_binary_sensor.py (1)
tests/components/plugwise/conftest.py (4)
mock_smile_adam(151-181)setup_platform(135-147)mock_smile_anna(240-263)mock_smile_p1(267-289)
tests/components/plugwise/test_sensor.py (1)
tests/components/plugwise/conftest.py (7)
mock_smile_adam(151-181)setup_platform(135-147)mock_smile_adam_jip(212-236)init_integration(347-356)mock_smile_anna(240-263)mock_smile_p1(267-289)mock_stretch(320-343)
tests/components/plugwise/test_number.py (1)
tests/components/plugwise/conftest.py (4)
mock_smile_adam(151-181)setup_platform(135-147)mock_smile_adam_heat_cool(185-208)mock_smile_anna(240-263)
tests/components/plugwise/test_switch.py (1)
tests/components/plugwise/conftest.py (4)
mock_smile_adam(151-181)setup_platform(135-147)init_integration(347-356)mock_stretch(320-343)
tests/components/plugwise/test_climate.py (1)
tests/components/plugwise/conftest.py (6)
mock_smile_adam(151-181)setup_platform(135-147)init_integration(347-356)mock_smile_adam_heat_cool(185-208)mock_smile_adam_jip(212-236)mock_smile_anna(240-263)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Setup for HA-core (release/master)
- GitHub Check: Setup for HA-core (dev)
- GitHub Check: Prepare and validate pre-commit
🔇 Additional comments (8)
tests/components/plugwise/test_climate.py (5)
10-31: LGTM! Well-structured imports for snapshot testing.The import additions properly support the snapshot testing framework and promote best practices by using constants instead of string literals throughout the test file.
38-48: Excellent snapshot test implementation.The snapshot tests are well-structured with:
- Consistent naming convention
- Proper parametrization for different device configurations
- Correct use of fixtures and snapshot_platform helper
This approach will make it easier to catch unintended changes to entity states and attributes.
Also applies to: 141-153, 269-281, 355-382
51-124: Good refactoring of service call tests.The updates demonstrate several improvements:
- Using ATTR_* constants instead of string literals reduces the risk of typos
- Proper exception handling with
ServiceValidationError- Consistent mock usage with the renamed
mock_smile_adam_jipThe test logic remains sound while code quality is enhanced.
Also applies to: 215-267, 284-352
126-138: Good addition of negative path testing.This test ensures proper error handling when the Plugwise library raises exceptions. Error path coverage strengthens the test suite's reliability.
156-213: LGTM! Comprehensive testing of dynamic mode transitions.This test effectively validates the climate entity's behavior during heating/cooling mode transitions by:
- Manipulating mock data to simulate real-world state changes
- Using the freezer fixture to trigger coordinator updates
- Verifying both HVAC mode and action attributes
tests/components/plugwise/test_sensor.py (3)
10-26: Consistent snapshot testing implementation.The imports and snapshot test structure align well with the climate tests, demonstrating a unified approach across the test suite. The parametrization and fixture usage follow established patterns.
29-168: Comprehensive snapshot coverage for sensor entities.The sensor tests demonstrate excellent coverage across device types:
- Adam, Anna, P1 (single and 3-phase), and Stretch sensors are all covered with snapshot tests
- Proper parametrization with
gateway_idfor P1 configurations- The unique ID migration test is maintained with mock updates
- The disabled entities test appropriately uses manual assertions to verify enable/disable behavior
The refactoring maintains existing test coverage while adding snapshot validation.
16-168: All snapshot files exist and have valid syrupy headers. The sensor and climate tests are covered by matching.ambrsnapshots.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/components/plugwise/test_binary_sensor.py (1)
47-63: Consider migrating or documenting the rationale for keeping this behavioral test.While the other tests have been migrated to snapshot-based validation,
test_anna_climate_binary_sensor_changestill uses the old style with explicit state checks. If this test is intentionally preserved to verify dynamic state changes over time (which snapshot tests cannot easily capture), consider adding a comment explaining why it wasn't migrated. Otherwise, evaluate whether it can be replaced with a snapshot test or removed if redundant.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/components/plugwise/conftest.py(3 hunks)tests/components/plugwise/test_binary_sensor.py(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: bouwew
PR: plugwise/plugwise-beta#906
File: tests/components/plugwise/test_binary_sensor.py:9-16
Timestamp: 2025-08-23T16:26:34.700Z
Learning: The Plugwise integration test files import utilities like `snapshot_platform`, `MockConfigEntry`, and `async_fire_time_changed` from `tests.common`, which is from the Home Assistant core repository at https://github.com/home-assistant/core/blob/master/tests/common.py, not from local test files.
📚 Learning: 2025-08-23T16:26:34.700Z
Learnt from: bouwew
PR: plugwise/plugwise-beta#906
File: tests/components/plugwise/test_binary_sensor.py:9-16
Timestamp: 2025-08-23T16:26:34.700Z
Learning: The Plugwise integration test files import utilities like `snapshot_platform`, `MockConfigEntry`, and `async_fire_time_changed` from `tests.common`, which is from the Home Assistant core repository at https://github.com/home-assistant/core/blob/master/tests/common.py, not from local test files.
Applied to files:
tests/components/plugwise/test_binary_sensor.py
🧬 Code graph analysis (1)
tests/components/plugwise/test_binary_sensor.py (1)
tests/components/plugwise/conftest.py (4)
mock_smile_adam(151-181)setup_platform(135-147)mock_smile_anna(240-263)mock_smile_p1(267-289)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Setup for HA-core (dev)
- GitHub Check: Setup for HA-core (release/master)
- GitHub Check: Prepare and validate pre-commit
🔇 Additional comments (8)
tests/components/plugwise/conftest.py (3)
4-4: LGTM!The addition of
AsyncGeneratorto the imports is necessary for the newsetup_platformfixture's return type annotation.
128-147: LGTM! Past issue has been addressed.The
platformsfixture now correctly returns the parametrized domain list vialist(request.param), which resolves the critical issue from the previous review where it was returning an empty list. Thesetup_platformfixture properly patcheshomeassistant.components.plugwise.PLATFORMSwith the requested domains and sets up the config entry, enabling platform-scoped snapshot tests.
212-236: LGTM!The rename from
mock_smile_adam_4tomock_smile_adam_jipaligns the fixture name with the test environment (m_adam_jip) and improves clarity. The internal implementation remains functionally unchanged.tests/components/plugwise/test_binary_sensor.py (5)
9-16: LGTM!The imports correctly include snapshot testing utilities (
SnapshotAssertion,snapshot_platform) and entity registry support. Based on learnings,tests.commonimports are from the Home Assistant core repository.
19-29: LGTM!The new snapshot test for Adam binary sensors follows the correct pattern: properly parametrized for the
BINARY_SENSOR_DOMAIN, uses theentity_registry_enabled_by_defaultfixture, and delegates tosnapshot_platformfor validation.
32-44: LGTM!The renamed Anna binary sensor test correctly adopts the snapshot approach with proper parametrization for environment, cooling, and platform domain.
66-80: LGTM!The P1 v4 binary sensor snapshot test properly parametrizes environment, gateway_id, and platform domain, following the established snapshot testing pattern.
84-100: LGTM!The
pw-beta onlytest for P1 v4 notification behavior appropriately remains as a behavioral test since it verifies time-based notification logic, which is not suitable for static snapshot validation.
|
Build on @CoMPaTech's earlier work
Summary by CodeRabbit
Tests
Chores