Skip to content

Commit

Permalink
[#530] REFACTOR: condition names as callables by entity type...
Browse files Browse the repository at this point in the history
 ... to support name per entity type
  • Loading branch information
yashaka committed Jul 11, 2024
1 parent a3985aa commit 17ed83d
Show file tree
Hide file tree
Showing 6 changed files with 117 additions and 41 deletions.
8 changes: 6 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,12 @@ class Collection(WaitingEntity['Collection'], Iterable[Element]):

#### TODO: also check if have.size will be still consistent fully... maybe have.count?

### TODO: review Query._name impl. and processing

taking into account its callable based on Entity nature, and "name" vs "description" terminology

consider `str | Callable[[...], str]` vs `str | Callable[[...], str]` as type for name

## 2.0.0rc10: «copy&paste, frames, shadow & texts_like» (to be released on DD.05.2024)

### TODO: consider renaming Condition `test` arg to `match` for consistency with Match
Expand Down Expand Up @@ -204,8 +210,6 @@ should we even refactor out them from Condition and move to Match only?

### TODO: rename all conditions inside match.py so match can be fully used instead be + have #530

### TODO: Consider changing name type from str | Callable[[], str] to str | Callable[[E], str],

### Deprecated conditions

- `be.present` in favor of `be.present_in_dom`
Expand Down
35 changes: 28 additions & 7 deletions selene/common/_typing_functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
overload,
Type,
Iterable,
Protocol,
runtime_checkable,
)

from selene.common.fp import thread_last
Expand All @@ -50,24 +52,40 @@
Fn = Callable[[T], R]


@runtime_checkable
class _SupportsNameForEntity(Protocol):
def _name_for(self, entity: E | None) -> str: ...


class Query(Generic[E, R]):
def __init__(self, name: str, /, fn: Callable[[E], R | None]):
def __init__(
self,
name: str | Callable[[E | None], str],
/,
fn: Callable[[E], R | None],
):
self._name = name
self._fn = fn

def __call__(self, entity: E) -> R | None:
return self._fn(entity)

def __str__(self):
return self._name
return self._name if not callable(self._name) else self._name(None)

def _name_for(self, entity: E) -> str:
return self._name(entity) if callable(self._name) else self._name

@staticmethod
def full_name_for(callable_: Optional[Callable]) -> str | None:
def full_name_for(
callable_: Optional[Callable],
_entity: E | None = None,
) -> str | None:
if callable_ is None:
return None

if isinstance(callable_, Query):
return str(callable_)
if isinstance(callable_, _SupportsNameForEntity):
return callable_._name_for(_entity)

# callable_ has its own __str__ implementation in its class
if type(callable_).__str__ != object.__str__:
Expand Down Expand Up @@ -98,8 +116,11 @@ def full_name_for(callable_: Optional[Callable]) -> str | None:

