Feature/python testing homework #57
Conversation
There was a problem hiding this comment.
На мой взгляд, хорошие стандартные тесты, думаю, не много не пересекаются с представлением о том как должны они выглядить со стороны автора курса.
Из улучшений предложил был, все обьекты(модели, датаклассы запросов, ответов) представлять в виде фабрик с использованием фейк данных, это добавит гибкость к будущим тестам и спрячут не нужную логику из самих тестов.
Как указано в задании 2. Получить удовольствие! это самое главное :)
Жаль что не удалось(как понимаю, не хватило времени) поработать с таким строгим линтером
tests/conftest.py
Outdated
| @@ -8,7 +8,7 @@ | |||
|
|
|||
| pytest_plugins = [ | |||
| # Should be the first custom one: | |||
| 'plugins.django_settings', | |||
| # 'plugins.django_settings', | |||
There was a problem hiding this comment.
не очень понятно чем он помешал, по идее, он нужен что бы получить в общий список фикстур, фикстуры из этого файла
There was a problem hiding this comment.
Привет!
спасибо за ревью
я складывал в плагины, но у меня возник проблемс с импортами, решение их приводило к другой проблеме, которую я не мог фиксануть по бырому, отключение данного плагина спасло мою ситуацию
если у тебя есть в этом экспертиза и свободное время, был бы рад это обсудить в телеге!)
There was a problem hiding this comment.
не могу сказать что у меня есть экспертиза, но в целом, если скинешь подробное описание проблемы, по возможности, посмотрю и чем могу помогу :)
моя телега: как ник тут, только маленькими буквами)
There was a problem hiding this comment.
спасибо! я уже доковырялся))
| }) | ||
| return { | ||
| **schema.create()[0], | ||
| **{'password_first': password, 'password_second': password}, |
There was a problem hiding this comment.
могу ошибаться, но django потребует поля с именами password1 и password2
There was a problem hiding this comment.
у меня локально все мои тесты проходили, по идее значит ок)
There was a problem hiding this comment.
Интересно, а проверка User.check_password() есть?
|
|
||
|
|
||
| @pytest.fixture() | ||
| def registration_data_factory() -> RegistrationDataFactory: |
There was a problem hiding this comment.
NIT: фабрику лучше было бы положить в плагины, оттуда было бы удобнее переиспользовать в других файлах, так как они попадут в общий список фикстур
There was a problem hiding this comment.
да соглы, выше как раз проблему описал с этим, если подможешь буду признателен
tests/test_server/test_apps/test_identity/test_infrastructure/test_services/test_placeholder.py
Outdated
Show resolved
Hide resolved
| """Testing service that create lead user.""" | ||
| assert_correct_user(reg_data['email'], expected_user_data) | ||
| actual_id = 11 | ||
| actual_response = UserResponse(id=actual_id) |
There was a problem hiding this comment.
для UserResponse было бы хорошо так же реализовать фабрику и передавать как фикстуру, тем самым мы увеличим возможные вариации значений, удобство в переиспользовании и сокрытии подготовки данных внутри теста
There was a problem hiding this comment.
спасибо!
как раз еще хотел подпилить дальше тесты)
There was a problem hiding this comment.
Отличная работа! Я бы поработал с замечаниями линтера, возможно кажутся бесполезными, но иногда важно следовать строгому стайлгайду. На эту тему есть доклад Никиты и Григория Петрова, если есть время, советую глянуть
tests/test_server/test_apps/test_identity/test_infrastructure/test_services/test_placeholder.py
Outdated
Show resolved
Hide resolved
|
|
||
| def test_success_validate_user_response() -> None: | ||
| """Testing UserResponse model, success case.""" | ||
| expected_id = 1 |
There was a problem hiding this comment.
Может выносить 1 в переменную не имеет большого смысла. Будто бы строчка actual_id == 1, будет доносить эту же информацию
| """Testing usecase that create new user.""" | ||
| not_lead_user = User.objects.get(email=reg_data['email']) | ||
| assert not_lead_user.lead_id is None | ||
| assert_correct_user(reg_data['email'], expected_user_data) | ||
|
|
||
| UserCreateNew(settings=settings)(user=user) | ||
| lead_user = User.objects.get(email=reg_data['email']) | ||
| expected_lead_id = 11 | ||
| assert lead_user.lead_id == expected_lead_id |
There was a problem hiding this comment.
В этом тесте стадии (Arrange-Act-Assert) размазались
| """Testing usecase that update user id.""" | ||
| assert_correct_user(reg_data['email'], expected_user_data) | ||
| response = UserResponse(id=100) | ||
| UserCreateNew(settings=settings)._update_user_ids( # noqa: WPS437 |
There was a problem hiding this comment.
Можно ли здесь избежать использования приватного метода? Если нет, лучше указать причину рядом с noqa комментом
…date_user_response
|
Сорри за опоздание с ревью. Заработался. |
No description provided.