Conversation
vppuzakov
left a comment
There was a problem hiding this comment.
👍 Большой вышел ПР, далеко продвинулись в целом и parametrize написан и фикстуры уже появились - поэтому давай смержим.
А уже в следующих ПР давай поработаем над комментами.
В основном по комментам:
- параметрайз применять только там, где поведение тестируемое одно, но хочется докидать еще несколько входных параметров для уверенности
- во всех тестах с expenses очень трудно разобраться без использования фикстуры-фабрики make_expense - сделай ее первым делом - пойдет полегче. Это касается и тех тестов, где expense в параметрайз и тех где в теле теста.
- тестируемое поведение всегда отражай в названии теста
- не увлекайся с аннотациями типов в тестах - редко несут пользу
|
|
||
|
|
||
| def test__solve_square_equation__if_square_coefficient_is_equal_to_zero(): | ||
| def test__solve_square_equation__return_one_root_if_square_coefficient_is_equal_to_zero(): |
There was a problem hiding this comment.
👍 хорошо, что сохранил старый вариант тестов без параметрайза и полностью с параметрайзом.
Хотя тут можно попробовать найти золотую середину, когда хорошо иметь и отдельные тесты на разные поведения, а когда в рамках теста одного поведения мы хотим проверить несколько кейсов - на нескольких входных параметрах, используя parametrize
|
|
||
|
|
||
| @pytest.mark.xfail(reason='Incorrect work of function') | ||
| def test__is_subscription_if_expense_occurs_in_same_month(expense, history): |
There was a problem hiding this comment.
💡 предлагаю название с поведением: test__is_subscription__no_subscription_when_expense_occurs_in_same_month
|
|
||
|
|
||
| @pytest.mark.xfail(reason='Incorrect work of function') | ||
| def test__is_subscription_if_expense_occurs_in_same_month(expense, history): |
There was a problem hiding this comment.
history как соотносится с созданной внутри нашего теста expense_updated - неясно.
либо не хватает корректного названия у фикстуры, либо сама фикстура недостаточно универсальна, хотя и имеет неконкретизированное название. Либо она вообще не нужна и список фикстур можно создать прямо в тесте.
Условно:
# тут можем не задавать явно spent_in='Megafon' так как нам не важно,
# мегафон ли это, нам важно только чтобы были одинаковые.
# Такая же история с датами - но мы не имеем пока такой удобной фикстуры
prev_expense = make_expenses(spent_at=datetime(2023, 6, 1))
history = [prev_expense]
expense = make_expenses(
# можно заодно использовать значение предыдущего - сразу понятно,
# что в тесте важно, чтобы они были одинаковы
spent_in=prev_expense.spent_in,
# при наличии удобной фикстуры с датами тоже
# можно было придумать что-то похожее
spent_at=datetime(2023, 6, 10),
)
...| ) | ||
| assert is_subscription(expense_updated, history) == False | ||
|
|
||
| def test__is_subscription_if_expense_occurs_less_than_3_times(expense:list[Expense], history:list[list[Expense]])->bool: |
There was a problem hiding this comment.
👍 кейс хороший
💡 аннотиции в тестах зачастую только мешают восприятию теста
Сделал тесты level_1_7. Сломал голову на four_fraud.py. Нашел явную ошибку в функции, исправил. Но все равно тесты на нее работают некорректно. Думаю функция не возвращает нужный нам список. Могу ошибаться. По остальным думаю все ок.