Skip to content

Commit

Permalink
REFACTOR: enabled, disabled, clickable conditions + ...
Browse files Browse the repository at this point in the history
+ cover with tests
+ fix enabled.not_ behavior to fail on absent element

- left some TODOs, especially on falsy_exceptions case...
  • Loading branch information
yashaka committed Jun 19, 2024
1 parent 93f1c8b commit db8c502
Show file tree
Hide file tree
Showing 11 changed files with 475 additions and 40 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ seems like currently we do raise, but cover with tests

#### TODO: do we need positional actual and by args for Match?

### TODO: decide on _falsy_exceptions name

### Deprecated conditions

- `be.present` in favor of `be.present_in_dom`
Expand Down
66 changes: 56 additions & 10 deletions selene/core/condition.py
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,7 @@ def is_more_than(limit):
cast,
overload,
Union,
Type,
)

from selene.core.exceptions import ConditionMismatch
Expand Down Expand Up @@ -783,6 +784,7 @@ def __init__(
test: Lambda[E, None],
*,
_inverted=False,
_falsy_exceptions: Iterable[Type[Exception]] = (AssertionError,),
): ...

# @overload
Expand All @@ -799,9 +801,20 @@ def __init__(
self,
description: str | Callable[[], str],
*,
actual: Lambda[E, R] | None = None,
actual: Lambda[E, R],
by: Predicate[R],
_inverted=False,
_falsy_exceptions: Iterable[Type[Exception]] = (AssertionError,),
): ...

@overload
def __init__(
self,
description: str | Callable[[], str],
*,
by: Predicate[E],
_inverted=False,
_falsy_exceptions: Iterable[Type[Exception]] = (AssertionError,),
): ...

# todo: CONSIDER: accepting tuple of three as description
Expand All @@ -826,11 +839,33 @@ def __init__(
actual: Lambda[E, R] | None = None,
by: Predicate[R] | None = None,
_inverted=False,
# TODO: better name for _falsy_exceptions?
# falsy means that it will become true on inverted
# i.e. such exceptions if inverted, will be considered as "truthy"
# the problem with such name is that if _inverted=False
# then any exception is a False in context of condition behavior,
# that simply throw an error as Falsy behavior
# maybe then better name would be:
# _truthy_exceptions_on_inverted = (AssertionError,)
# or
# _pass_on_inverted_when = (AssertionError,)
# or
# _pass_on_inverted_when = (AssertionError,)
# hm... but maybe, we actually would want to keep _falsy_exceptions
# but make it more generous... i.e. not throwing ConditionMismatch
# on any error that is not "falsy"... Make the logic the following:
# if error is in _falsy_exceptions, then raise ConditionMismatch
# (or pass on inverted)
# else just pass the error through... This will give an opportunity
# to decide whether to ignore some non-condition-mismatch errors
# inside wait.for_ ...
_falsy_exceptions: Iterable[Type[Exception]] = (AssertionError,),
):
# can be already stored
self.__description = description
self.__inverted = _inverted
self.__by: Predicate[R] | None = None
self.__falsy_exceptions = _falsy_exceptions
self.__by = None

if by: # i.e. condition is based on predicate (fn returning True/False)
if test:
Expand All @@ -851,12 +886,12 @@ def __init__(
self.__by,
# TODO: should we DI? – remove this tight coupling to WebDriverException?
# here and elsewhere
_falsy_exceptions=(AssertionError, WebDriverException),
_falsy_exceptions=_falsy_exceptions,
)
if self.__actual
else ConditionMismatch._to_raise_if(
self.__by,
_falsy_exceptions=(AssertionError, WebDriverException),
_falsy_exceptions=_falsy_exceptions,
)
)
return
Expand Down Expand Up @@ -922,13 +957,17 @@ def not_(self) -> Condition[E]:
self.__description,
test=self.__test,
_inverted=not self.__inverted,
_falsy_exceptions=self.__falsy_exceptions,
)
if not self.__by
else Condition(
self.__description,
actual=self.__actual,
by=self.__by,
_inverted=not self.__inverted,
else (
Condition(
self.__description,
actual=self.__actual, # type: ignore
by=self.__by,
_inverted=not self.__inverted,
_falsy_exceptions=self.__falsy_exceptions,
)
)
)

Expand Down Expand Up @@ -1054,6 +1093,7 @@ def each(self) -> Condition[Iterable[E]]:
# hm... but won't it be confusing to pass "only predicate" in place of by
# in the Condition class? o_O
# It really looks like pretty confusing:D
# NOTE: if implemented, take into account _falsy_exceptions...
# TODO: just a crazy idea... why then import both Match and/or match.*
# why not to make match be a class over module –
# a class with static methods and attributes as predefined conditions
Expand Down Expand Up @@ -1411,6 +1451,7 @@ def __init__(
*,
by: Predicate[R],
_inverted=False,
_falsy_exceptions: Iterable[Type[Exception]] = (AssertionError,),
): ...

@overload
Expand All @@ -1420,6 +1461,7 @@ def __init__(
*,
by: Predicate[E],
_inverted=False,
_falsy_exceptions: Iterable[Type[Exception]] = (AssertionError,),
): ...

@overload
Expand All @@ -1429,6 +1471,7 @@ def __init__(
actual: Lambda[E, R],
by: Predicate[R],
_inverted=False,
_falsy_exceptions: Iterable[Type[Exception]] = (AssertionError,),
): ...

@overload
Expand All @@ -1437,6 +1480,7 @@ def __init__(
*,
by: Predicate[E],
_inverted=False,
_falsy_exceptions: Iterable[Type[Exception]] = (AssertionError,),
): ...

# TODO: should we rename description to name? won't it confuse with __name__?
Expand All @@ -1448,6 +1492,7 @@ def __init__(
*,
by: Predicate[E] | Predicate[R],
_inverted=False,
_falsy_exceptions: Iterable[Type[Exception]] = (AssertionError,),
):
"""
The only valid signatures in usage:
Expand Down Expand Up @@ -1475,9 +1520,10 @@ def __init__(
# TODO: fix "cannot infer type of argument 1 of __init__" or ignore
super().__init__( # type: ignore
description=description,
actual=actual,
actual=actual, # type: ignore
by=by,
_inverted=_inverted,
_falsy_exceptions=_falsy_exceptions,
)


Expand Down
12 changes: 7 additions & 5 deletions selene/core/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,9 @@ def describe_not_match(actual_value):
)

def describe_error(error):
# TODO: consider making it customizable
# TODO: should not we remove/add stacktrace somewhere in Wait? not here?
# or do it only for _falsy_exceptions?
# todo: consider making it customizable
# remove stacktrace if available:
stacktrace = getattr(error, 'stacktrace', None)
return (
Expand All @@ -183,8 +185,6 @@ def describe_error(error):
)
)

# TODO: should we catch errors on actual?
# for e.g. to consider them as False indicator
actual_to_test = None
try:
actual_to_test = actual(entity) if actual else entity
Expand All @@ -193,14 +193,16 @@ def describe_error(error):
isinstance(reason, exception) for exception in _falsy_exceptions
):
return
# TODO: do we even need this prefix?
# todo: do we even need this prefix?
# raise cls(f'Unable to get actual to match:\n{describe_error(reason)}')
# TODO: should we wrap as ConditionMismatch only those errors that are in _false_exceptions?
# seems like yes...
raise cls(describe_error(reason)) from reason

answer = None
try:
answer = by(actual_to_test)
# TODO: should we move Exception processing out of this helper?
# todo: should we move Exception processing out of this helper?
# should it be somewhere in Condition?
# cause now it's not a Mismatch anymore, it's a failure
# – no, we should not, we should keep it here,
Expand Down
37 changes: 24 additions & 13 deletions selene/core/match.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import warnings
from functools import reduce

from selenium.common import WebDriverException, NoSuchElementException
from typing_extensions import (
List,
Any,
Expand Down Expand Up @@ -60,6 +61,7 @@
'is present in DOM',
actual=lambda element: element.locate(),
by=lambda webelement: webelement is not None,
_falsy_exceptions=(NoSuchElementException,),
)

absent_in_dom: Condition[Element] = Condition.as_not(present_in_dom, 'is absent in DOM')
Expand All @@ -76,6 +78,7 @@ def __deprecated_is_present(element: Element) -> bool:
present: Condition[Element] = Match(
'is present in DOM',
by=__deprecated_is_present, # noqa
_falsy_exceptions=(NoSuchElementException,),
)
"""Deprecated 'is present' condition. Use present_in_dom instead. """

Expand All @@ -95,12 +98,14 @@ def __deprecated_is_existing(element: Element) -> bool:
existing: Condition[Element] = Match(
'is present in DOM',
by=__deprecated_is_existing, # noqa
_falsy_exceptions=(NoSuchElementException,),
)
"""Deprecated 'is existing' condition. Use present_in_dom instead."""

visible: Condition[Element] = Match(
'is visible',
by=lambda element: element.locate().is_displayed(),
_falsy_exceptions=(NoSuchElementException,),
)

# todo: remove once decide on the best implementation
Expand Down Expand Up @@ -142,19 +147,30 @@ def __deprecated_is_existing(element: Element) -> bool:
hidden_in_dom: Condition[Element] = present_in_dom.and_(visible.not_)


element_is_enabled: Condition[Element] = ElementCondition.raise_if_not(
'is enabled', lambda element: element().is_enabled()
enabled: Condition[Element] = Match(
'is enabled',
by=lambda element: element.locate().is_enabled(),
)

element_is_disabled: Condition[Element] = ElementCondition.as_not(element_is_enabled)
# disabled: Condition[Element] = Condition.as_not(enabled, 'disabled')
disabled: Condition[Element] = enabled.not_

element_is_clickable: Condition[Element] = visible.and_(element_is_enabled)
clickable: Condition[Element] = visible.and_(enabled)

# TODO: how will it work for mobile?
element_is_focused: Condition[Element] = ElementCondition.raise_if_not(

selected: Condition[Element] = Match(
'is selected',
by=lambda element: element.locate().is_selected(),
)


# TODO: how will it work for mobile? – it will not work:)
element_is_focused: Condition[Element] = Match(
'is focused',
lambda element: element()
== element.config.driver.execute_script('return document.activeElement'),
by=lambda element: (
element.locate()
== element.config.driver.execute_script('return document.activeElement')
),
)


Expand Down Expand Up @@ -438,11 +454,6 @@ def values_containing(
)


element_is_selected: Condition[Element] = ElementCondition.raise_if_not(
'is selected', lambda element: element().is_selected()
)


def element_has_value(expected: str | int | float) -> Condition[Element]:
return element_has_attribute('value').value(expected)

Expand Down
8 changes: 4 additions & 4 deletions selene/support/conditions/be.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@
hidden = match.hidden
visible = match.visible

selected = match.element_is_selected
selected = match.selected

enabled = match.element_is_enabled
disabled = match.element_is_disabled
enabled = match.enabled
disabled = match.disabled

clickable = match.element_is_clickable
clickable = match.clickable

blank = match.element_is_blank

Expand Down
4 changes: 2 additions & 2 deletions selene/support/conditions/not_.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@
in_dom: Condition[Element] = _match.present_in_dom.not_


enabled: Condition[Element] = _match.element_is_enabled.not_
disabled: Condition[Element] = _match.element_is_disabled.not_
enabled: Condition[Element] = _match.enabled.not_
disabled: Condition[Element] = _match.disabled.not_

blank: Condition[Element] = _match.element_is_blank.not_

Expand Down
2 changes: 2 additions & 0 deletions tests/const.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
# TINYMCE_URL = 'https://www.tiny.cloud/docs/tinymce/latest/cloud-quick-start/'
TINYMCE_URL = 'https://autotest.how/demo/tinymce'
# SELENOID_HOST = 'selenoid.autotests.cloud'
SELENOID_HOST = 'selenoid.autotest.how'
Loading

0 comments on commit db8c502

Please sign in to comment.