2. - Serggi0/python-project-lvl3 GitHub Wiki

Привет!

https://github.com/Serggi0/python-project-lvl3/blob/232e11832dda624a2ac4c2a938907df6094d8a62/tests/test_.py#L123 В тестах лучше избегать взаимодействия со сторонними сервисами по сети. Эти сервисы не предоставляют нам никаких гарантий надежности и в любом момент могут упасть, сменить домен, начать долго отвечать и т.д. Это все негативно влияет на стабильность тестов и скорость работы. Судя по всему ты уже и сам с этим столкнулся, раз поставил try/except с игнором :) В данном случае лучше такой тест вообще убрать, т.к. он на самом деле не сильно полезнее, чем тест всей функции download с моками.

https://github.com/Serggi0/python-project-lvl3/blob/232e11832dda624a2ac4c2a938907df6094d8a62/tests/test_.py#L150 Не критично, но у requests_mock есть встроенная фикстура для pytest, с помощью которой можно не делать руками контекстные менеджеры. Посмотри в документации пожалуйста.

https://github.com/Serggi0/python-project-lvl3/blob/232e11832dda624a2ac4c2a938907df6094d8a62/tests/test_.py#L154 Еще хорошо бы ресурсы проверять тоже. Что сохранились в нужные места и содержимое правильное.

https://github.com/Serggi0/python-project-lvl3/blob/232e11832dda624a2ac4c2a938907df6094d8a62/tests/test_.py#L163 Кажется какой-то лишний отладочный вывод остался :)

https://github.com/Serggi0/python-project-lvl3/blob/232e11832dda624a2ac4c2a938907df6094d8a62/tests/test_.py#L188 Лучше конкретные типы исключений сделать и проверять их в pytest.raises явно. Полагаться на сообщения в ошибках лучше не стоит — мы их в любой момент можем поменять, добавить переводы на разные языки в зависимости от локали компьютера и т.д. А вот тип ошибки — это более или менее стабильная вещь, вокруг которой можно построить тестирование.

https://github.com/Serggi0/python-project-lvl3/blob/main/tests/test_.py Посмотри пожалуйста, все ли из unit тестов действительно нужны. Кажется большая часть из них дублируется в функциональных тестах на функцию download, а лишние тесты скорее мешают, чем помогают.

https://github.com/Serggi0/python-project-lvl3/blob/232e11832dda624a2ac4c2a938907df6094d8a62/page_loader/page_loader.py#L25 Лучше не заворачивать большой кусок кода в один большой try/except — непонятно, что за ошибку мы обрабатываем, в каком случае она возникает и т.д. Лучше по возможности обрабатывать ошибки максимально локально — близко к тем местам, где они возникают. Тогда проще по коду быстро разобраться, правильно ли обрабатывается ошибка или что-то не учтено.

https://github.com/Serggi0/python-project-lvl3/blob/232e11832dda624a2ac4c2a938907df6094d8a62/page_loader/page_loader.py#L17 Есть такой общий принцип — «лучше просить прощения, чем разрешения», который часто позволяет упростить код и сделать его более надежным. Пытаясь заранее «проверить» все ошибочные ситуации, мы все равно рискуем что-то не учесть и в итоге получить ошибку при выполнении целевого действия. При этом проблему можно решить очень просто — не проверять ничего заранее и просто попытаться выполнить действие — записать файл, создать директорию, выполнить сетевой запрос и т.д. И уже в моменте мы можем обработать любые ошибки, которые возникают. Тогда мы точно будем уверены, что все возможные ошибки обработаны корректно.

https://github.com/Serggi0/python-project-lvl3/blob/232e11832dda624a2ac4c2a938907df6094d8a62/page_loader/page_loader.py#L19 см. выше, нет смысла заранее делать какие-то проверки сетевой доступности страницы, можно просто попробовать скачать её и обработать ошибки.

https://github.com/Serggi0/python-project-lvl3/blob/232e11832dda624a2ac4c2a938907df6094d8a62/page_loader/web_data_processing.py#L57 Кажется sleep тут лишнее.

https://github.com/Serggi0/python-project-lvl3/blob/232e11832dda624a2ac4c2a938907df6094d8a62/page_loader/web_data_processing.py#L40 Снова лишний check.

https://github.com/Serggi0/python-project-lvl3/blob/232e11832dda624a2ac4c2a938907df6094d8a62/page_loader/normalize_data.py#L80 Тут в except всегда будет отрабатывать обработка OSError, т.к. она является базовой по отношению ко всем остальным. Чтобы обработать более конкретные ошибки отдельно, нужно расположить их в обратном порядке — от конкретным к общим.

https://github.com/Serggi0/python-project-lvl3/blob/232e11832dda624a2ac4c2a938907df6094d8a62/page_loader/page_loader.py#L22 Мы же уже скачали страницу в load_web_page, кажется еще раз её скачивать это лишнее.

https://github.com/Serggi0/python-project-lvl3/blob/232e11832dda624a2ac4c2a938907df6094d8a62/page_loader/web_data_processing.py#L102 В функции все еще смешана чистая функциональность (парсинг html страницы, выделение ресурсов и генерация путей для сохранения) и разные побочные эффекты (скачивание ресурсов, работа с файловой системой). Давай попробуем разделить это явно на этапы, чтобы не было так много всего. https://www.youtube.com/watch?v=_tPRetDsLj4

https://github.com/Serggi0/python-project-lvl3/blob/232e11832dda624a2ac4c2a938907df6094d8a62/page_loader/web_data_processing.py#L114 Кажется много копипасты в коде, хотя принципиально логика работы с разными видами ресурсов никак не отличается. Может быть попробуем как-то обобщить для всех ресурсов логику?