Conversation
demologin
left a comment
demologin
left a comment
There was a problem hiding this comment.
Общий вывод по проекту
Проект демонстрирует хорошее владение инструментами тестирования (JUnit 5, Mockito) и понимание структуры веб-приложения на сервлетах. Код тестов структурирован, используются базовые классы для общих настроек, что свидетельствует о понимании принципов DRY.
Однако, есть замечания по части обработки исключений и эффективности работы с данными в сервисах (StatisticService). Множественные проходы по коллекциям и отсутствие валидации входных данных в контроллерах — это те зоны роста, которые отделяют текущий код от уровня Production-ready. Также стоит обратить внимание на безопасность (хардкод паролей) и архитектурную чистоту (не смешивать логику и контроллеры).
Проделанная работа впечатляет объемом написанных тестов. Продолжайте в том же духе, уделяя больше внимания деталям реализации и алгоритмической сложности!
Итоговая оценка: A (за тесты - их много ;)
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.api.extension.ExtendWith; | ||
| import org.mockito.InjectMocks; | ||
| import org.mockito.Mockito.*; |
There was a problem hiding this comment.
Использование 'import org.mockito.Mockito.*;' совместно со статическим импортом тех же методов на строке 28 создает избыточность. Рекомендуется использовать только статические импорты для улучшения читаемости тестов. [INFO]
|
|
||
| @FieldDefaults(level = AccessLevel.PRIVATE) | ||
| @ExtendWith(MockitoExtension.class) | ||
| class DeleteQuestTest extends BaseTest { |
There was a problem hiding this comment.
Класс объявлен с доступом по умолчанию (package-private). В JUnit 5 это допустимо, однако стоит придерживаться единого стиля во всем проекте. Если большинство классов публичные, стоит сделать и этот public. [INFO]
| @Mock | ||
| QuestService questService; | ||
| @InjectMocks | ||
| DeleteQuest servlet; |
There was a problem hiding this comment.
Поле 'servlet' не имеет модификатора доступа. Рекомендуется явно указывать модификатор доступа (например, private) для соблюдения принципов инкапсуляции. [WARNING]
| @Test | ||
| @Tag("http-client") | ||
| @DisplayName("When open delete quest page then body contains close tag") | ||
| void whenOpenDeleteQuestPageThenBodyContainsSeTagIT() throws IOException, InterruptedException { |
There was a problem hiding this comment.
Опечатка в названии метода: 'should...ContainsSeTagIT' вместо 'CloseTag'. Также суффикс 'IT' обычно зарезервирован для интеграционных тестов, запускаемых FailSafe плагином. Если это юнит-тест, лучше переименовать. [INFO]
| import java.util.Arrays; | ||
|
|
||
| public abstract class BaseTest { | ||
| public static final String ROOT = "http://localhost:8088"; |
There was a problem hiding this comment.
Хардкод URL 'http://localhost:8088'. Рекомендуется выносить конфигурационные параметры в .properties или .yaml файлы, чтобы тесты могли запускаться в разных окружениях (CI/CD) без изменения кода. [WARNING]
| verify(request).setAttribute("quest", quest); | ||
| verify(request).getRequestDispatcher(JSP_PATH); | ||
| verify(requestDispatcher).forward(request, response); | ||
| verifyNoMoreInteractions(response); |
There was a problem hiding this comment.
Использование 'verifyNoMoreInteractions(response)' может быть слишком строгим и приводить к падению тестов при незначительных изменениях в коде. Рекомендуется использовать только когда это критично для логики. [INFO]
| @InjectMocks | ||
| private EditUser servlet; | ||
|
|
||
| private final Long TEST_USER_ID = 123L; |
There was a problem hiding this comment.
Поле TEST_USER_ID помечено как final, но не static. Константы уровня класса принято делать 'static final' и именовать в UPPER_CASE. [INFO]
| import static org.mockito.Mockito.*; | ||
| import static org.junit.jupiter.api.Assertions.*; | ||
|
|
||
| @FieldDefaults(level = AccessLevel.PRIVATE) |
There was a problem hiding this comment.
Использование @FieldDefaults(level = AccessLevel.PRIVATE) — хорошее решение для сокращения кода, но оно должно применяться единообразно во всем проекте. [INFO]
| mapper = new ObjectMapper(); | ||
| httpClient = HttpClient.newBuilder() | ||
| .cookieHandler(cookieManager) | ||
| .version(HttpClient.Version.HTTP_1_1) |
There was a problem hiding this comment.
Указание версии HTTP/1.1 вручную. Современный HttpClient поддерживает HTTP/2 по умолчанию. Если нет специфических ограничений сервера, лучше оставить выбор версии на усмотрение клиента. [INFO]
| @Test | ||
| @Tag("http-client") | ||
| @DisplayName("When open create game users page then body contains se tag") | ||
| void whenOpenCreateGamePageThenBodyContainsSeTag() throws IOException, InterruptedException { |
There was a problem hiding this comment.
В названии теста 'whenOpenCreateGamePageThenBodyContainsSeTag' есть слово 'users', которого нет в логике. Это сбивает с толку. [INFO]
3 participants
No description provided.