From 679b7f403283ec7c374bbfa1a58d5117d5a19721 Mon Sep 17 00:00:00 2001 From: Arie Catsman <120491684+catsmanac@users.noreply.github.com> Date: Mon, 27 Jan 2025 16:40:39 +0100 Subject: [PATCH] Fix test logic flaw in enphase_envoy test_select (#136570) * Fix test logic flaw in enphase_envoy test_select * Replace test loops by test parameters * Implement review feedback to Improve use of parametrize parameters --- tests/components/enphase_envoy/test_select.py | 179 ++++++++---------- 1 file changed, 74 insertions(+), 105 deletions(-) diff --git a/tests/components/enphase_envoy/test_select.py b/tests/components/enphase_envoy/test_select.py index 9b3a63d1e23..e13492c7f54 100644 --- a/tests/components/enphase_envoy/test_select.py +++ b/tests/components/enphase_envoy/test_select.py @@ -7,15 +7,12 @@ from syrupy.assertion import SnapshotAssertion from homeassistant.components.enphase_envoy.const import Platform from homeassistant.components.enphase_envoy.select import ( - ACTION_OPTIONS, - MODE_OPTIONS, RELAY_ACTION_MAP, RELAY_MODE_MAP, REVERSE_RELAY_ACTION_MAP, REVERSE_RELAY_MODE_MAP, REVERSE_STORAGE_MODE_MAP, STORAGE_MODE_MAP, - STORAGE_MODE_OPTIONS, ) from homeassistant.components.select import ATTR_OPTION, DOMAIN as SELECT_DOMAIN from homeassistant.const import ATTR_ENTITY_ID, SERVICE_SELECT_OPTION @@ -28,9 +25,9 @@ from tests.common import MockConfigEntry, snapshot_platform @pytest.mark.parametrize( - ("mock_envoy"), + "mock_envoy", ["envoy_metered_batt_relay", "envoy_eu_batt"], - indirect=["mock_envoy"], + indirect=True, ) @pytest.mark.usefixtures("entity_registry_enabled_by_default") async def test_select( @@ -47,14 +44,14 @@ async def test_select( @pytest.mark.parametrize( - ("mock_envoy"), + "mock_envoy", [ "envoy", "envoy_1p_metered", "envoy_nobatt_metered_3p", "envoy_tot_cons_metered", ], - indirect=["mock_envoy"], + indirect=True, ) async def test_no_select( hass: HomeAssistant, @@ -68,13 +65,31 @@ async def test_no_select( assert not er.async_entries_for_config_entry(entity_registry, config_entry.entry_id) +@pytest.mark.parametrize("mock_envoy", ["envoy_metered_batt_relay"], indirect=True) @pytest.mark.parametrize( - ("mock_envoy"), ["envoy_metered_batt_relay"], indirect=["mock_envoy"] + ("relay", "target", "expected_state", "call_parameter"), + [ + ("NC1", "generator_action", "shed", "generator_action"), + ("NC1", "microgrid_action", "shed", "micro_grid_action"), + ("NC1", "grid_action", "shed", "grid_action"), + ("NC2", "generator_action", "shed", "generator_action"), + ("NC2", "microgrid_action", "shed", "micro_grid_action"), + ("NC2", "grid_action", "apply", "grid_action"), + ("NC3", "generator_action", "apply", "generator_action"), + ("NC3", "microgrid_action", "apply", "micro_grid_action"), + ("NC3", "grid_action", "shed", "grid_action"), + ], ) +@pytest.mark.parametrize("action", ["powered", "not_powered", "schedule", "none"]) async def test_select_relay_actions( hass: HomeAssistant, mock_envoy: AsyncMock, config_entry: MockConfigEntry, + target: str, + expected_state: str, + call_parameter: str, + relay: str, + action: str, ) -> None: """Test select platform entities dry contact relay actions.""" with patch("homeassistant.components.enphase_envoy.PLATFORMS", [Platform.SELECT]): @@ -82,54 +97,37 @@ async def test_select_relay_actions( entity_base = f"{Platform.SELECT}." - for contact_id, dry_contact in mock_envoy.data.dry_contact_settings.items(): - name = dry_contact.load_name.lower().replace(" ", "_") - for target in ( - ("generator_action", dry_contact.generator_action, "generator_action"), - ("microgrid_action", dry_contact.micro_grid_action, "micro_grid_action"), - ("grid_action", dry_contact.grid_action, "grid_action"), - ): - test_entity = f"{entity_base}{name}_{target[0]}" - assert (entity_state := hass.states.get(test_entity)) - assert RELAY_ACTION_MAP[target[1]] == (current_state := entity_state.state) - # set all relay modes except current mode - for action in [action for action in ACTION_OPTIONS if not current_state]: - await hass.services.async_call( - SELECT_DOMAIN, - SERVICE_SELECT_OPTION, - { - ATTR_ENTITY_ID: test_entity, - ATTR_OPTION: action, - }, - blocking=True, - ) - mock_envoy.update_dry_contact.assert_called_once_with( - {"id": contact_id, target[2]: REVERSE_RELAY_ACTION_MAP[action]} - ) - mock_envoy.update_dry_contact.reset_mock() - # and finally back to original - await hass.services.async_call( - SELECT_DOMAIN, - SERVICE_SELECT_OPTION, - { - ATTR_ENTITY_ID: test_entity, - ATTR_OPTION: current_state, - }, - blocking=True, - ) - mock_envoy.update_dry_contact.assert_called_once_with( - {"id": contact_id, target[2]: REVERSE_RELAY_ACTION_MAP[current_state]} - ) - mock_envoy.update_dry_contact.reset_mock() + assert (dry_contact := mock_envoy.data.dry_contact_settings[relay]) + assert (name := dry_contact.load_name.lower().replace(" ", "_")) + + test_entity = f"{entity_base}{name}_{target}" + + assert (entity_state := hass.states.get(test_entity)) + assert entity_state.state == RELAY_ACTION_MAP[expected_state] + + await hass.services.async_call( + SELECT_DOMAIN, + SERVICE_SELECT_OPTION, + { + ATTR_ENTITY_ID: test_entity, + ATTR_OPTION: action, + }, + blocking=True, + ) + mock_envoy.update_dry_contact.assert_called_once_with( + {"id": relay, call_parameter: REVERSE_RELAY_ACTION_MAP[action]} + ) -@pytest.mark.parametrize( - ("mock_envoy"), ["envoy_metered_batt_relay"], indirect=["mock_envoy"] -) +@pytest.mark.parametrize("mock_envoy", ["envoy_metered_batt_relay"], indirect=True) +@pytest.mark.parametrize("relay_mode", ["battery", "standard"]) +@pytest.mark.parametrize("relay", ["NC1", "NC2", "NC3"]) async def test_select_relay_modes( hass: HomeAssistant, mock_envoy: AsyncMock, config_entry: MockConfigEntry, + relay_mode: str, + relay: str, ) -> None: """Test select platform dry contact relay mode changes.""" with patch("homeassistant.components.enphase_envoy.PLATFORMS", [Platform.SELECT]): @@ -137,40 +135,26 @@ async def test_select_relay_modes( entity_base = f"{Platform.SELECT}." - for contact_id, dry_contact in mock_envoy.data.dry_contact_settings.items(): - name = dry_contact.load_name.lower().replace(" ", "_") - test_entity = f"{entity_base}{name}_mode" - assert (entity_state := hass.states.get(test_entity)) - assert RELAY_MODE_MAP[dry_contact.mode] == (current_state := entity_state.state) - for mode in [mode for mode in MODE_OPTIONS if not current_state]: - await hass.services.async_call( - SELECT_DOMAIN, - SERVICE_SELECT_OPTION, - { - ATTR_ENTITY_ID: test_entity, - ATTR_OPTION: mode, - }, - blocking=True, - ) - mock_envoy.update_dry_contact.assert_called_once_with( - {"id": contact_id, "mode": REVERSE_RELAY_MODE_MAP[mode]} - ) - mock_envoy.update_dry_contact.reset_mock() + assert (dry_contact := mock_envoy.data.dry_contact_settings[relay]) + assert (name := dry_contact.load_name.lower().replace(" ", "_")) - # and finally current mode again - await hass.services.async_call( - SELECT_DOMAIN, - SERVICE_SELECT_OPTION, - { - ATTR_ENTITY_ID: test_entity, - ATTR_OPTION: current_state, - }, - blocking=True, - ) - mock_envoy.update_dry_contact.assert_called_once_with( - {"id": contact_id, "mode": REVERSE_RELAY_MODE_MAP[current_state]} - ) - mock_envoy.update_dry_contact.reset_mock() + test_entity = f"{entity_base}{name}_mode" + + assert (entity_state := hass.states.get(test_entity)) + assert entity_state.state == RELAY_MODE_MAP[dry_contact.mode] + + await hass.services.async_call( + SELECT_DOMAIN, + SERVICE_SELECT_OPTION, + { + ATTR_ENTITY_ID: test_entity, + ATTR_OPTION: relay_mode, + }, + blocking=True, + ) + mock_envoy.update_dry_contact.assert_called_once_with( + {"id": relay, "mode": REVERSE_RELAY_MODE_MAP[relay_mode]} + ) @pytest.mark.parametrize( @@ -181,11 +165,13 @@ async def test_select_relay_modes( ], indirect=["mock_envoy"], ) +@pytest.mark.parametrize(("mode"), ["backup", "self_consumption", "savings"]) async def test_select_storage_modes( hass: HomeAssistant, mock_envoy: AsyncMock, config_entry: MockConfigEntry, use_serial: str, + mode: str, ) -> None: """Test select platform entities storage mode changes.""" with patch("homeassistant.components.enphase_envoy.PLATFORMS", [Platform.SELECT]): @@ -194,38 +180,21 @@ async def test_select_storage_modes( test_entity = f"{Platform.SELECT}.{use_serial}_storage_mode" assert (entity_state := hass.states.get(test_entity)) - assert STORAGE_MODE_MAP[mock_envoy.data.tariff.storage_settings.mode] == ( - current_state := entity_state.state + assert ( + entity_state.state + == STORAGE_MODE_MAP[mock_envoy.data.tariff.storage_settings.mode] ) - for mode in [mode for mode in STORAGE_MODE_OPTIONS if not current_state]: - await hass.services.async_call( - SELECT_DOMAIN, - SERVICE_SELECT_OPTION, - { - ATTR_ENTITY_ID: test_entity, - ATTR_OPTION: mode, - }, - blocking=True, - ) - mock_envoy.set_storage_mode.assert_called_once_with( - REVERSE_STORAGE_MODE_MAP[mode] - ) - mock_envoy.set_storage_mode.reset_mock() - - # and finally with original mode await hass.services.async_call( SELECT_DOMAIN, SERVICE_SELECT_OPTION, { ATTR_ENTITY_ID: test_entity, - ATTR_OPTION: current_state, + ATTR_OPTION: mode, }, blocking=True, ) - mock_envoy.set_storage_mode.assert_called_once_with( - REVERSE_STORAGE_MODE_MAP[current_state] - ) + mock_envoy.set_storage_mode.assert_called_once_with(REVERSE_STORAGE_MODE_MAP[mode]) @pytest.mark.parametrize(