# todo: would not human_readable_name_for be a better name for this helper?
@staticmethod
def full_description_for(callable_: Optional[Callable]) -> str | None:
full_name = Query.full_name_for(callable_)
def full_description_for(
callable_: Optional[Callable],
_entity: E | None = None,
) -> str | None:
full_name = Query.full_name_for(callable_, _entity)
return (
thread_last(
full_name,
Expand Down
57 changes: 41 additions & 16 deletions selene/core/condition.py
Original file line number Diff line number Diff line change
Expand Up @@ -788,7 +788,7 @@ def raise_if_not_actual(
@overload
def __init__(
self,
name: str | Callable[[], str],
name: str | Callable[[E | None], str],
/,
test: Lambda[E, None],
*,
Expand All @@ -799,7 +799,7 @@ def __init__(
# @overload
# def __init__(
# self,
# name: str | Callable[[], str],
# name: str | Callable[[E | None], str],
# *,
# by: Tuple[Lambda[E, R], Predicate[R]],
# _inverted=False,
Expand All @@ -808,7 +808,7 @@ def __init__(
@overload
def __init__(
self,
name: str | Callable[[], str],
name: str | Callable[[E | None], str],
/,
*,
actual: Lambda[E, R],
Expand All @@ -821,7 +821,7 @@ def __init__(
@overload
def __init__(
self,
name: str | Callable[[], str],
name: str | Callable[[E | None], str],
/,
*,
by: Predicate[E],
Expand All @@ -845,8 +845,7 @@ def __init__(
# where first is name for query and second is query fn
def __init__(
self,
# TODO: consider removing Callable[[], str] as supported type for name
name: str | Callable[[], str],
name: str | Callable[[E | None], str], # TODO: consider Callable[[...], str]
/,
test: Lambda[E, None] | None = None,
*,
Expand Down Expand Up @@ -981,22 +980,48 @@ def not_(self) -> Condition[E]:
)
)

def __describe(self) -> str:
return self.__name if not callable(self.__name) else self.__name()
def __describe(self, _entity: E | None = None) -> str:
return self.__name if not callable(self.__name) else self.__name(_entity)

def __describe_inverted(self) -> str:
condition_words = self.__describe().split(' ')
def __describe_inverted(self, _entity: E | None = None) -> str:
condition_words = self.__describe(_entity).split(' ')
is_or_have = condition_words[0]
if is_or_have not in ('is', 'has', 'have'):
return f'not ({self.__describe()})'
return f'not ({self.__describe(_entity)})'
name = ' '.join(condition_words[1:])
no_or_not = 'not' if is_or_have == 'is' else 'no'
return f'{is_or_have} {no_or_not} ({name})'

# TODO: consider changing has to have on the fly for CollectionConditions
# TODO: or changing in collection locator rendering `all` to `collection`
# todo: or changing in collection locator rendering `all` to `collection`
def __str__(self):
return self.__describe() if not self.__inverted else self.__describe_inverted()
# return self.__describe() if not self.__inverted else self.__describe_inverted()
return self._name_for()

# todo: finalize method name (see _typing_functions._SupportsNameForEntity protocol)
# should even be a method? or just a property with callable?
# hm, probably yes, it should be a method to eliminate branching on usage
# while on init we can accept param that can be str or callable
# but when coming to final action - it should be always a method call
# though, we still can implement it as a property returning callable
# but kind of... what for? seems like for no any benefit...
# ok... let's think on naming... is the following good? –
# condition.describe(entity) - ?
# in fact condition does not describe entity, it's more an entity describing condition
# ok, other options:
# - condition.described_by(entity)
# - condition.describe_for(entity)
# - condition.description_for(entity) # + descriptive! emphasize that it is human readable
# - condition.name_for(entity) # + concise! correlate with name param on init
# - condition.repr_for(entity) # + correlate with __repr__ over __str__; + concise; - weird shortcut
# - condition.for(entity) # - looks like a builder
# # to remember entity so condition can be .__call__() later without passing entity
def _name_for(self, _entity: E | None = None):
return (
self.__describe(_entity)
if not self.__inverted
else self.__describe_inverted(_entity)
)

# todo: we already have entity.matching for Callable[[E], bool]
# is it a good idea to use same term for Callable[[E], None] raising error?
Expand Down Expand Up @@ -1493,7 +1518,7 @@ def __init__x(self, *args, **kwargs):
@overload
def __init__(
self,
name: str | Callable[[], str],
name: str | Callable[[E | None], str],
/,
actual: Lambda[E, R],
*,
Expand All @@ -1506,7 +1531,7 @@ def __init__(
@overload
def __init__(
self,
name: str | Callable[[], str],
name: str | Callable[[E | None], str],
/,
*,
by: Predicate[E],
Expand Down Expand Up @@ -1536,7 +1561,7 @@ def __init__(

def __init__(
self,
name: str | Callable[[], str] | None = None,
name: str | Callable[[E | None], str] | None = None,
actual: Lambda[E, R] | None = None,
*,
by: Predicate[E] | Predicate[R],
Expand Down
30 changes: 21 additions & 9 deletions selene/core/match.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@
Callable,
Self,
Type,
cast,
Optional,
)

from selene.common import predicate, helpers, appium_tools
Expand Down Expand Up @@ -734,14 +736,16 @@ class size(Match[Union[Collection, Browser, Element]]):
def __init__(
self,
expected: int | dict,
_name='has size', # TODO: fix to `have` for Collection, has otherwise
# should we also tune actual rendering based on
# config._match_only_visible_elements_size?
_name=lambda entity: (
'have size' if isinstance(entity, Collection) else 'has size'
),
# TODO: should we also tune actual rendering based on
# config._match_only_visible_elements_size?
_by=predicate.equals,
_inverted=False,
):
self.__expected = expected
self.__name = f'{_name} {expected}'
self.__name = lambda entity: f'{_name(entity)} {expected}'
self.__by = _by
self.__inverted = _inverted

Expand All @@ -768,17 +772,19 @@ def __init__(
# )
@property
def or_less(self) -> Condition[Collection]:
name = cast(Callable[[Optional[Collection]], str], self.__name)
return Match(
f'{self.__name} or less',
lambda entity: f'{name(entity)} or less',
query.size,
by=predicate.is_less_than_or_equal(self.__expected),
_inverted=self.__inverted,
)

@property
def or_more(self) -> Condition[Collection]:
name = cast(Callable[[Optional[Collection]], str], self.__name)
return Match(
f'{self.__name} or more',
lambda entity: f'{name(entity)} or more',
query.size,
by=predicate.is_greater_than_or_equal(self.__expected),
_inverted=self.__inverted,
Expand All @@ -787,7 +793,10 @@ def or_more(self) -> Condition[Collection]:
@property
def _more_than(self) -> Condition[Collection]:
return Match(
f'has size more than {self.__expected}',
lambda entity: (
('have' if isinstance(entity, Collection) else 'has')
+ f' size more than {self.__expected}'
),
query.size,
by=predicate.is_greater_than(self.__expected),
_inverted=self.__inverted,
Expand All @@ -796,7 +805,10 @@ def _more_than(self) -> Condition[Collection]:
@property
def _less_than(self) -> Condition[Collection]:
return Match(
f'has size less than {self.__expected}',
lambda entity: (
('have' if isinstance(entity, Collection) else 'has')
+ f' size less than {self.__expected}'
),
query.size,
by=predicate.is_less_than(self.__expected),
_inverted=self.__inverted,
Expand Down Expand Up @@ -924,7 +936,7 @@ def __init__(
): # noqa
if self._MATCHING_SEPARATOR.__len__() != 1:
raise ValueError('MATCHING_SEPARATOR should be a one character string')
super().__init__(self.__str__, test=self.__call__)
super().__init__(lambda _: self.__str__(), test=self.__call__)
self._expected = expected
self._inverted = _inverted
self._globs = _globs if _globs else _exact_texts_like._DEFAULT_GLOBS
Expand Down
2 changes: 1 addition & 1 deletion selene/core/wait.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ def logic(fn: Callable[[E], R]) -> R:

# TODO: consider using Query.full_description_for(fn)
# TODO: consider customizing what to use on __init__
fn_name = Query.full_name_for(fn) or str(fn)
fn_name = Query.full_name_for(fn, entity) or str(fn)

failure = TimeoutException(
f'\n'
Expand Down
26 changes: 20 additions & 6 deletions tests/integration/condition__mixed_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,16 @@ def test_should_have_size__applied_to_collection__passed_and_failed(
ss('li').should(have.size(8))
ss('li').should(have.no.size(7))

try:
ss('li').should(have.size(7))
pytest.fail('expect mismatch')
except AssertionError as error:
assert (
"browser.all(('css selector', 'li')).have size 7\n"
'\n'
'Reason: ConditionMismatch: actual: 8\n'
) in str(error)

# have size or less?
ss('li').should(have.size(9).or_less)
ss('li').should(have.size(8).or_less)
Expand All @@ -170,7 +180,7 @@ def test_should_have_size__applied_to_collection__passed_and_failed(
pytest.fail('expect mismatch')
except AssertionError as error:
assert (
"browser.all(('css selector', 'li')).has size 7 or less\n"
"browser.all(('css selector', 'li')).have size 7 or less\n"
'\n'
'Reason: ConditionMismatch: actual size: 8\n'
) in str(error)
Expand All @@ -179,11 +189,13 @@ def test_should_have_size__applied_to_collection__passed_and_failed(
ss('li').should(have.no.size(8).or_less)
pytest.fail('expect mismatch')
except AssertionError as error:
assert (
"browser.all(('css selector', 'li')).has no (size 8 or less)\n"
assert ( # TODO: fix it because now inversion does not come into action... in context of describing...
"browser.all(('css selector', 'li')).have no (size 8 or less)\n"
'\n'
'Reason: ConditionMismatch: actual size: 8\n'
) in str(error)
) in str(
error
)
ss('li').should(have.no.size_less_than_or_equal(7))

# todo: add a few failed cases below...
Expand Down Expand Up @@ -241,6 +253,7 @@ def test_should_have_size__applied_to_browser__passed_and_failed(
"Reason: ConditionMismatch: actual: {'width': 720, 'height': 480}\n"
) in str(error)
browser.should(have.no.size({'height': 481, 'width': 720}))
# TODO: add failed inverted case


# todo: make it pass both locally and on CI (on CI the field has different size, why?)
Expand Down Expand Up @@ -359,7 +372,7 @@ def test_should_be_emtpy__applied_to_non_form__passed_and_failed__compared(
pytest.fail('expect mismatch')
except AssertionError as error:
assert (
"browser.all(('css selector', '#hidden')).has size 0\n"
"browser.all(('css selector', '#hidden')).have size 0\n"
'\n'
'Reason: ConditionMismatch: actual: 2\n'
) in str(error)
Expand Down Expand Up @@ -416,10 +429,11 @@ def test_should_be_emtpy__applied_to_non_form__passed_and_failed__compared(
pytest.fail('expect mismatch')
except AssertionError as error:
assert (
"browser.all(('css selector', '.absent')).has no (size 0)\n"
"browser.all(('css selector', '.absent')).have no (size 0)\n"
'\n'
'Reason: ConditionMismatch: actual: 0\n'
) in str(error)
pytest.exit('passed')
ss('.absent').should(be._empty)
try:
ss('.absent').should(be.not_._empty)
Expand Down

0 comments on commit 17ed83d

Please sign in to comment.