Add test vol. 2 #4
Conversation
test_code.py
Outdated
| ('ebay', 2021), | ||
| ] | ||
| ) | ||
| def test_set_listed_at(mocker, marketplace_value, expected): |
There was a problem hiding this comment.
suggestion (non-blocking): Думаю стоит добавить случай, когда вещь в итоге не залистилась. когда в нашей функции отрабатывается случай когда if hasattr(item, listed_at_field_name): item не будет иметь атрибута
There was a problem hiding this comment.
Я добавил попозже на проверку None атрибут.
code_2.py
Outdated
| else: | ||
| raise ColumnError(f'Unable to convert to date {value}.') | ||
| if self.user_timezone: | ||
| print(self.user_timezone) |
There was a problem hiding this comment.
nitpick: Кажется добавление в исходные функции что мы тестируем это плохая идея. Особенно принтов
There was a problem hiding this comment.
Забыл убрать отладочный принт. Убираю
test_code.py
Outdated
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| 'path, isdir, expected', |
There was a problem hiding this comment.
nitpick: нейминг, вроде isdir надо бы разделять слова is_dir
| mocker.patch('code_2.Path.glob', return_value=path) | ||
| mocker.patch('code_2.os.path.isdir', return_value=isdir) | ||
|
|
||
| assert get_all_filepathes_recursively('path', 'py') == expected |
There was a problem hiding this comment.
suggestion: формат файлов можно попробовать вынести в parametrize и добавить файлы разных расширений
There was a problem hiding this comment.
Я тебе писал в ревью, в параметизе мы выносим только изменяющиеся данные. Если данные не меняются, то мы их прописываем в тесте. Логика функции такова, что изменение расширения файла не изменяет ее. Соответственно нет смысла добавлять разные расширения файлов
| @@ -0,0 +1,197 @@ | |||
| import datetime | |||
There was a problem hiding this comment.
suggestion: Тесты с conftest лучше хранить в отдельной папке, чтобы они не мешались и легче их было находить.
| ('ebay', 2021), | ||
| ] | ||
| ('ebay', 'ebay_listed_at'), | ||
| ('etsy', None), |
There was a problem hiding this comment.
praise: молодец! я зря сделал замечание, что отсутствует данный кэйс
.flake8
Outdated
| [flake8] | ||
| max-line-length = 120 | ||
| exclude = venv, code_2.py, code.py | ||
| ignore = C812, D103, TAE001, D100, D101 |
There was a problem hiding this comment.
question: Почему мы добавляем столько исключений?
There was a problem hiding this comment.
Из-за пачки flake8 расширений, которые я добавил из рабочего проекта.
- Пофиксил исключение C812 - thralling commas
- D103 Missing docstring in public function. - оставил исключение
- TAE001 - убрал из проекта, так как проверяю только тестовый файл, а в нем не нужны аннотации
- D100 — Missing docstring in public module - - оставил исключение
- D101 - Missing docstring in public class. - убрал за ненадобностью
| requests-mock==1.9.3 | ||
| pytest-mock==3.6.1 | ||
|
|
||
| flake8==3.9.2 |
There was a problem hiding this comment.
nitpick: излишнее по флэйку. разве не достаточно только одного flake8==3.9.2?
There was a problem hiding this comment.
Я добавил расширения из рабочего проекта. Единственное что, сейчас убрал расширешиние с аннотациями, тк я в тестовом файле нигде не использую
requirements.txt
Outdated
| flake8-absolute-import==1.0 | ||
| flake8-simplify==0.13.0 | ||
| flake8-functions-names==0.0.5 | ||
| mypy-extensions==0.4.3 |
There was a problem hiding this comment.
question: добавил, но вроде нигде не используешь при проверке
There was a problem hiding this comment.
Да, верно. Добавил Makefile в проект
DmitryTokyo
force-pushed
the
master
branch
4 times, most recently
from
e0651ef to
5807e7f
Compare
2 participants
No description provided.