Ревью

[eliizaveta]

А бейзпейджу точно нужен тулбар? https://github.com/Anilochka/technopolis-autotesting/blob/bccd061f51369fcab1b17a1f379184e253125f83/src/java/main/pages/BasePage.java#L13

visible.because("Некорректные данные пользователя, не удалось выполнить вход") https://github.com/Anilochka/technopolis-autotesting/blob/bccd061f51369fcab1b17a1f379184e253125f83/src/java/main/pages/LoginPage.java#L55

Нужно ли это отдельным методом https://github.com/Anilochka/technopolis-autotesting/blob/bccd061f51369fcab1b17a1f379184e253125f83/src/java/main/pages/LoginPage.java#L38

Есть желание разбить на более маленькие шаги, может быть сделать метод получения последнего сообщения, а тут уже lastMessage = getLastMessage(); и дальше работать с переменной lastMessage https://github.com/Anilochka/technopolis-autotesting/blob/bccd061f51369fcab1b17a1f379184e253125f83/src/java/main/pages/MessagePage.java#L89

Зачем шаг входа в рекомендации? https://github.com/Anilochka/technopolis-autotesting/blob/bccd061f51369fcab1b17a1f379184e253125f83/src/java/main/pages/MusicPage.java#L122-L123

Создание страницы логина и выход лучше для каждого теста делать в Before After в тесте, а не в общем BaseTest, тк мы можем захотеть написать тест, где не нужен вход, например, на регистрацию

Один тест = один класс (кроме @ Nested)

Добавить fail() после логина на случай, что удалось войти, а не должно https://github.com/Anilochka/technopolis-autotesting/blob/bccd061f51369fcab1b17a1f379184e253125f83/src/java/tests/LoginTest.java#L27-L32 Можно делать проверку на эксепшн с помощью assertThat("", login, throwsExeption(Exeption.class, equal()))

Тут тоже можно использовать логгер https://github.com/Anilochka/technopolis-autotesting/blob/bccd061f51369fcab1b17a1f379184e253125f83/src/java/tests/MessageReplyTest.java#L51

Почему удаляем дважды с обновлением стр? https://github.com/Anilochka/technopolis-autotesting/blob/bccd061f51369fcab1b17a1f379184e253125f83/src/java/tests/MessageReplyTest.java#L52-L54

Доп к ревью #16 :)

[AlexanderKiselyov]

Исправили недочеты Насчет последнего комментария по поводу удаления сообщения дважды с обновлением страницы: тест заключался в пересылке сообщения и проверки, что оно переслалось. В BeforeEach подготовилось и отправилось сообщение. Сам тест заключался в пересылке этого сообщения тому же получателю. Далее в AfterEach проводилась зачистка сообщений: удалялось сначала пересланное сообщение, а затем само отправленное сообщение. При удалении пересланного сообщения, оно становилось hidden и если не обновлять страницу, то получаем ошибку на проверке shouldBe(visible.because("No messages found!")). Обновляя страницу, происходит удаление пересланного сообщения из списка сообщений в диалоге и далее можно уже удалить изначально отправленное сообщение.

[eliizaveta]

Исправили недочеты Насчет последнего комментария по поводу удаления сообщения дважды с обновлением страницы: тест заключался в пересылке сообщения и проверки, что оно переслалось. В BeforeEach подготовилось и отправилось сообщение. Сам тест заключался в пересылке этого сообщения тому же получателю. Далее в AfterEach проводилась зачистка сообщений: удалялось сначала пересланное сообщение, а затем само отправленное сообщение. При удалении пересланного сообщения, оно становилось hidden и если не обновлять страницу, то получаем ошибку на проверке shouldBe(visible.because("No messages found!")). Обновляя страницу, происходит удаление пересланного сообщения из списка сообщений в диалоге и далее можно уже удалить изначально отправленное сообщение.

Тогда лучше сделать метод, который бы удалял сообщения по индексу в массиве сообщений, например, потому что такой подход кажется немного костыльным

[eliizaveta]

мы проверяем тестом появление конкретной ошибки, поэтому лучше сравнивать, что полученная ошибка совпадает с ожидаемой, после логина добавить fail(), на случай, если тест пройдет, а не должен https://github.com/Anilochka/technopolis-autotesting/blob/38e16dec66a70165efd603a333cf88da07532b38/src/java/tests/LoginTest.java#L30-L35

[eliizaveta]

убрать в BaseTest https://github.com/Anilochka/technopolis-autotesting/blob/38e16dec66a70165efd603a333cf88da07532b38/src/java/main/pages/LoginPage.java#L36

убрать в isLoaded() (в других пейджах тоже перенести похожие проверки) https://github.com/Anilochka/technopolis-autotesting/blob/38e16dec66a70165efd603a333cf88da07532b38/src/java/main/pages/MainPage.java#L23-L25

метод load() можно просто оставить пустым в BasePage и забыть про него, а где нужно тоже переписывать

остальное 👍🏻

[Anilochka]

Поправили) Только остался один момент по поводу fail() после логина: у нас смешанные данные (валидные и невалидные), и если мы вставим fail(), мы отловим ложно-положительное срабатывание теста, но на валидных данных тест будет валиться, хотя не должен, и, как нам кажется, этот подход нам не совсем подходит

[eliizaveta]

Поправили) Только остался один момент по поводу fail() после логина: у нас смешанные данные (валидные и невалидные), и если мы вставим fail(), мы отловим ложно-положительное срабатывание теста, но на валидных данных тест будет валиться, хотя не должен, и, как нам кажется, этот подход нам не совсем подходит

поняла, тогда вы можете оставить один тест на валидный логин (без try catch) и parametrized тест для невалидных, добавив еще вариантов невалидных логинов

[Anilochka]

Сделано)

[eliizaveta]

Сделано)

в тесте на логин еще можно сделать проверку, что имя пользователя на странице совпадает с ожидаемым, ну это для полноты теста

сообщение об ошибке можно писать сразу в fail https://github.com/Anilochka/technopolis-autotesting/blob/de8cdf7db6fa63b8e3fbb0ebb7ac97ff1da52f5c/src/java/tests/InvalidLoginTest.java#L38

в ассертах сообщения пишите кстати, помогает при падении понять проблему https://github.com/Anilochka/technopolis-autotesting/blob/de8cdf7db6fa63b8e3fbb0ebb7ac97ff1da52f5c/src/java/tests/MessageReplyTest.java#L49

мне кажется эта конструкция сразу возвращает true/false, можно просто return $().text().equals() https://github.com/Anilochka/technopolis-autotesting/blob/de8cdf7db6fa63b8e3fbb0ebb7ac97ff1da52f5c/src/java/main/pages/MusicPage.java#L120

закрываю ишью как решенное :) если будут какие-то вопросы пишите в тамтам, я на связи ✌️