fix(config_flow): clearing an entity field no longer fails validation (t47 C)
t46 F1, reproduced against real Home Assistant: submitting the options page with a
cleared entity field (script_turn_on='') produced "Schema validation failed at
script_turn_on". HA validates the submitted page against the schema the flow last
returned, and real EntitySelector rejects BOTH '' and None — "Entity is neither a
valid entity ID nor a valid UUID" (verified directly against homeassistant's
EntitySelector). Clearing therefore depended on the frontend OMITTING the key, on
every entity field of the page, not only the script ones t44 added.
_ClearingEntitySelector subclasses EntitySelector and returns '' for None or an
empty/whitespace string, delegating everything else to the parent — so a real entity
id is still domain-checked, and the class is still an EntitySelector: its serialized
form is byte-identical to the real selector's
({"selector": {"entity": {"domain": ["script"], "multiple": false, "reorder": false}}}),
so the frontend keeps rendering an entity picker.
The step also normalizes a submitted None to '' before it acts: an explicit empty
value clears the setting (the script dict drops it, the patched options store '' which
every reader already treats as unset), while an OMITTED key still means "the page did
not touch it" for the patched options — and still means "cleared" for the script dict,
which is rebuilt from the page (t44's editor-is-the-truth rule, unchanged).
The test harness now models the trap instead of hiding it: EntitySelector is a CLASS
(entity_stublib.EntitySelectorStub) that rejects ''/None like real HA, defined once and
shared by all five stub installers through install_selector_faces(), so the stubs
cannot drift apart.
Tests (tests/test_config_flow_ux.py::TestClearingAnEntityField, 8): the schema accepts
omitted/None/'' while passing a real entity id through; the plain selector is pinned as
rejecting the same values (the trap itself); '' and None each clear a script AND a
pre-existing entity field (jellyfin, the older instance of the pattern) while
untouched fields survive; an omitted script field is cleared; an omitted entity field
is kept; and a prefill→submit round trip changes nothing.
RED PROOF: with the five selector sites switched back to the plain EntitySelector,
exactly the five tolerance tests fail (5 failed, 4 passed) and the rest of the file
stays green; restored, 70 pass. Real-HA probe run separately (see the task output):
strict rejects '' and None, the subclass accepts both and returns '', and serialization
is unchanged.
Verified: 423 passed / 2 skipped, ruff clean, drift engine exit 0 under real HA.
This commit is contained in:
1 parent
0cfa045190
commit
b3f897c36e
6 files changed
+233
-34
No files matched your search
+166
-12
@@ -37,6 +37,7 @@ from nsh.const import (
|
||||
CONF_HOST,
|
||||
CONF_JELLYFIN_ENTITY,
|
||||
CONF_PAIRING_CODE,
|
||||
CONF_POWER_ENTITY,
|
||||
CONF_PROTOCOL_ENABLED,
|
||||
CONF_SKIP_PAIRING,
|
||||
DEFAULT_ADB_PORT,
|
||||
@@ -227,11 +228,7 @@ def _install_stubs() -> None:
|
||||
sys.modules["homeassistant.helpers.device_registry"] = device_registry
|
||||
|
||||
selector = types.ModuleType("homeassistant.helpers.selector")
|
||||
selector.EntitySelector = lambda *a, **k: None
|
||||
selector.EntitySelectorConfig = lambda *a, **k: ""
|
||||
selector.SelectSelector = lambda *a, **k: None
|
||||
selector.SelectSelectorConfig = lambda **k: ""
|
||||
selector.SelectSelectorMode = types.SimpleNamespace(DROPDOWN="dropdown")
|
||||
entity_stublib.install_selector_faces(selector)
|
||||
helpers.selector = selector
|
||||
sys.modules["homeassistant.helpers.selector"] = selector
|
||||
sys.modules["homeassistant.helpers"] = helpers
|
||||
@@ -1451,6 +1448,17 @@ class TestCopyContract:
|
||||
assert field in step["data"], f"options.{field} has no label"
|
||||
|
||||
|
||||
def _options_flow(tmp_path, options=None):
|
||||
"""An options flow over an entry with `options`, on a fake hass."""
|
||||
module = _form_module()
|
||||
flow = module.NvidiaShieldOptionsFlow()
|
||||
flow.config_entry = SimpleNamespace(
|
||||
options=dict(options or {}), entry_id="e1", title="Shield"
|
||||
)
|
||||
flow.hass = FakeHass(str(tmp_path))
|
||||
return flow
|
||||
|
||||
|
||||
class TestCommandScriptOptions:
|
||||
"""t44: one script selector per command, stored as one dict option.
|
||||
|
||||
@@ -1460,13 +1468,7 @@ class TestCommandScriptOptions:
|
||||
"""
|
||||
|
||||
def _flow(self, tmp_path, options=None):
|
||||
module = _form_module()
|
||||
flow = module.NvidiaShieldOptionsFlow()
|
||||
flow.config_entry = SimpleNamespace(
|
||||
options=dict(options or {}), entry_id="e1", title="Shield"
|
||||
)
|
||||
flow.hass = FakeHass(str(tmp_path))
|
||||
return flow
|
||||
return _options_flow(tmp_path, options)
|
||||
|
||||
@staticmethod
|
||||
def _submitted(schema):
|
||||
@@ -1542,6 +1544,158 @@ class TestCommandScriptOptions:
|
||||
)
|
||||
|
||||
|
||||
class TestClearingAnEntityField:
|
||||
"""t47 F1: clearing must work whichever shape the field arrives in.
|
||||
|
||||
Home Assistant validates the submitted page against the schema the flow last
|
||||
returned, and real HA's EntitySelector rejects BOTH ``''`` and ``None``
|
||||
("Entity is neither a valid entity ID nor a valid UUID") — so an explicit clear
|
||||
was reported as a malformed form. The three shapes are pinned here, validated
|
||||
through the schema exactly as HA does, and the happy path is pinned too so the
|
||||
tolerance cannot start swallowing real edits.
|
||||
"""
|
||||
|
||||
@staticmethod
|
||||
def _validate(form, data):
|
||||
"""Run a submitted page through the schema the form returned, as HA does.
|
||||
|
||||
This is the step between the frontend and the flow, and it is where the
|
||||
clearing trap used to fire. The harness's voluptuous mirror carries marker
|
||||
defaults but does not validate (``vol.Schema`` is the identity there), so
|
||||
each field is sent through its own schema entry — the same call HA makes,
|
||||
and the one that used to raise for a cleared entity field.
|
||||
"""
|
||||
raw = form["data_schema"]
|
||||
validators = {
|
||||
getattr(key, "schema", None): value
|
||||
for key, value in raw.items()
|
||||
if getattr(key, "schema", None) is not None
|
||||
}
|
||||
validated = {}
|
||||
for field, value in data.items():
|
||||
validator = validators.get(field)
|
||||
validated[field] = validator(value) if callable(validator) else value
|
||||
return validated
|
||||
|
||||
async def test_the_schema_accepts_an_omitted_none_and_empty_value(self, tmp_path):
|
||||
flow = _options_flow(
|
||||
tmp_path, {CONF_COMMAND_SCRIPTS: {"turn_on": "script.watch"}}
|
||||
)
|
||||
form = await flow.async_step_init(None)
|
||||
|
||||
for value in ("", None):
|
||||
assert self._validate(form, {f"{SCRIPT_FIELD_PREFIX}turn_on": value}) == {
|
||||
f"{SCRIPT_FIELD_PREFIX}turn_on": ""
|
||||
}
|
||||
# a real entity id is still passed through, and an omitted key stays omitted
|
||||
validated = self._validate(form, {f"{SCRIPT_FIELD_PREFIX}turn_on": "script.other"})
|
||||
assert validated[f"{SCRIPT_FIELD_PREFIX}turn_on"] == "script.other"
|
||||
assert f"{SCRIPT_FIELD_PREFIX}volume_up" not in validated
|
||||
|
||||
def test_a_plain_entity_selector_rejects_the_same_values(self):
|
||||
"""The trap itself: without the tolerant selector these shapes blow up.
|
||||
|
||||
The stub mirrors real HA here (see entity_stublib.EntitySelectorStub), so
|
||||
this stays honest without needing a real-HA interpreter.
|
||||
"""
|
||||
import homeassistant.helpers.selector as selector_module
|
||||
|
||||
strict = selector_module.EntitySelector(
|
||||
selector_module.EntitySelectorConfig(domain="script")
|
||||
)
|
||||
|
||||
# the stub raises ValueError; real HA raises probatio.Invalid for the same
|
||||
# values ("Entity is neither a valid entity ID nor a valid UUID", verified)
|
||||
for value in ("", None):
|
||||
with pytest.raises(ValueError):
|
||||
strict(value)
|
||||
|
||||
@pytest.mark.parametrize("value", ["", None])
|
||||
async def test_an_explicit_empty_value_clears_the_script(self, tmp_path, value):
|
||||
flow = _options_flow(
|
||||
tmp_path, {CONF_COMMAND_SCRIPTS: {"turn_on": "script.watch"}}
|
||||
)
|
||||
form = await flow.async_step_init(None)
|
||||
|
||||
# what HA does: validate the submitted page, then run the step
|
||||
result = await flow.async_step_init(
|
||||
self._validate(form, {f"{SCRIPT_FIELD_PREFIX}turn_on": value})
|
||||
)
|
||||
|
||||
assert result["type"] == "create_entry"
|
||||
assert result["data"][CONF_COMMAND_SCRIPTS] == {}
|
||||
|
||||
@pytest.mark.parametrize("value", ["", None])
|
||||
async def test_an_explicit_empty_value_clears_a_preexisting_entity_field(
|
||||
self, tmp_path, value
|
||||
):
|
||||
"""The pattern predates the script selectors — jellyfin had it too."""
|
||||
flow = _options_flow(
|
||||
tmp_path,
|
||||
{CONF_JELLYFIN_ENTITY: "media_player.jellyfin", CONF_HOST: "10.0.0.9"},
|
||||
)
|
||||
form = await flow.async_step_init(None)
|
||||
|
||||
result = await flow.async_step_init(
|
||||
self._validate(form, {CONF_JELLYFIN_ENTITY: value})
|
||||
)
|
||||
|
||||
assert not result["data"][CONF_JELLYFIN_ENTITY] # cleared, not kept
|
||||
assert result["data"][CONF_HOST] == "10.0.0.9" # untouched fields survive
|
||||
|
||||
async def test_an_omitted_script_field_is_cleared_as_well(self, tmp_path):
|
||||
"""The script dict is rebuilt from the page, so omitting a command clears it."""
|
||||
flow = _options_flow(
|
||||
tmp_path, {CONF_COMMAND_SCRIPTS: {"turn_on": "script.watch"}}
|
||||
)
|
||||
form = await flow.async_step_init(None)
|
||||
|
||||
result = await flow.async_step_init(self._validate(form, {}))
|
||||
|
||||
assert result["data"][CONF_COMMAND_SCRIPTS] == {}
|
||||
|
||||
async def test_an_omitted_entity_field_keeps_its_stored_value(self, tmp_path):
|
||||
"""An omitted key is "not touched" for the patched options, not a clear."""
|
||||
flow = _options_flow(
|
||||
tmp_path, {CONF_JELLYFIN_ENTITY: "media_player.jellyfin"}
|
||||
)
|
||||
form = await flow.async_step_init(None)
|
||||
|
||||
result = await flow.async_step_init(self._validate(form, {CONF_HOST: "10.0.0.9"}))
|
||||
|
||||
assert result["data"][CONF_JELLYFIN_ENTITY] == "media_player.jellyfin"
|
||||
|
||||
async def test_a_prefill_submit_round_trip_changes_nothing(self, tmp_path):
|
||||
stored = {
|
||||
CONF_COMMAND_SCRIPTS: {"turn_on": "script.watch"},
|
||||
CONF_JELLYFIN_ENTITY: "media_player.jellyfin",
|
||||
CONF_POWER_ENTITY: "switch.plug",
|
||||
CONF_HOST: "10.0.0.9",
|
||||
}
|
||||
flow = _options_flow(tmp_path, stored)
|
||||
form = await flow.async_step_init(None)
|
||||
suggested = form["data_schema"].get("__suggested__", {})
|
||||
|
||||
# the page comes back carrying the stored values, and the user submits them
|
||||
assert suggested[f"{SCRIPT_FIELD_PREFIX}turn_on"] == "script.watch"
|
||||
result = await flow.async_step_init(
|
||||
self._validate(
|
||||
form,
|
||||
{
|
||||
f"{SCRIPT_FIELD_PREFIX}turn_on": "script.watch",
|
||||
CONF_JELLYFIN_ENTITY: "media_player.jellyfin",
|
||||
CONF_POWER_ENTITY: "switch.plug",
|
||||
CONF_HOST: "10.0.0.9",
|
||||
},
|
||||
)
|
||||
)
|
||||
|
||||
assert result["data"][CONF_COMMAND_SCRIPTS] == {"turn_on": "script.watch"}
|
||||
assert result["data"][CONF_JELLYFIN_ENTITY] == "media_player.jellyfin"
|
||||
assert result["data"][CONF_POWER_ENTITY] == "switch.plug"
|
||||
assert result["data"][CONF_HOST] == "10.0.0.9"
|
||||
|
||||
|
||||
class TestStringsTranslationsParity:
|
||||
"""Both copies of the UI copy must expose the same key tree."""
|
||||
|
||||
|
||||
Reference in new issue
Block a user