Skip to content

HomeWork solution#9

Open
agarkovand wants to merge 11 commits intodmdev2020:homeworkfrom
agarkovand:hw_solution
Open

HomeWork solution#9
agarkovand wants to merge 11 commits intodmdev2020:homeworkfrom
agarkovand:hw_solution

Conversation

@agarkovand
Copy link
Copy Markdown

No description provided.

<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-junit-jupiter</artifactId>
<version>4.11.0</version>
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а не должен совпадать с junit 5 версией?


class SubscriptionDaoIT extends IntegrationTestBase {

private final SubscriptionDao subDao = SubscriptionDao.getInstance();
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Лучше использовать общепринятые сокращения в именовании всего в Java (sub).

Subscription sub3 = subDao.insert(getSubscription("User 3", 3));

List<Subscription> actualSub = subDao.findAll();
List<Integer> userIds = actualSub.stream().map(Subscription::getUserId).toList();
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

разделяй четко 3 секции в тесте, чтобы улучшить его читабильность: given/when/then

@Test
void whenSubExists_thenItIsDeleted() {
Subscription savedSub = subDao.insert(getSubscription("User 1", 1));
boolean isDeleted = subDao.delete(savedSub.getId());
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is* больше относится к именованию методов, а не локальных переменных
для переменных не нужны эти префиксы

void findByUserId() {
Subscription expectedSub = subDao.insert(getSubscription("User 1", 1));

List<Subscription> actualSubs = subDao.findByUserId(expectedSub.getUserId());
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Для теста метода findByUserId нужна вставить несколько подписок с разным userId - иначе некорректный dataset. Его можно заменить на findAll, например, и он все еще будет проходить

@Mock
private CreateSubscriptionValidator createSubValidator;
@Mock
private Clock clock = Clock.fixed(Instant.now(), ZoneId.systemDefault());
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

это не мок

private SubscriptionService subService; // );

@Test
void whenSubscriptionExists_thenUpdateAndInsert() {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

не может сразу и обновить и вставить метод
Плохое название тестового метода - не говорит что именно тестируется в нем

void whenSubIsActive_thenExceptionThrown() {
doReturn(Optional.of(Subscription.builder().status(Status.EXPIRED).build()))
.when(subDao).findById(anyInt());
assertThrows(SubscriptionException.class, () -> subService.cancel(anyInt()));
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Нужно проверить какое именно исключение упало, например, код/текст исключения и т.д. Иначе причина может поменяться, а тест будет проходить

import java.time.format.DateTimeFormatter;
import java.util.Locale;

public class DateUtil {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

если это утилитный класс - то final класс и private консторуктор

assertThat(actualResult.hasErrors()).isTrue();
assertThat(actualResult.getErrors().size()).isEqualTo(1);
assertThat(actualResult.getErrors().get(0).getCode()).isEqualTo(102);
assertThat(actualResult.getErrors().get(0).getMessage()).isEqualTo("provider is invalid");
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

можно просто получить объект ошибки и проверять объект, вместо множества ассертов

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants