Add platform tests to enphase_envoy #114390
Conversation
catsmanac
requested review from
bdraco,
cgarwood,
dgomes and
joostlek
as code owners
home-assistant
bot
added
cla-signed
code-quality
has-tests
integration: enphase_envoy
small-pr
|
Hey there @bdraco, @cgarwood, @dgomes, @joostlek, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
| @pytest.fixture(name="setup_enphase_envoy_binary_sensor") | ||
| async def setup_enphase_envoy_binary_sensor_fixture( | ||
| hass: HomeAssistant, config: ConfigType, mock_envoy: AsyncMock | ||
| ): | ||
| """Define a fixture to set up Enphase Envoy with binary sensor platform only.""" | ||
| with ( | ||
| patch( | ||
| "homeassistant.components.enphase_envoy.Envoy", | ||
| return_value=mock_envoy, | ||
| ), | ||
| patch( | ||
| "homeassistant.components.enphase_envoy.PLATFORMS", | ||
| [Platform.BINARY_SENSOR], | ||
| ), | ||
| ): | ||
| assert await async_setup_component(hass, DOMAIN, config) | ||
| await hass.async_block_till_done() | ||
| yield |
There was a problem hiding this comment.
I think it would make sense to make the Envoy mock a fixture (I mean it is already, but a fixture that patches the right packages automatically) and to make this whole a function.
A good example to look at is flexit_bacnet. They also use snapshot tests, and they have a nice way to setup the integration for specific platforms.
Also, we should not use the config: ConfigType here, since this isn't a YAML integration anymore, but flexit_bacnet is a good example to look from
There was a problem hiding this comment.
Thanks for the review and suggestion. Used the flexit_bacnet example to refactor this one.
| # number entities states should be created from test data | ||
| assert len(hass.states.async_all()) == 4 | ||
|
|
||
| entity_registry = er.async_get(hass) |
There was a problem hiding this comment.
This can be loaded in as a test fixture
| entity_entries = er.async_entries_for_config_entry( | ||
| entity_registry, config_entry.entry_id | ||
| ) | ||
| assert entity_entries | ||
| assert entity_entries == snapshot |
There was a problem hiding this comment.
I think it's good to have one snapshot entry per entity. With this you avoid a large diff when updating an entity, since this is an (unordered) list, adding an entity can make the list move all over the place, making the diff useless. The snapshot test in flexit_bacnet and analytics_insight is a good example.
There was a problem hiding this comment.
Like that concept, the large ones don't really help finding what changes. Implemented.
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
|
It looks like there is a conflict now |
|
Yes, expected that with the other pr disabling the phase entities. I'll resolve that. |
|
Conflicts resolved, tests updated for disabled phase data and added/modified tests using |
| if entity_count == 0: | ||
| assert len(entity_entries) == 0 | ||
| else: | ||
| # compare registered entities against snapshot of prior run | ||
| assert entity_entries | ||
| for entity_entry in entity_entries: | ||
| assert entity_entry == snapshot(name=f"{entity_entry.entity_id}-entry") | ||
| assert hass.states.get(entity_entry.entity_id) == snapshot( | ||
| name=f"{entity_entry.entity_id}-state" | ||
| ) |
There was a problem hiding this comment.
We try to avoid using if statements in tests as much as possible as it makes it much harder to read and see what we are actually testing
There was a problem hiding this comment.
Removed where possible.
Proposed change
Currently enphase_envoy integration has only tests for configflow and sensor platform. This pr adds tests for the used platforms which are missing tests.
The test data model is extended with Encharge/Enpower battery data and new tests are added for:
The existing sensor platform test is updated for the new battery sensors.
All snapshot are updated or newly provided.
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.To help with the load of incoming pull requests: