Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BE] test: 도메인 연관관계 재설정 후 테스트 작성 #101

Merged
merged 14 commits into from
Jul 25, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions backend/src/main/java/reviewme/member/domain/GithubId.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
import jakarta.persistence.Column;
import jakarta.persistence.Embeddable;
import lombok.AccessLevel;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Embeddable
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@EqualsAndHashCode(of = "id")
@Getter
public class GithubId {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@
import jakarta.persistence.Id;
import jakarta.persistence.ManyToOne;
import lombok.AccessLevel;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Entity
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@EqualsAndHashCode(of = "id")
@Getter
public class GithubIdReviewerGroup {

Expand Down
25 changes: 23 additions & 2 deletions backend/src/main/java/reviewme/member/domain/Member.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,13 @@
import jakarta.persistence.GenerationType;
import jakarta.persistence.Id;
import jakarta.persistence.Table;
import java.util.Objects;
import lombok.AccessLevel;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Entity
@Table(name = "member")
@EqualsAndHashCode(of = "id")
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@Getter
public class Member {
Expand All @@ -33,4 +32,26 @@ public Member(String name, long githubId) {
this.name = name;
this.githubId = new GithubId(githubId);
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (!(o instanceof Member member)) {
return false;
}
if (id == null) {
return Objects.equals(githubId, member.githubId);
}
return Objects.equals(id, member.id);
}

@Override
public int hashCode() {
if (id == null) {
return Objects.hash(githubId);
}
return Objects.hash(id);
}
Comment on lines +36 to +56
Copy link
Contributor

Choose a reason for hiding this comment

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

갑자기 든 생각 :
이런 함수 키워드에도 있었던 것 같은데 @EqualsAndHashCode(of = "id")로 해도 괜찮을 것 같아요, 아이디가 어떤건 null 이고, 다른건 null 이 아니면 다르다고 판단하는게 맞지 않을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

도메인 테스트에서 ID가 존재하지 않는 경우, 같다고 판단하게 됩니다. githubId가 다른 경우 다른 사람으로 진행되어야 합니다 !

Copy link
Contributor

@nayonsoso nayonsoso Jul 25, 2024

Choose a reason for hiding this comment

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

이해완!
확실히 도메인 테스트에서는 필요하겠네요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

다른 말로 하면 도메인 테스트에 어울리지 않는 로직이 도메인에 들어왔다고도 이해할 수 있겠습니다. 다음에 서비스로 올려보아요

}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import lombok.AccessLevel;
import lombok.Getter;
import lombok.NoArgsConstructor;
import reviewme.member.domain.exception.DescriptionLengthExceededException;
import reviewme.member.domain.exception.InvalidDescriptionLengthException;
import reviewme.member.domain.exception.InvalidGroupNameLengthException;
import reviewme.member.domain.exception.SelfReviewException;
import reviewme.review.domain.Review;
Expand Down Expand Up @@ -68,7 +68,7 @@ public ReviewerGroup(Member reviewee, List<GithubId> reviewerGithubIds,
throw new InvalidGroupNameLengthException(MAX_GROUP_NAME_LENGTH);
}
if (description.length() > MAX_DESCRIPTION_LENGTH) {
throw new DescriptionLengthExceededException(MAX_DESCRIPTION_LENGTH);
throw new InvalidDescriptionLengthException(MAX_DESCRIPTION_LENGTH);
}
if (reviewerGithubIds.contains(reviewee.getGithubId())) {
throw new SelfReviewException();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

import reviewme.global.exception.BadRequestException;

public class DescriptionLengthExceededException extends BadRequestException {
public class InvalidDescriptionLengthException extends BadRequestException {

public DescriptionLengthExceededException(int maxLength) {
public InvalidDescriptionLengthException(int maxLength) {
super("리뷰어 그룹 설명은 %d자 이하로 작성해야 합니다.".formatted(maxLength));
}
}
2 changes: 2 additions & 0 deletions backend/src/test/java/reviewme/fixture/MemberFixture.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ public enum MemberFixture {

회원_산초("산초", 1L),
회원_아루("아루", 2L),
회원_커비("커비", 3L),
회원_테드("테드", 4L),
;

private final String name;
Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

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

오오 이거 알잘딱깔센 픽스처네요👍

Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package reviewme.fixture;

import java.time.LocalDateTime;
import java.util.List;
import lombok.Getter;
import lombok.RequiredArgsConstructor;
import reviewme.member.domain.GithubId;
import reviewme.member.domain.Member;
import reviewme.member.domain.ReviewerGroup;

@RequiredArgsConstructor
@Getter
public enum ReviewerGroupFixture {

데드라인_남은_그룹("데드라인 이전 그룹", "설명", LocalDateTime.now().plusDays(1)),
데드라인_지난_그룹("데드라인 지난 그룹", "설명", LocalDateTime.now().minusDays(1)),
;

private final String groupName;
private final String description;
private final LocalDateTime deadline;

public ReviewerGroup create(Member reviewee, List<GithubId> reviewerGithubIds) {
return new ReviewerGroup(reviewee, reviewerGithubIds, groupName, description, deadline);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,49 +2,120 @@

import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static reviewme.fixture.MemberFixture.회원_산초;
import static reviewme.fixture.MemberFixture.회원_커비;

import java.time.LocalDateTime;
import java.util.List;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import reviewme.member.domain.exception.DescriptionLengthExceededException;
import reviewme.member.domain.exception.DuplicateReviewerException;
import reviewme.member.domain.exception.EmptyReviewerException;
import reviewme.member.domain.exception.InvalidDescriptionLengthException;
import reviewme.member.domain.exception.InvalidGroupNameLengthException;
import reviewme.member.domain.exception.SelfReviewException;

class ReviewerGroupTest {

@Test
void 리뷰_그룹이_올바르게_생성된다() {
// given
Member sancho = new Member("산초", 1);
Member reviewee = 회원_산초.create();
String groupName = "a".repeat(100);
String description = "a".repeat(50);
LocalDateTime createdAt = LocalDateTime.of(2024, 7, 17, 12, 0);
LocalDateTime deadline = LocalDateTime.now().plusDays(1);
List<GithubId> reviewerGithubIds = List.of(new GithubId(3));

// when, then
assertDoesNotThrow(() -> new ReviewerGroup(sancho, List.of(new GithubId(3)), groupName, description, createdAt));
assertDoesNotThrow(
() -> new ReviewerGroup(reviewee, reviewerGithubIds, groupName, description, deadline));
}

@ParameterizedTest
@ValueSource(ints = {0, 101})
void 리뷰_그룹_이름_길이_제한을_벗어나는_경우_예외를_발생한다(int length) {
void 리뷰_그룹_이름_길이_제한을_벗어날_수_없다(int length) {
// given
String groupName = "a".repeat(length);
Member sancho = new Member("산초", 1);
LocalDateTime createdAt = LocalDateTime.of(2024, 7, 17, 12, 0);
Member reviewee = 회원_산초.create();
LocalDateTime deadline = LocalDateTime.now().plusDays(1);
List<GithubId> reviewerGithubIds = List.of();

// when, then
assertThatThrownBy(() -> new ReviewerGroup(sancho, List.of(), groupName, "설명", createdAt))
assertThatThrownBy(() -> new ReviewerGroup(reviewee, reviewerGithubIds, groupName, "설명", deadline))
.isInstanceOf(InvalidGroupNameLengthException.class);
}

@Test
void 리뷰_그룹_설명_길이_제한을_벗어나는_경우_예외를_발생한다() {
void 리뷰_그룹_설명_길이_제한을_벗어날_수_없다() {
// given
String description = "a".repeat(51);
Member sancho = new Member("산초", 1);
LocalDateTime createdAt = LocalDateTime.of(2024, 7, 17, 12, 0);
Member reviewee = 회원_산초.create();
LocalDateTime deadline = LocalDateTime.now().plusDays(1);
List<GithubId> reviewerGithubIds = List.of();

// when, then
assertThatThrownBy(() -> new ReviewerGroup(reviewee, reviewerGithubIds, "그룹 이름", description, deadline))
Copy link
Contributor

Choose a reason for hiding this comment

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

List reviewerGithubIds = List.of();
이 부분에 빈 값을 넣지 말고 뭔가를 넣어주면 좋을 것 같아요.

테스트 코드에서는 검증하고자 하는 것이 명확해야하잖아요, 그런데 아무리 '빈 리뷰어 그룹 검증'이 있기 전에 예외가 발생한다고 해도, 테스트하고자 하는 곳에 하나의 예외만 터지게 하고, 그 예외 발생을 검증하는 것이 "어떤것을 테스트할 것인지?"를 더 명확하게 드러낸다는 생각이 들어요!

그리고 나중에 도메인 내부에서 생성 로직이나 검증 함수 순서가 바뀌더라도 안심할 수 있기도 하구용

Copy link
Contributor Author

Choose a reason for hiding this comment

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

좋네요, 모두 반영했습니다 !

.isInstanceOf(InvalidDescriptionLengthException.class);
}

@Test
void 리뷰어_목록에_리뷰이가_들어갈_수_없다() {
// given
Member reviewee = 회원_산초.create();
String groupName = "Group";
String description = "Description";
LocalDateTime deadline = LocalDateTime.now().plusDays(1);
List<GithubId> reviewerGithubIds = List.of(reviewee.getGithubId());
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

완료 !


// when, then
assertThatThrownBy(() -> new ReviewerGroup(reviewee, reviewerGithubIds, groupName, description, deadline))
.isInstanceOf(SelfReviewException.class);
}

@Test
void 리뷰어_목록이_비어있을_수_없다() {
Member reviewee = 회원_산초.create();
String groupName = "Group";
String description = "Description";
LocalDateTime deadline = LocalDateTime.now().plusDays(1);
List<GithubId> reviewerGithubIds = List.of();

// when, then
assertThatThrownBy(() -> new ReviewerGroup(reviewee, reviewerGithubIds, groupName, description, deadline))
.isInstanceOf(EmptyReviewerException.class);
}

@Test
void 리뷰어를_중복으로_가지게_그룹을_생성할_수_없다() {
// given
Member reviewer = 회원_산초.create();
Member reviewee = 회원_커비.create();
String groupName = "Group";
String description = "Description";
LocalDateTime deadline = LocalDateTime.now().plusDays(1);
List<GithubId> reviewerGithubIds = List.of(reviewer.getGithubId(), reviewer.getGithubId());

// when, then
assertThatThrownBy(() -> new ReviewerGroup(reviewee, reviewerGithubIds, groupName, description, deadline))
.isInstanceOf(DuplicateReviewerException.class);
}

@Test
void 리뷰어를_중복으로_추가할_수_없다() {
// given
Member reviewer = 회원_커비.create();
Member reviewee = 회원_산초.create();

String groupName = "Group";
String description = "Description";
LocalDateTime deadline = LocalDateTime.now().plusDays(1);
List<GithubId> reviewerGithubIds = List.of(reviewer.getGithubId());
ReviewerGroup reviewerGroup = new ReviewerGroup(reviewee, reviewerGithubIds, groupName, description, deadline);
GithubIdReviewerGroup githubIdReviewerGroup = new GithubIdReviewerGroup(reviewee.getGithubId(), reviewerGroup);

// when, then
Copy link
Contributor

Choose a reason for hiding this comment

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

개행 (;´д`)ゞ

Copy link
Contributor Author

Choose a reason for hiding this comment

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

반영완료!

assertThatThrownBy(() -> new ReviewerGroup(sancho, List.of(), "그룹 이름", description, createdAt))
.isInstanceOf(DescriptionLengthExceededException.class);
assertThatThrownBy(() -> reviewerGroup.addReviewerGithubId(githubIdReviewerGroup))
.isInstanceOf(DuplicateReviewerException.class);
Comment on lines +120 to +121
Copy link
Contributor

Choose a reason for hiding this comment

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

이 함수 사용되는 곳이 테스트밖에 없는 것 같은데 맞나요..?

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

지금은 사용하지 않는 함수인데, 추후 리뷰 그룹 수정 때 필요해서 들어갔나 봐요. 그대로 둘까요, 삭제하는 게 좋을까요 ?

}
}
70 changes: 69 additions & 1 deletion backend/src/test/java/reviewme/review/domain/ReviewTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,28 @@
import static reviewme.fixture.KeywordFixture.꼼꼼하게_기록해요;
import static reviewme.fixture.KeywordFixture.의견을_잘_조율해요;
import static reviewme.fixture.MemberFixture.회원_산초;
import static reviewme.fixture.MemberFixture.회원_아루;
import static reviewme.fixture.MemberFixture.회원_커비;
import static reviewme.fixture.ReviewerGroupFixture.데드라인_남은_그룹;
import static reviewme.fixture.ReviewerGroupFixture.데드라인_지난_그룹;

import java.time.LocalDateTime;
import java.util.List;
import org.junit.jupiter.api.Test;
import reviewme.keyword.domain.Keyword;
import reviewme.member.domain.GithubId;
import reviewme.member.domain.Member;
import reviewme.member.domain.ReviewerGroup;
import reviewme.review.domain.exception.DeadlineExpiredException;
import reviewme.review.domain.exception.IllegalReviewerException;
import reviewme.review.domain.exception.RevieweeMismatchException;
import reviewme.review.exception.GithubReviewerGroupUnAuthorizedException;
import reviewme.review.exception.ReviewAlreadySubmittedException;

class ReviewTest {

@Test
void 리뷰어와_리뷰이가_다른_경우_예외를_발생한다() {
void 리뷰어와_리뷰이가_같을_수_없다() {
// given
Member member = 회원_산초.create();
LocalDateTime createdAt = LocalDateTime.now();
Expand All @@ -25,4 +35,62 @@ class ReviewTest {
assertThatThrownBy(() -> new Review(member, member, null, keywords, createdAt))
.isInstanceOf(IllegalReviewerException.class);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

@Test
    void 리뷰어와_리뷰이가_다른_경우_예외를_발생한다() {

"같은_경우" 로 변경되어야 할 것 같슴다

Copy link
Contributor Author

@donghoony donghoony Jul 25, 2024

Choose a reason for hiding this comment

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

리뷰어와_리뷰이가_같을_수_없다로 수정합니다~!

@Test
void 마감_기한이_지난_그룹에_리뷰를_등록할_수_없다() {
// given
Member reviewer = 회원_산초.create();
Member reviewee = 회원_아루.create();
ReviewerGroup reviewerGroup = 데드라인_지난_그룹.create(reviewee, List.of(reviewer.getGithubId()));
LocalDateTime createdAt = LocalDateTime.now();
List<Keyword> keywords = List.of();

// when, then
assertThatThrownBy(() -> new Review(reviewer, reviewee, reviewerGroup, keywords, createdAt))
.isInstanceOf(DeadlineExpiredException.class);
}

@Test
void 하나의_리뷰_그룹에_중복으로_리뷰를_등록할_수_없다() {
// given
Member reviewer = 회원_산초.create();
Member reviewee = 회원_아루.create();
ReviewerGroup reviewerGroup = 데드라인_남은_그룹.create(reviewee, List.of(reviewer.getGithubId()));
new Review(reviewer, reviewee, reviewerGroup, List.of(), LocalDateTime.now());
LocalDateTime createdAt = LocalDateTime.now();
List<Keyword> keywords = List.of();

// when, then
assertThatThrownBy(() -> new Review(reviewer, reviewee, reviewerGroup, keywords, createdAt))
.isInstanceOf(ReviewAlreadySubmittedException.class);
}

@Test
void 리뷰어로_등록되지_않은_회원은_리뷰를_등록할_수_없다() {
// given
Member reviewer = new Member("reviewer", 1);
Member reviewee = new Member("reviewee", 2);
ReviewerGroup reviewerGroup = 데드라인_남은_그룹.create(reviewee, List.of(new GithubId(3)));
LocalDateTime createdAt = LocalDateTime.now();
List<Keyword> keywords = List.of();

// when, then
assertThatThrownBy(() -> new Review(reviewer, reviewee, reviewerGroup, keywords, createdAt))
.isInstanceOf(GithubReviewerGroupUnAuthorizedException.class);
}

@Test
void 그룹_내에서_그룹_밖으로_리뷰를_작성할_수_없다() {
// given
Member reviewer = 회원_산초.create();
Member reviewee = 회원_아루.create();
Member other = 회원_커비.create();
ReviewerGroup reviewerGroup = 데드라인_남은_그룹.create(reviewee, List.of(reviewer.getGithubId()));
LocalDateTime createdAt = LocalDateTime.now();
List<Keyword> keywords = List.of();

// when, then
assertThatThrownBy(() -> new Review(reviewer, other, reviewerGroup, keywords, createdAt))
.isInstanceOf(RevieweeMismatchException.class);
}
}