From d1da03f8e41d3469bf63e942226836f388635fd2 Mon Sep 17 00:00:00 2001 From: yashaka Date: Mon, 8 Jul 2024 22:23:35 +0300 Subject: [PATCH] [#530] REFACTOR: disable description as keyword arg... ... to Condition, Match, Query, Command. Now it's a positional only. It's kind of a prep step to potential renaming it to name. Also leave some new TODOs on Condition/Match args design. And made tiny fixes in Condition docstring examples. --- CHANGELOG.md | 59 +++++++++++++++++++++++--- selene/common/_typing_functions.py | 2 +- selene/core/condition.py | 55 +++++++++++++++--------- selene/core/exceptions.py | 2 +- selene/core/match.py | 68 +++--------------------------- 5 files changed, 98 insertions(+), 88 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b44937ed..891b01c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -137,6 +137,60 @@ class Collection(WaitingEntity['Collection'], Iterable[Element]): ## 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 + +Compare current: + +```python +from selene.core.condition import Condition, Match +from selene.core.exceptions import ConditionMismatch + +has_positive_number = Condition( + 'has positive number', + test=ConditionMismatch._to_raise_if_not( + actual=lambda entity: entity.number, + by=lambda number: number > 0, + ) +) +# => +has_positive_number_ = Match( + 'has positive number', + actual=lambda entity: entity.number, + by=lambda number: number > 0, +) +``` + +to: + +```python +from selene.core.condition import Condition, Match +from selene.core.exceptions import ConditionMismatch + +has_positive_number = Condition( + 'has positive number', + match=ConditionMismatch._to_raise_if_not( + actual=lambda entity: entity.number, + by=lambda number: number > 0, + ) +) +# => +has_positive_number_ = Match( + 'has positive number', + actual=lambda entity: entity.number, + by=lambda number: number > 0, +) +``` + +Then the reason of Match existence becomes easier to understand:) +Who knows, maybe we even have to remove `actual` and by `args` of Condition, +or, at least, not remove, but make them private (`_actual`, `_by`) and reveal them for usage only in `Match` as `actual` and `by`... + +Then we have to consider rething condition.__call__ aliases... And corresponding `test` vs `match` naming... We have some kind of conflict between `entity.matching(condition)` and `condition.match(entity)`, because the term is same, but meaning different – predicate vs assertion... But also the gramatic form is different `*ing` vs `*`... So maybe it's ok... + +`Test` might be also a good candidate over `Match` ... But `Test` does not correlate in `entity.should(Test(actual=..., by=...)) + +### TODO: consider removing Callable[[], str] as supported type for description in Condition + ### TODO: decide on ... vs (...,) as one_or_more ### TODO: ensure no warnings @@ -147,11 +201,6 @@ class Collection(WaitingEntity['Collection'], Iterable[Element]): ### TODO: check if there are no type errors on passing be._empty to should -### TODO: decide on Match `__init__` fate: allow by as positional or not? - -- probably not for now (let's come back after 2.0)... actual already can be passed as positional... - - passing by as positional would not add so much benefits... - ### Consider making configurable the "filtering collection for visibility" like in texts type of conditions ### Deprecated conditions diff --git a/selene/common/_typing_functions.py b/selene/common/_typing_functions.py index 5ef0edfa..99344b35 100644 --- a/selene/common/_typing_functions.py +++ b/selene/common/_typing_functions.py @@ -51,7 +51,7 @@ class Query(Generic[E, R]): - def __init__(self, description: str, fn: Callable[[E], R | None]): + def __init__(self, description: str, /, fn: Callable[[E], R | None]): self._description = description self._fn = fn diff --git a/selene/core/condition.py b/selene/core/condition.py index 7fa44a8f..5de73f7f 100644 --- a/selene/core/condition.py +++ b/selene/core/condition.py @@ -525,7 +525,7 @@ class Condition(Generic[E]): ```python has_positive_number = Condition( - 'has positive number' + 'has positive number', ConditionMismatch._to_raise_if_not_actual( lambda entity: entity.number, lambda number: number > 0, @@ -567,7 +567,7 @@ class Condition(Generic[E]): ```python has_positive_number = Condition( - 'has positive number' + 'has positive number', actual=lambda entity: entity.number, by=lambda number: number > 0, ) @@ -595,7 +595,7 @@ class Condition(Generic[E]): ```python has_positive_number = Condition( - 'has positive number' + 'has positive number', ConditionMismatch._to_raise_if_not_actual( lambda entity: entity.number, lambda number: number > 0, @@ -609,7 +609,7 @@ class Condition(Generic[E]): ```python has_positive_number = Condition( - description='has positive number' + 'has positive number', test=ConditionMismatch._to_raise_if_not_actual( lambda entity: entity.number, lambda number: number > 0, @@ -631,10 +631,10 @@ class Condition(Generic[E]): ```python has_positive_number = Condition( - description='has positive number' - test=ConditionMismatch._to_raise_if_not_actual( - lambda entity: entity.number, - lambda number: number > 0, + 'has positive number', + test=ConditionMismatch._to_raise_if_not( + actual=lambda entity: entity.number, + by=lambda number: number > 0, ) ) ``` @@ -643,7 +643,7 @@ class Condition(Generic[E]): ```python has_positive_number = Condition( - 'has positive number' + 'has positive number', actual=lambda entity: entity.number, by=lambda number: number > 0, ) @@ -653,7 +653,7 @@ class Condition(Generic[E]): ```python has_positive_number = Condition( - 'has positive number' + 'has positive number', by=lambda entity: entity.number > 0, ) ``` @@ -781,6 +781,7 @@ def raise_if_not_actual( def __init__( self, description: str | Callable[[], str], + /, test: Lambda[E, None], *, _inverted=False, @@ -800,6 +801,7 @@ def __init__( def __init__( self, description: str | Callable[[], str], + /, *, actual: Lambda[E, R], by: Predicate[R], @@ -812,6 +814,7 @@ def __init__( def __init__( self, description: str | Callable[[], str], + /, *, by: Predicate[E], _inverted=False, @@ -834,7 +837,9 @@ 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 description description: str | Callable[[], str], + /, test: Lambda[E, None] | None = None, *, actual: Lambda[E, R] | None = None, @@ -1391,9 +1396,19 @@ def __init__x( @overload def __init__x(self, actual: Lambda[E, R], by: Predicate[R]): ... - # TODO: do we really need such complicated impl in order + # todo: do we really need such complicated impl in order # to allow passing actual and by as positional arguments? - # also, take into account that currently the _describe_actual_result + # probably not for now (let's come back after 2.0)... + # actual already can be passed as positional in some cases... + # and by is pretty concise as it is... + # so seems like passing by as positional would not add so much benefits... + # though passing actual as positional in some cases (where not it is mandatory) + # can make it cleaner... like in: + # Match(lambda x: x - 1, by=lambda result: result > 0) + # # vs + # Match(actual=lambda x: x - 1, by=lambda actual: actual > 0) + # yet, the latter is not so bad... even has its benefits... + # todo: also, take into account that currently the _describe_actual_result # is not counted in the impl below def __init__x(self, *args, **kwargs): """ @@ -1475,6 +1490,7 @@ def __init__x(self, *args, **kwargs): def __init__( self, description: str | Callable[[], str], + /, actual: Lambda[E, R], *, by: Predicate[R], @@ -1487,6 +1503,7 @@ def __init__( def __init__( self, description: str | Callable[[], str], + /, *, by: Predicate[E], _inverted=False, @@ -1556,7 +1573,7 @@ def __init__( ) # TODO: fix "cannot infer type of argument 1 of __init__" or ignore super().__init__( # type: ignore - description=description, + description, actual=actual, # type: ignore by=by, _describe_actual_result=_describe_actual_result, @@ -1577,13 +1594,13 @@ def __init__( # Match(lambda x: x > 0) # Match(lambda actual: actual - 1, lambda res: res > 0) # Match('has positive decrement', lambda actual: actual - 1, lambda res: res > 0) +Match( + lambda: 'has positive decrement', + actual=lambda actual: actual - 1, + by=lambda res: res > 0, +) # Match( -# description=lambda: 'has positive decrement', -# actual=lambda actual: actual - 1, -# by=lambda res: res > 0, -# ) -# Match( -# description=lambda: 'has positive decrement', +# lambda: 'has positive decrement', # by=lambda res: res > 0, # ) diff --git a/selene/core/exceptions.py b/selene/core/exceptions.py index 2a157db4..7e0ef1ce 100644 --- a/selene/core/exceptions.py +++ b/selene/core/exceptions.py @@ -153,7 +153,7 @@ def wrapped(entity: E) -> None: def describe_not_match(actual_value): describe_actual_result = _describe_actual_result or ( lambda value: ( - f'actual' + 'actual' + ( f' {actual_name}' if (actual_name := Query.full_description_for(actual)) diff --git a/selene/core/match.py b/selene/core/match.py index 7f20bd82..c830ee4c 100644 --- a/selene/core/match.py +++ b/selene/core/match.py @@ -78,7 +78,7 @@ def __init__( self.__falsy_exceptions = _falsy_exceptions super().__init__( - description=( + ( f'{description}{" ignoring case:" if _ignore_case else ""} \'{expected}\'' # todo: refactor to and change tests correspondingly: # f'{" ignoring case:" if _ignore_case else ":"} «{expected}»' @@ -141,7 +141,7 @@ def compare(actual: Iterable) -> bool: ) super().__init__( - description=( + ( f'{description}{" ignoring case:" if _ignore_case else ""}' f' {list(expected)}' # todo: refactor to and change tests correspondingly: @@ -349,7 +349,7 @@ def __x_exact_text(expected: str | int | float, _ignore_case=False, _inverted=Fa def text(expected: str | int | float, _ignore_case=False, _inverted=False): return _ElementHasSomethingSupportingIgnoreCase( - 'has text', + 'has text', # TODO: is it here name or description? probably it's even a "name prefix" expected, actual=query.text, by=predicate.includes, @@ -578,7 +578,7 @@ def class_attribute_value(element: Element): return element.locate().get_attribute('class') return _ElementHasSomethingSupportingIgnoreCase( - f"has css class", + 'has css class', expected=name, actual=class_attribute_value, by=predicate.includes_word, @@ -775,68 +775,12 @@ def collection_has_size_less_than_or_equal( ) -class _CollectionHasTexts(Condition[Collection]): - - def __init__( - self, - *expected: str | int | float | Iterable[str], - _describing_matched_to='have texts', - _compared_by_predicate_to=predicate.equals_by_contains_to_list, - _ignore_case=False, - _inverted=False, - ): - self.__expected = expected - self.__describe_matched_to = _describing_matched_to - self.__compared_by_predicate_to = _compared_by_predicate_to - self.__ignore_case = _ignore_case - self.__inverted = _inverted - - # TODO: should we store flattened version in self? - # how should we render nested expected in error? - # should we transform actual to same un-flattened structure as expected? - # (when rendering, of course) - - def compare(actual: Iterable) -> bool: - expected_flattened = helpers.flatten(expected) - str_lower = lambda some: str(some).lower() - return ( - _compared_by_predicate_to(map(str_lower, expected_flattened))( - map(str_lower, actual) - ) - if _ignore_case - else _compared_by_predicate_to(map(str, expected_flattened))( - map(str, actual) - ) - ) - - super().__init__( # type: ignore - description=( - f'{_describing_matched_to}' - f'{" ignoring case:" if _ignore_case else ""} {expected}' - ), - actual=query.visible_texts, - by=compare, - _inverted=_inverted, - ) - - # returning Condition[Collection] to not allow .ignore_case.ignore_case usage:) - @property - def ignore_case(self) -> Condition[Collection]: - return self.__class__( - *self.__expected, - _describing_matched_to=self.__describe_matched_to, - _compared_by_predicate_to=self.__compared_by_predicate_to, - _ignore_case=True, - _inverted=self.__inverted, - ) - - # TODO: make it configurable whether assert only visible texts or not def texts( *expected: str | int | float | Iterable[str], _ignore_case=False, _inverted=False ): return _CollectionHasSomethingSupportingIgnoreCase( - f"have texts", + 'have texts', *expected, actual=query.visible_texts, by=predicate.equals_by_contains_to_list, @@ -849,7 +793,7 @@ def exact_texts( *expected: str | int | float | Iterable[str], _ignore_case=False, _inverted=False ): return _CollectionHasSomethingSupportingIgnoreCase( - f"have exact texts", + 'have exact texts', *expected, actual=query.visible_texts, by=predicate.equals_to_list,