ronantakizawa/codereview-bench huggingface dataset:
Repository: home-assistant/core
File: tests/components/home_connect/test_number.py (Python)

Code before:
    entity_registry: er.EntityRegistry,
    client: MagicMock,
    config_entry: MockConfigEntry,
    integration_setup: Callable[[MagicMock], Awaitable[bool]],
    appliance: HomeAppliance,
) -> None:
    """Test that option entities are restored if the available program does not include them but they existed once."""
    entity_id = "number.oven_setpoint_temperature"
    client.get_available_program = AsyncMock(
        return_value=ProgramDefinition(
            ProgramKey.UNKNOWN,
            options=[],
        )
    )

    entity_registry.async_get_or_create(
        Platform.NUMBER,
        DOMAIN,
        f"{appliance.ha_id}-{OptionKey.COOKING_OVEN_SETPOINT_TEMPERATURE}",
        suggested_object_id="oven_setpoint_temperature",
    )

    assert await integration_setup(client)
    assert config_entry.state is ConfigEntryState.LOADED

    assert hass.states.is_state(entity_id, STATE_UNAVAILABLE)


Review comment (suggestion):
I think we can also assert that the entity state is not just restored. We don't restore states at the moment just by setting up the integration but it's done when starting Home Assistant. But if we'd change the test setup details, we could get a false positive.
```suggestion
    state = hass.states.get(entity_id)
    assert state is not None
    assert state == STATE_UNAVAILABLE
    assert not state.attributes.get(ATTR_RESTORED)
```

Code after:


@pytest.mark.parametrize("appliance", ["Oven"], indirect=True)
async def test_restore_option_entity(
    hass: HomeAssistant,
    entity_registry: er.EntityRegistry,
    client: MagicMock,
    config_entry: MockConfigEntry,
    integration_setup: Callable[[MagicMock], Awaitable[bool]],
    appliance: HomeAppliance,
) -> None:
    """Test restoration of option entities when program options are missing.

    This test ensures that number entities representing options are restored
    to the entity registry and set to unavailable if the current available
    program does not include them, but they existed previously.
    """
    entity_id = "number.oven_setpoint_temperature"
    client.get_available_program = AsyncMock(
        return_value=ProgramDefinition(
            ProgramKey.UNKNOWN,
            options=[],
        )
    )

    entity_registry.async_get_or_create(
        Platform.NUMBER,
        DOMAIN,
        f"{appliance.ha_id}-{OptionKey.COOKING_OVEN_SETPOINT_TEMPERATURE}",
        suggested_object_id="oven_setpoint_temperature",
    )

    assert await integration_setup(client)
    assert config_entry.state is ConfigEntryState.LOADED

    state = hass.states.get(entity_id)
    assert state is not None
    assert state.state == STATE_UNAVAILABLE
    assert not state.attributes.get(ATTR_RESTORED)
Repository: home-assistant/core
File: homeassistant/components/bsblan/services.py (Python)

Code before:

            # Type cast to BSBLanConfigEntry to access runtime_data
            bsblan_entry: BSBLanConfigEntry = config_entry
            client = bsblan_entry.runtime_data.client

            try:
                # Get current device time
                device_time = await client.time()
                current_time = dt_util.now()
                current_time_str = current_time.strftime("%d.%m.%Y %H:%M:%S")

                # Only sync if device time differs from HA time
                if device_time.time.value != current_time_str:
                    await client.set_time(current_time_str)
            except BSBLANError as err:
                raise HomeAssistantError(
                    f"Failed to sync time for {config_entry.title}: {err}"
                ) from err

    hass.services.async_register(
        DOMAIN,
        "sync_time",
        async_sync_time,
        schema=vol.Schema(
            {
                vol.Optional("config_entry_id"): cv.string,
            }
        ),
    )


Review comment (suggestion):
I'd make this required. Also, you can use `ATTR_CONFIG_ENTRY_ID`

