Skip to content

Commit

Permalink
[#530] REFACTOR: disable description as keyword arg...
Browse files Browse the repository at this point in the history 
... 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.
  • Loading branch information
@yashaka
yashaka committed Jul 8, 2024
1 parent 1aac3b9 commit d1da03f
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 88 deletions.
59 changes: 54 additions & 5 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion selene/common/_typing_functions.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
55 changes: 36 additions & 19 deletions selene/core/condition.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
)
)
```
Expand All @@ -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,
)
Expand All @@ -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,
)
```
Expand Down Expand Up @@ -781,6 +781,7 @@ def raise_if_not_actual(
def __init__(
self,
description: str | Callable[[], str],
/,
test: Lambda[E, None],
*,
_inverted=False,
Expand All @@ -800,6 +801,7 @@ def __init__(
def __init__(
self,
description: str | Callable[[], str],
/,
*,
actual: Lambda[E, R],
by: Predicate[R],
Expand All @@ -812,6 +814,7 @@ def __init__(
def __init__(
self,
description: str | Callable[[], str],
/,
*,
by: Predicate[E],
_inverted=False,
Expand All @@ -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,
Expand Down Expand Up @@ -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):
"""
Expand Down Expand Up @@ -1475,6 +1490,7 @@ def __init__x(self, *args, **kwargs):
def __init__(
self,
description: str | Callable[[], str],
/,
actual: Lambda[E, R],
*,
by: Predicate[R],
Expand All @@ -1487,6 +1503,7 @@ def __init__(
def __init__(
self,
description: str | Callable[[], str],
/,
*,
by: Predicate[E],
_inverted=False,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
# )

Expand Down
2 changes: 1 addition & 1 deletion selene/core/exceptions.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
68 changes: 6 additions & 62 deletions selene/core/match.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}»'
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down

0 comments on commit d1da03f

Please sign in to comment.