fix: graceful handling of malformed element declarations; quiet absent-element log

This commit is contained in:
Mathew Sir Guest the best 2026-07-07 22:58:05 -06:00
parent 6af8468730
commit fbb7d4114a
3 changed files with 100 additions and 10 deletions

@ -1563,8 +1563,8 @@ from unittest.mock import MagicMock
from selenium.common.exceptions import NoSuchElementException from selenium.common.exceptions import NoSuchElementException
from wabot.fields import CheckField, NullField, SelectField, TextField from wabot.fields import CheckField, NullField, SelectField, TextField
from wabot.page import Page
from wabot.pacing import NoPacing from wabot.pacing import NoPacing
from wabot.page import Page
class BasePage(Page): class BasePage(Page):
@ -1606,6 +1606,14 @@ class TestElementResolution:
def test_missing_key_returns_none(self): def test_missing_key_returns_none(self):
assert BasePage(make_browser()).find_element_locators("nope") is None assert BasePage(make_browser()).find_element_locators("nope") is None
def test_empty_override_still_inherits_ancestors(self):
class Grandchild(ChildPage):
elements = {} # declares nothing; must still resolve ancestor keys
page = Grandchild(make_browser())
assert page.find_element_locators("state") == ("select", ("id", "state"))
assert page.find_element_locators("extra") == ("el", ("id", "extra"))
class TestGetProxy: class TestGetProxy:
def test_typed_dispatch(self): def test_typed_dispatch(self):
@ -1649,6 +1657,26 @@ class TestGetProxy:
assert isinstance(CustomPage(make_browser())["thing"], MyField) assert isinstance(CustomPage(make_browser())["thing"], MyField)
def test_malformed_locator_missing_accessors_returns_nullfield(self):
class BadPage(Page):
elements = {"x": ("el",)} # missing the accessors half
el = BadPage(make_browser())["x"]
assert isinstance(el, NullField)
assert not el
def test_malformed_accessors_arity_returns_nullfield(self):
class BadPage(Page):
elements = {"x": ("els", ("id",))} # accessors not a (by, value) pair
assert isinstance(BadPage(make_browser())["x"], NullField)
def test_unknown_type_returns_nullfield(self):
class BadPage(Page):
elements = {"x": ("selct", ("id", "x"))} # typo'd type string
assert isinstance(BadPage(make_browser())["x"], NullField)
class TestVerify: class TestVerify:
def test_default_verify_is_true(self): def test_default_verify_is_true(self):
@ -1679,8 +1707,9 @@ Pages declare a class-level ``elements`` map::
} }
Lookup walks the MRO, so subclasses inherit and override parent maps. Lookup walks the MRO, so subclasses inherit and override parent maps.
``page[key]`` returns a typed field wrapper — never a raw WebElement — ``page[key]`` returns a typed field wrapper (except the ``els`` type, which
and a falsy ``NullField`` when the element cannot be found. returns a raw list of WebElements) and a falsy ``NullField`` when the element
cannot be found.
""" """
from __future__ import annotations from __future__ import annotations
@ -1728,7 +1757,23 @@ class Page:
if not locators: if not locators:
LOGGER.warning("element not in page map: %s", key) LOGGER.warning("element not in page map: %s", key)
return NullField(name=key) return NullField(name=key)
if not isinstance(locators, (tuple, list)) or len(locators) < 2:
LOGGER.error(
"malformed element %r: expected (type, accessors), got %r", key, locators
)
return NullField(name=key)
obj_type, accessors = locators[0], locators[1] obj_type, accessors = locators[0], locators[1]
# built-in types need a (by, value) accessors pair; a custom field class
# manages its own accessors, so only the built-ins are validated here.
if (obj_type in FIELD_TYPES or obj_type == "els") and (
not isinstance(accessors, (tuple, list)) or len(accessors) != 2
):
LOGGER.error(
"malformed accessors for element %r: expected (by, value), got %r",
key,
accessors,
)
return NullField(name=key)
try: try:
if obj_type == "els": if obj_type == "els":
by, value = accessors by, value = accessors
@ -1739,7 +1784,7 @@ class Page:
if field_cls is not None: if field_cls is not None:
return field_cls(page=self, accessors=accessors, name=key) return field_cls(page=self, accessors=accessors, name=key)
except NoSuchElementException: except NoSuchElementException:
LOGGER.warning("element %r not present on current page", key) LOGGER.debug("element %r not present on current page", key)
return NullField(name=key) return NullField(name=key)
LOGGER.error("unknown element type for %r: %r", key, obj_type) LOGGER.error("unknown element type for %r: %r", key, obj_type)
return NullField(name=key) return NullField(name=key)
@ -1751,7 +1796,7 @@ class Page:
- [ ] **Step 4: Run tests to verify they pass** - [ ] **Step 4: Run tests to verify they pass**
Run: `uv run pytest tests/unit/test_page.py -v` Run: `uv run pytest tests/unit/test_page.py -v`
Expected: 12 passed Expected: 15 passed
- [ ] **Step 5: Commit** - [ ] **Step 5: Commit**
@ -2090,7 +2135,7 @@ Append these methods to `class Page`:
- [ ] **Step 4: Run tests to verify they pass** - [ ] **Step 4: Run tests to verify they pass**
Run: `uv run pytest tests/unit/test_page.py -v` Run: `uv run pytest tests/unit/test_page.py -v`
Expected: 23 passed (12 from Task 8 + 11 new) Expected: 26 passed (15 from Task 8 + 11 new)
- [ ] **Step 5: Commit** - [ ] **Step 5: Commit**
@ -2830,7 +2875,7 @@ Run: `uv run pytest tests/unit/test_api.py -v`
Expected: 11 passed Expected: 11 passed
Run: `uv run pytest` Run: `uv run pytest`
Expected: full unit suite passes (113 tests: pacing 7, sessions 14, hosts 19, reattach 6, fields 17, page 23, screenshot 3, browser 13, api 11) Expected: full unit suite passes (116 tests: pacing 7, sessions 14, hosts 19, reattach 6, fields 17, page 26, screenshot 3, browser 13, api 11)
- [ ] **Step 5: Commit** - [ ] **Step 5: Commit**

@ -12,8 +12,9 @@ Pages declare a class-level ``elements`` map::
} }
Lookup walks the MRO, so subclasses inherit and override parent maps. Lookup walks the MRO, so subclasses inherit and override parent maps.
``page[key]`` returns a typed field wrapper never a raw WebElement ``page[key]`` returns a typed field wrapper (except the ``els`` type, which
and a falsy ``NullField`` when the element cannot be found. returns a raw list of WebElements) and a falsy ``NullField`` when the element
cannot be found.
""" """
from __future__ import annotations from __future__ import annotations
@ -61,7 +62,23 @@ class Page:
if not locators: if not locators:
LOGGER.warning("element not in page map: %s", key) LOGGER.warning("element not in page map: %s", key)
return NullField(name=key) return NullField(name=key)
if not isinstance(locators, (tuple, list)) or len(locators) < 2:
LOGGER.error(
"malformed element %r: expected (type, accessors), got %r", key, locators
)
return NullField(name=key)
obj_type, accessors = locators[0], locators[1] obj_type, accessors = locators[0], locators[1]
# built-in types need a (by, value) accessors pair; a custom field class
# manages its own accessors, so only the built-ins are validated here.
if (obj_type in FIELD_TYPES or obj_type == "els") and (
not isinstance(accessors, (tuple, list)) or len(accessors) != 2
):
LOGGER.error(
"malformed accessors for element %r: expected (by, value), got %r",
key,
accessors,
)
return NullField(name=key)
try: try:
if obj_type == "els": if obj_type == "els":
by, value = accessors by, value = accessors
@ -72,7 +89,7 @@ class Page:
if field_cls is not None: if field_cls is not None:
return field_cls(page=self, accessors=accessors, name=key) return field_cls(page=self, accessors=accessors, name=key)
except NoSuchElementException: except NoSuchElementException:
LOGGER.warning("element %r not present on current page", key) LOGGER.debug("element %r not present on current page", key)
return NullField(name=key) return NullField(name=key)
LOGGER.error("unknown element type for %r: %r", key, obj_type) LOGGER.error("unknown element type for %r: %r", key, obj_type)
return NullField(name=key) return NullField(name=key)

@ -46,6 +46,14 @@ class TestElementResolution:
def test_missing_key_returns_none(self): def test_missing_key_returns_none(self):
assert BasePage(make_browser()).find_element_locators("nope") is None assert BasePage(make_browser()).find_element_locators("nope") is None
def test_empty_override_still_inherits_ancestors(self):
class Grandchild(ChildPage):
elements = {} # declares nothing; must still resolve ancestor keys
page = Grandchild(make_browser())
assert page.find_element_locators("state") == ("select", ("id", "state"))
assert page.find_element_locators("extra") == ("el", ("id", "extra"))
class TestGetProxy: class TestGetProxy:
def test_typed_dispatch(self): def test_typed_dispatch(self):
@ -89,6 +97,26 @@ class TestGetProxy:
assert isinstance(CustomPage(make_browser())["thing"], MyField) assert isinstance(CustomPage(make_browser())["thing"], MyField)
def test_malformed_locator_missing_accessors_returns_nullfield(self):
class BadPage(Page):
elements = {"x": ("el",)} # missing the accessors half
el = BadPage(make_browser())["x"]
assert isinstance(el, NullField)
assert not el
def test_malformed_accessors_arity_returns_nullfield(self):
class BadPage(Page):
elements = {"x": ("els", ("id",))} # accessors not a (by, value) pair
assert isinstance(BadPage(make_browser())["x"], NullField)
def test_unknown_type_returns_nullfield(self):
class BadPage(Page):
elements = {"x": ("selct", ("id", "x"))} # typo'd type string
assert isinstance(BadPage(make_browser())["x"], NullField)
class TestVerify: class TestVerify:
def test_default_verify_is_true(self): def test_default_verify_is_true(self):