Code after:
_SLOT_SCHEMA = vol.Schema(
    {
        vol.Required("start_time"): cv.time,
        vol.Required("end_time"): cv.time,
    }
)


SERVICE_SET_HOT_WATER_SCHEDULE_SCHEMA = vol.Schema(
    {
        vol.Required(ATTR_DEVICE_ID): cv.string,
        vol.Optional(ATTR_MONDAY_SLOTS): vol.All(cv.ensure_list, [_SLOT_SCHEMA]),
        vol.Optional(ATTR_TUESDAY_SLOTS): vol.All(cv.ensure_list, [_SLOT_SCHEMA]),
        vol.Optional(ATTR_WEDNESDAY_SLOTS): vol.All(cv.ensure_list, [_SLOT_SCHEMA]),
        vol.Optional(ATTR_THURSDAY_SLOTS): vol.All(cv.ensure_list, [_SLOT_SCHEMA]),
        vol.Optional(ATTR_FRIDAY_SLOTS): vol.All(cv.ensure_list, [_SLOT_SCHEMA]),
        vol.Optional(ATTR_SATURDAY_SLOTS): vol.All(cv.ensure_list, [_SLOT_SCHEMA]),
        vol.Optional(ATTR_SUNDAY_SLOTS): vol.All(cv.ensure_list, [_SLOT_SCHEMA]),
    }
)


def _convert_time_slots_to_day_schedule(
    slots: list[dict[str, time]] | None,
) -> DaySchedule | None:
    """Convert list of time slot dicts to a DaySchedule object.

    Example: [{"start_time": time(6, 0), "end_time": time(8, 0)},
              {"start_time": time(17, 0), "end_time": time(21, 0)}]
    becomes: DaySchedule with two TimeSlot objects

    None returns None (don't modify this day).
    Empty list returns DaySchedule with empty slots (clear this day).
    """
    if slots is None:
        return None

    if not slots:
        return DaySchedule(slots=[])

    time_slots = []
    for slot in slots:
        start_time = slot["start_time"]
        end_time = slot["end_time"]

        # Validate that end time is after start time
        if end_time <= start_time:
            raise ServiceValidationError(
                translation_domain=DOMAIN,
                translation_key="end_time_before_start_time",
                translation_placeholders={
Repository: home-assistant/core
File: tests/components/cloudflare/conftest.py (Python)

Code before:


@pytest.fixture
def cfupdate() -> Generator[MagicMock]:
    """Mock the CloudflareUpdater for easier testing."""
    mock_cfupdate = get_mock_client()
    with patch(
        "homeassistant.components.cloudflare.coordinator.pycfdns.Client",
        return_value=mock_cfupdate,
    ) as mock_api:
        yield mock_api


@pytest.fixture
def cfupdate_flow() -> Generator[MagicMock]:
    """Mock the CloudflareUpdater for easier config flow testing."""
    mock_cfupdate = get_mock_client()
    with patch(
        "homeassistant.components.cloudflare.config_flow.pycfdns.Client",
        return_value=mock_cfupdate,
    ) as mock_api:
        yield mock_api


@pytest.fixture
def location_info() -> Generator:
    """Mock the CloudflareUpdater for easier testing."""
    with patch(
        LOCATION_PATCH_TARGET,
        return_value=LocationInfo(
            "0.0.0.0",
            "US",
            "USD",
            "CA",
            "California",
            "San Diego",
            "92122",
            "America/Los_Angeles",
            32.8594,
            -117.2073,
            True,
        ),
    ):
        yield


Review comment (suggestion):
```suggestion
def location_info() -> Generator[None]:
```

Code after:


@pytest.fixture
def cfupdate() -> Generator[MagicMock]:
    """Mock the CloudflareUpdater for easier testing."""
    mock_cfupdate = get_mock_client()
    with patch(
        "homeassistant.components.cloudflare.coordinator.pycfdns.Client",
        return_value=mock_cfupdate,
    ) as mock_api:
        yield mock_api


@pytest.fixture
def cfupdate_flow() -> Generator[MagicMock]:
    """Mock the CloudflareUpdater for easier config flow testing."""
    mock_cfupdate = get_mock_client()
    with patch(
        "homeassistant.components.cloudflare.config_flow.pycfdns.Client",
        return_value=mock_cfupdate,
    ) as mock_api:
        yield mock_api


@pytest.fixture
def location_info() -> Generator[None]:
    """Mock the LocationInfo for easier testing."""
    with patch(
        LOCATION_PATCH_TARGET,
        return_value=LocationInfo(
            "0.0.0.0",
            "US",
            "USD",
            "CA",
            "California",
            "San Diego",
            "92122",
            "America/Los_Angeles",
            32.8594,
            -117.2073,
            True,
        ),
    ):
        yield
Repository: home-assistant/core
File: homeassistant/components/vacuum/trigger.py (Python)

Code before:
"""Provides triggers for vacuum cleaners."""

from homeassistant.core import HomeAssistant
from homeassistant.helpers.trigger import Trigger, make_entity_state_trigger

from .const import DOMAIN, VacuumActivity

TRIGGERS: dict[str, type[Trigger]] = {
    "docked": make_entity_state_trigger(DOMAIN, VacuumActivity.DOCKED),
    "errored": make_entity_state_trigger(DOMAIN, VacuumActivity.ERROR),
    "paused_cleaning": make_entity_state_trigger(DOMAIN, VacuumActivity.PAUSED),
    "returning": make_entity_state_trigger(DOMAIN, VacuumActivity.RETURNING),
    "started_cleaning": make_entity_state_trigger(DOMAIN, VacuumActivity.CLEANING),
}


async def async_get_triggers(hass: HomeAssistant) -> dict[str, type[Trigger]]:
    """Return the triggers for vacuum cleaners."""
    return TRIGGERS


Review comment (suggestion):
I'm not sure about the name. The other triggers are named using past tense. We should preferably follow the same pattern here, eg `"started_returning"`.

Code after:
"""Provides triggers for vacuum cleaners."""

from homeassistant.core import HomeAssistant
from homeassistant.helpers.trigger import Trigger, make_entity_target_state_trigger

from .const import DOMAIN, VacuumActivity

TRIGGERS: dict[str, type[Trigger]] = {
    "docked": make_entity_target_state_trigger(DOMAIN, VacuumActivity.DOCKED),
    "errored": make_entity_target_state_trigger(DOMAIN, VacuumActivity.ERROR),
    "paused_cleaning": make_entity_target_state_trigger(DOMAIN, VacuumActivity.PAUSED),
    "started_cleaning": make_entity_target_state_trigger(
        DOMAIN, VacuumActivity.CLEANING
    ),
    "started_returning": make_entity_target_state_trigger(
        DOMAIN, VacuumActivity.RETURNING
    ),
}


async def async_get_triggers(hass: HomeAssistant) -> dict[str, type[Trigger]]:
    """Return the triggers for vacuum cleaners."""
    return TRIGGERS
Repository: home-assistant/core
File: homeassistant/components/ecowitt/sensor.py (Python)

Code before:
"""Support for Ecowitt Weather Stations."""

from __future__ import annotations

import dataclasses
from datetime import datetime
import re
from typing import Final

from aioecowitt import EcoWittSensor, EcoWittSensorTypes

from homeassistant.components.sensor import (
    SensorDeviceClass,
    SensorEntity,
    SensorEntityDescription,
    SensorStateClass,
)
from homeassistant.const import (
    CONCENTRATION_MICROGRAMS_PER_CUBIC_METER,
    CONCENTRATION_PARTS_PER_MILLION,
    DEGREE,
    LIGHT_LUX,
    PERCENTAGE,
    UV_INDEX,
    EntityCategory,
    UnitOfElectricPotential,
    UnitOfIrradiance,
    UnitOfLength,
    UnitOfPrecipitationDepth,
    UnitOfPressure,
    UnitOfSpeed,
    UnitOfTemperature,

Review comment (suggestion):
Wouldn't be needed with the explicit set.

```suggestion
```

Code after:
"""Support for Ecowitt Weather Stations."""

from __future__ import annotations

import dataclasses
from datetime import datetime
import logging
from typing import Final

from aioecowitt import EcoWittSensor, EcoWittSensorTypes

from homeassistant.components.sensor import (
    SensorDeviceClass,
    SensorEntity,
    SensorEntityDescription,
    SensorStateClass,
)
from homeassistant.const import (
    CONCENTRATION_MICROGRAMS_PER_CUBIC_METER,
    CONCENTRATION_PARTS_PER_MILLION,
    DEGREE,
    LIGHT_LUX,
    PERCENTAGE,
    UV_INDEX,
    EntityCategory,
    UnitOfElectricPotential,
    UnitOfIrradiance,
    UnitOfLength,
    UnitOfPrecipitationDepth,
    UnitOfPressure,
    UnitOfSpeed,
    UnitOfTemperature,
Repository: home-assistant/core
File: tests/components/gentex_homelink/test_config_flow.py (Python)

Code before:
    assert aioclient_mock.mock_calls[0][0] == "POST"


async def test_reauth_flow(
    hass: HomeAssistant, mock_srp_auth: AsyncMock, aioclient_mock: AiohttpClientMocker
) -> None:
    """Test the reauth flow."""
    config_entry = MockConfigEntry(
        domain=DOMAIN,
        unique_id="some-uuid",
        version=1,
        data={
            "auth_implementation": "gentex_homelink",
            "token": {
                "expires_at": time.time() + 10000,
                "access_token": "",
                "refresh_token": "",
            },
            "last_update_id": None,
        },
        state=ConfigEntryState.LOADED,
    )
    config_entry.add_to_hass(hass)
    result = await config_entry.start_reauth_flow(hass)
    assert result["step_id"] == "reauth_confirm"
    assert result["type"] == FlowResultType.FORM
    result = await hass.config_entries.flow.async_configure(
        result["flow_id"],
        user_input=TEST_CREDENTIALS,
    )
    assert result["reason"] == "reauth_successful"


Review comment (suggestion):
```suggestion
    assert result["type"] is FlowResultType.FORM
```

Code after:
    result = await hass.config_entries.flow.async_configure(
        result["flow_id"],
        user_input={"email": "test@test.com", "password": "SomePassword"},
    )
    assert len(aioclient_mock.mock_calls) == 1
    assert aioclient_mock.mock_calls[0][0] == "POST"
    assert result["type"] is FlowResultType.CREATE_ENTRY


async def test_reauth_successful(
    hass: HomeAssistant,
    mock_srp_auth: AsyncMock,
    aioclient_mock: AiohttpClientMocker,
    mock_config_entry: MockConfigEntry,
) -> None:
    """Test the reauth flow."""
    await setup_integration(hass, mock_config_entry)
    result = await mock_config_entry.start_reauth_flow(hass)
    assert result["step_id"] == "reauth_confirm"
    assert result["type"] is FlowResultType.FORM
    result = await hass.config_entries.flow.async_configure(
        result["flow_id"],
        user_input=TEST_CREDENTIALS,
    )
    assert result["reason"] == "reauth_successful"
    assert result["type"] is FlowResultType.ABORT


@pytest.mark.parametrize("mock_srp_access_token", [INVALID_TEST_ACCESS_JWT])
async def test_reauth_error(
    hass: HomeAssistant,
    mock_srp_auth: AsyncMock,
    aioclient_mock: AiohttpClientMocker,
    mock_config_entry: MockConfigEntry,
) -> None:
    """Test the reauth flow."""
    await setup_integration(hass, mock_config_entry)
    result = await mock_config_entry.start_reauth_flow(hass)
    assert result["step_id"] == "reauth_confirm"
    assert result["type"] is FlowResultType.FORM
    result = await hass.config_entries.flow.async_configure(
        result["flow_id"],
        user_input=INVALID_TEST_CREDENTIALS,
    )
    assert result["reason"] == "unique_id_mismatch"
    assert result["type"] is FlowResultType.ABORT
Repository: home-assistant/core
File: homeassistant/components/unifiprotect/const.py (Python)

Code before:
"""Constant definitions for UniFi Protect Integration."""

from typing import Final

from uiprotect.data import ModelType, Version

from homeassistant.const import Platform
from homeassistant.util.hass_dict import HassKey

DOMAIN = "unifiprotect"

# Typed key for hass.data access - stores auth retries per config entry
DATA_AUTH_RETRIES: HassKey[dict[str, int]] = HassKey(f"{DOMAIN}_auth_retries")
# Typed key for discovery state
DATA_DISCOVERY_STARTED: HassKey[bool] = HassKey(f"{DOMAIN}_discovery")
# If rate limit for 4.x or later a 429 is returned
# so we can use a lower value
AUTH_RETRIES = 2

ATTR_EVENT_SCORE = "event_score"
ATTR_EVENT_ID = "event_id"
ATTR_WIDTH = "width"
ATTR_HEIGHT = "height"
ATTR_FPS = "fps"
ATTR_BITRATE = "bitrate"
ATTR_CHANNEL_ID = "channel_id"
ATTR_MESSAGE = "message"
ATTR_DURATION = "duration"
ATTR_ANONYMIZE = "anonymize"

CONF_DISABLE_RTSP = "disable_rtsp"
CONF_ALL_UPDATES = "all_updates"
CONF_OVERRIDE_CHOST = "override_connection_host"
CONF_MAX_MEDIA = "max_media"
CONF_ALLOW_EA = "allow_ea_channel"

CONFIG_OPTIONS = [
    CONF_ALL_UPDATES,
    CONF_DISABLE_RTSP,
    CONF_OVERRIDE_CHOST,

Review comment (security):
Would it not be possible to have a single key, initiated inside `async_setup` (for example with auth_retries as an empty dict, and discovery as False)?


Code after:
"""Constant definitions for UniFi Protect Integration."""

from typing import Final

from uiprotect.data import ModelType, Version

from homeassistant.const import Platform

DOMAIN = "unifiprotect"
# If rate limit for 4.x or later a 429 is returned
# so we can use a lower value
AUTH_RETRIES = 2

ATTR_EVENT_SCORE = "event_score"
ATTR_EVENT_ID = "event_id"
ATTR_WIDTH = "width"
ATTR_HEIGHT = "height"
ATTR_FPS = "fps"
ATTR_BITRATE = "bitrate"
ATTR_CHANNEL_ID = "channel_id"
ATTR_MESSAGE = "message"
ATTR_DURATION = "duration"
ATTR_ANONYMIZE = "anonymize"

CONF_DISABLE_RTSP = "disable_rtsp"
CONF_ALL_UPDATES = "all_updates"
CONF_OVERRIDE_CHOST = "override_connection_host"
CONF_MAX_MEDIA = "max_media"
CONF_ALLOW_EA = "allow_ea_channel"

CONFIG_OPTIONS = [
    CONF_ALL_UPDATES,
    CONF_DISABLE_RTSP,
    CONF_OVERRIDE_CHOST,
]

DEFAULT_PORT = 443
DEFAULT_ATTRIBUTION = "Powered by UniFi Protect Server"
DEFAULT_BRAND = "Ubiquiti"
DEFAULT_VERIFY_SSL = False
Repository: home-assistant/core
File: homeassistant/components/openevse/__init__.py (Python)

Code before:
"""The OpenEVSE integration."""

from __future__ import annotations

import openevsewifi

from homeassistant.config_entries import ConfigEntry
from homeassistant.const import CONF_HOST, Platform
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import ConfigEntryError


async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
    """Set up openevse from a config entry."""

    entry.runtime_data = openevsewifi.Charger(entry.data[CONF_HOST])
    try:
        result = await hass.async_add_executor_job(entry.runtime_data.getStatus)
        assert result is not None
    except AttributeError as ex:
        raise ConfigEntryError("Unable to connect to charger") from ex

    await hass.config_entries.async_forward_entry_setups(entry, [Platform.SENSOR])
    return True


async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
    """Unload a config entry."""
    return await hass.config_entries.async_unload_platforms(entry, [Platform.SENSOR])


Review comment (suggestion):
Extend the ConfigEntry type for OpenEVSE and add `Charger` in the typeslot to make our tools be able to understand what's in `entry.runtime_data`. https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/runtime-data

Code after:
"""The OpenEVSE integration."""

from __future__ import annotations

import openevsewifi

from homeassistant.config_entries import ConfigEntry
from homeassistant.const import CONF_HOST, Platform
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import ConfigEntryError

type OpenEVSEConfigEntry = ConfigEntry[openevsewifi.Charger]


async def async_setup_entry(hass: HomeAssistant, entry: OpenEVSEConfigEntry) -> bool:
    """Set up openevse from a config entry."""

    entry.runtime_data = openevsewifi.Charger(entry.data[CONF_HOST])
    try:
        await hass.async_add_executor_job(entry.runtime_data.getStatus)
    except AttributeError as ex:
        raise ConfigEntryError("Unable to connect to charger") from ex

    await hass.config_entries.async_forward_entry_setups(entry, [Platform.SENSOR])
    return True


async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
    """Unload a config entry."""
    return await hass.config_entries.async_unload_platforms(entry, [Platform.SENSOR])
Repository: home-assistant/core
File: homeassistant/components/openevse/config_flow.py (Python)

Code before:
"""Config flow for OpenEVSE integration."""

import openevsewifi
import voluptuous as vol

from homeassistant import config_entries
from homeassistant.const import CONF_HOST

from .const import DOMAIN


class OpenEVSEConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
    """OpenEVSE config flow."""

    VERSION = 1
    MINOR_VERSION = 1

    async def check_status(self, host: str) -> bool:
        """Check if we can connect to the OpenEVSE charger."""

        try:
            charger = openevsewifi.Charger(host)
            result = await self.hass.async_add_executor_job(charger.getStatus)
        except AttributeError:
            return False
        else:
            return result is not None

    async def async_step_user(self, user_input=None) -> config_entries.ConfigFlowResult:
        """Handle the initial step."""

        if user_input is not None:
            await self.async_set_unique_id(user_input[CONF_HOST])
            self._abort_if_unique_id_configured()

            if not await self.check_status(user_input[CONF_HOST]):
                return self.async_show_form(

Review comment (suggestion):
```suggestion
class OpenEVSEConfigFlow(ConfigFlow, domain=DOMAIN):
```

Code after:
"""Config flow for OpenEVSE integration."""

from typing import Any

import openevsewifi
import voluptuous as vol

from homeassistant.config_entries import ConfigFlow, ConfigFlowResult
from homeassistant.const import CONF_HOST

from .const import DOMAIN


class OpenEVSEConfigFlow(ConfigFlow, domain=DOMAIN):
    """OpenEVSE config flow."""

    VERSION = 1
    MINOR_VERSION = 1

    async def check_status(self, host: str) -> bool:
        """Check if we can connect to the OpenEVSE charger."""

        charger = openevsewifi.Charger(host)
        try:
            result = await self.hass.async_add_executor_job(charger.getStatus)
        except AttributeError:
            return False
        else:
            return result is not None

    async def async_step_user(
        self, user_input: dict[str, Any] | None = None
    ) -> ConfigFlowResult:
        """Handle the initial step."""

        errors = None
        if user_input is not None: