This repository has been archived by the owner on Mar 19, 2024. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Feature/python testing homework #57
base: master
Are you sure you want to change the base?
Feature/python testing homework #57
Changes from 39 commits
91b3d52
63829d4
a7bb432
98b438d
1b84666
2d45717
0693c44
3ae8d67
941e2f4
f8fcbc4
873ed29
577859c
2c77850
f2cfe57
21330d0
75f0952
0f446e9
188743d
89faa6e
e815cd0
e970b74
7f513e9
b5f96f1
0337705
889f180
b20eb6a
3f7952d
764029b
49ecefb
747684d
f8f5bb9
e07740b
257a5a9
ff273a3
7946c12
fa37aab
d66bc12
abe1699
8e7449b
c72efe1
94e3ca8
d94f101
8d1e7fc
c2f0b81
42b901a
a140df8
613da1e
f522e09
dfd0357
6143d15
4c32e52
5606ac5
f842aa0
6c02234
6e8e430
4f243a0
9958c85
1c25f00
941971d
949c222
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
не очень понятно чем он помешал, по идее, он нужен что бы получить в общий список фикстур, фикстуры из этого файла
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Привет!
спасибо за ревью
я складывал в плагины, но у меня возник проблемс с импортами, решение их приводило к другой проблеме, которую я не мог фиксануть по бырому, отключение данного плагина спасло мою ситуацию
если у тебя есть в этом экспертиза и свободное время, был бы рад это обсудить в телеге!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
не могу сказать что у меня есть экспертиза, но в целом, если скинешь подробное описание проблемы, по возможности, посмотрю и чем могу помогу :)
моя телега: как ник тут, только маленькими буквами)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
спасибо! я уже доковырялся))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: фабрику лучше было бы положить в плагины, оттуда было бы удобнее переиспользовать в других файлах, так как они попадут в общий список фикстур
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
да соглы, выше как раз проблему описал с этим, если подможешь буду признателен
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
могу ошибаться, но django потребует поля с именами
password1
иpassword2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
у меня локально все мои тесты проходили, по идее значит ок)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Интересно, а проверка
User.check_password()
есть?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
для
UserResponse
было бы хорошо так же реализовать фабрику и передавать как фикстуру, тем самым мы увеличим возможные вариации значений, удобство в переиспользовании и сокрытии подготовки данных внутри тестаThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
спасибо!
как раз еще хотел подпилить дальше тесты)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Может выносить 1 в переменную не имеет большого смысла. Будто бы строчка
actual_id == 1
, будет доносить эту же информациюThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В этом тесте стадии (Arrange-Act-Assert) размазались
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Можно ли здесь избежать использования приватного метода? Если нет, лучше указать причину рядом с
noqa
комментом