Skip to content

level_2 tests#4

Merged
uraniumcore238 merged 1 commit intomainfrom
level_two_fixes
Mar 19, 2024
Merged

level_2 tests#4
uraniumcore238 merged 1 commit intomainfrom
level_two_fixes

Conversation

@uraniumcore238
Copy link
Copy Markdown
Owner

No description provided.

Copy link
Copy Markdown

@vppuzakov vppuzakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 на самом деле уже очень хорошие тесты, вмерживай
💡 дальше погляди по комментам и в след ПР попробуй поправить

assert solve_square_equation(1.0, 0.1, 0.1) == (None, None)


def test__two_square_equation__no_square_coefficient():
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 тут можно было бы добавить к названию ожидаемое поведение - __return_one_root_when_no_square_coefficient

from functions.level_2.two_square_equation import solve_square_equation


def test__two_square_equation__descriminant_less_than_zero():
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 добавить к названию - found_no_root_when_descriminant_less_than_zero

assert solve_square_equation(0.0, 0.0, 0.1) == (None, None)


def test__two_square_equation__no_square_coefficient_and_no():
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 тут явно про название забыли поправить

assert solve_square_equation(0.1, 5.1, 0.1) == (-50.980384612481814, -0.019615387518183702)


def test__two_square_equation__descriminant_is_zero(): #
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 тут дискриминант меньше нуля, а тест такой уже есть у нас - первый в этом модуле - поэтому раз один кейс - давай объединим эти два входа с помощью parametrize

@@ -0,0 +1,21 @@
from functions.level_2.two_square_equation import solve_square_equation
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 ну в целом то отличные и понятные тесты, но не хватает поведения ожидаемого - оно более важно, чем вход даже. Вход лишь дополнение к поведению, которое может и отсутствовать

do_something_when_something

('one two three four five six', '', ''),
('one two three four five six', ' ', ' ')
])
def test__five_replace_word__no_replacement_if_no_replace_from_in_the_text(text, replace_from, replace_to):
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 тут все отлично и название и параметры

('one two three four five six', 'one', ''),
('one two three four five six', 'one', ' ')
])
def test__five_replace_word__replacement_if_replace_from_is_in_the_text(text, replace_from, replace_to):
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 отличный тест
💡 поведение - глагол (не может быть существительным replacement) - используй replace, return_replaced, change_

('one two three four five six', 'one', ' ')
])
def test__five_replace_word__replacement_if_replace_from_is_in_the_text(text, replace_from, replace_to):
removed_first_word = text.split(' ', 1)[1]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 вот подготовка к тесту опасная - делает тест чуть более запутанней, может нам лучше иметь expected параметр как раз тут и не кодить в тесте - тест станет гораздо понятнее - давай так и сделаем

assert replace_word(text, replace_from, replace_to) == f"{replace_to} {removed_first_word}"


def test__five_replace_word__replacement_without_changes_if_the_same_words_in_all_parameters():
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 тест супер во всем, понятное название, понятное тело - параметры реально ни к чему

('one two three four five six', 'ONE', ' '),
('ONE two three four five six', 'one', 'seven'),
])
def test__five_replace_word__replacement_if_replace_from_is_in_the_text_ignore_case(text, replace_from, replace_to):
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 отличный тест
💡 давай вместо жонглирования текстом внутри теста добавим expected параметр и тест станет гораздо явнее (зачем нам там код - кто будет тестить наши тесты тогда?)

@uraniumcore238 uraniumcore238 merged commit c8cb9b0 into main Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants