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

Member Service 테스트 #646

Merged
merged 9 commits into from
Sep 20, 2024
Original file line number Diff line number Diff line change
@@ -1,18 +1,11 @@
package codezap.global.cors;

import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
import static org.junit.jupiter.api.Assertions.*;

import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.boot.test.context.ConfigDataApplicationContextInitializer;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.TestPropertySource;
import org.springframework.test.context.junit.jupiter.SpringExtension;
zangsu marked this conversation as resolved.
Show resolved Hide resolved

@SpringBootTest
class CorsPropertiesTest {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import org.springframework.boot.test.autoconfigure.jdbc.AutoConfigureTestDatabase;
import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest;
import org.springframework.context.annotation.Import;
import org.springframework.test.context.ActiveProfiles;

import codezap.global.DatabaseIsolation;
import codezap.global.auditing.JpaAuditingConfiguration;
Expand Down
124 changes: 102 additions & 22 deletions backend/src/test/java/codezap/member/service/MemberServiceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,34 +3,44 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertAll;

import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;

import codezap.category.domain.Category;
import codezap.category.repository.CategoryRepository;
import codezap.category.repository.FakeCategoryRepository;
import codezap.fixture.CategoryFixture;
import codezap.fixture.TemplateFixture;
import codezap.global.DatabaseIsolation;
import codezap.global.exception.CodeZapException;
import codezap.member.domain.Member;
import codezap.member.dto.MemberDto;
import codezap.member.dto.request.SignupRequest;
import codezap.member.dto.response.FindMemberResponse;
import codezap.member.fixture.MemberFixture;
import codezap.member.repository.FakeMemberRepository;
import codezap.member.repository.MemberRepository;
import codezap.auth.encryption.PasswordEncryptor;
import codezap.auth.encryption.RandomSaltGenerator;
import codezap.auth.encryption.SHA2PasswordEncryptor;
import codezap.auth.encryption.SaltGenerator;
import codezap.template.domain.Template;
import codezap.template.repository.TemplateRepository;

public class MemberServiceTest {
@SpringBootTest
kyum-q marked this conversation as resolved.
Show resolved Hide resolved
@DatabaseIsolation
class MemberServiceTest {

private final MemberRepository memberRepository = new FakeMemberRepository();
private final CategoryRepository categoryRepository = new FakeCategoryRepository();
private final SaltGenerator saltGenerator = new RandomSaltGenerator();
private final PasswordEncryptor passwordEncryptor = new SHA2PasswordEncryptor();
private final MemberService memberService = new MemberService(memberRepository, categoryRepository, saltGenerator, passwordEncryptor);
@Autowired
private MemberRepository memberRepository;

@Autowired
private CategoryRepository categoryRepository;

@Autowired
private TemplateRepository templateRepository;

@Autowired
private MemberService memberService;

@Nested
@DisplayName("회원가입 테스트")
Expand All @@ -42,7 +52,13 @@ void signup() {
Member member = MemberFixture.memberFixture();
SignupRequest signupRequest = new SignupRequest(member.getName(), member.getPassword());

assertEquals(memberService.signup(signupRequest), 1L);
Long savedId = memberService.signup(signupRequest);

boolean isDefaultCategory = categoryRepository.existsByNameAndMember("카테고리 없음", member);
jminkkk marked this conversation as resolved.
Show resolved Hide resolved
zeus6768 marked this conversation as resolved.
Show resolved Hide resolved
assertAll(
() -> assertThat(savedId).isEqualTo(member.getId()),
() -> assertThat(isDefaultCategory).isTrue()
);
}

@Test
Expand All @@ -59,11 +75,11 @@ void signup_fail_name_duplicate() {

@Nested
@DisplayName("아이디 중복 검사 테스트")
class AssertUniquename {
class AssertUniqueName {

@Test
@DisplayName("아이디 중복 검사 통과: 사용가능한 아이디")
void assertUniquename() {
void assertUniqueName() {
String name = "code";

assertThatCode(() -> memberService.assertUniqueName(name))
Expand All @@ -72,36 +88,100 @@ void assertUniquename() {

@Test
@DisplayName("아이디 중복 검사 실패: 중복된 아이디")
void assertUniquename_fail_duplicate() {
void assertUniqueName_fail_duplicate() {
Member member = memberRepository.save(MemberFixture.memberFixture());
String memberName = member.getName();

assertThatThrownBy(() -> memberService.assertUniqueName(member.getName()))
assertThatThrownBy(() -> memberService.assertUniqueName(memberName))
jminkkk marked this conversation as resolved.
Show resolved Hide resolved
.isInstanceOf(CodeZapException.class)
.hasMessage("아이디가 이미 존재합니다.");
}
}

@Nested
@DisplayName("회원 조회 테스트")
class findMember {
class FindMember {

@Test
@DisplayName("회원 정보 조회 성공")
void findMember() {
zangsu marked this conversation as resolved.
Show resolved Hide resolved
Member member = memberRepository.save(MemberFixture.memberFixture());

assertThat(memberService.findMember(MemberDto.from(member), member.getId()))
.isEqualTo(FindMemberResponse.from(member));
FindMemberResponse actual = memberService.findMember(MemberDto.from(member), member.getId());

assertThat(actual).isEqualTo(FindMemberResponse.from(member));
}

@Test
@DisplayName("회원 정보 조회 실패: 본인 정보가 아닌 경우")
zeus6768 marked this conversation as resolved.
Show resolved Hide resolved
void findMember_Throw() {
Member member = memberRepository.save(MemberFixture.memberFixture());
MemberDto memberDto = MemberDto.from(member);
Long otherId = member.getId() + 1;

assertThatThrownBy(() -> memberService.findMember(MemberDto.from(member), member.getId() + 1))
assertThatThrownBy(() -> memberService.findMember(memberDto, otherId))
.isInstanceOf(CodeZapException.class)
.hasMessage("본인의 정보만 조회할 수 있습니다.");
}

@Test
@DisplayName("회원 정보 조회 실패: DB에 없는 멤버인 경우")
void findMember_Throw_Not_Exists() {
Member member = MemberFixture.memberFixture();
MemberDto memberDto = MemberDto.from(member);
Long memberId = member.getId();

assertThatThrownBy(() -> memberService.findMember(memberDto, memberId))
.isInstanceOf(CodeZapException.class)
.hasMessage("식별자 " + memberId + "에 해당하는 멤버가 존재하지 않습니다.");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Repository 단에서 나는 에러 처리도 서비스 테스트에서 확인하는게 맞을지 의문이 들긴합니다.
일단 추가하긴했는데 함께 고민하면 좋을 것 같아요 여러분들의 의견은 어떤가요?

이 부분인가요??

저는 예외 발생이 해당 메서드의 명세가 되는가 / 그렇지 않은가 에 따라 달라질 것이라 생각해요.
이 부분은 "저장되지 않은 멤버의 정보를 조회하면 예외가 발생한다" 라는 명세를 테스트 하는 것으로 느껴지네요.
테스트가 있는 것이 자연스럽게 느껴집니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

네 해당 부분은 서비스 단에서는 예외처리 로직이 존재하지 않고, 레포지토리 단에 예외 로직이 있어요 !
그래서 추가할까 고민했었습니다 !
짱수 의견 말해줘서 고마워요 ~ 저도 동의합니다 !

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

Choose a reason for hiding this comment

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

저도요~
모두와 동일하게 생각해요~

[고민]

그래서 테스트를 작성하고 리뷰를 작성하다가 의문이 드는 게 있어요.
테스트에 대한 이야기는 아니지만,,, 테스트를 보다가 생긴 의문이라 공유할게요~

바로 이전 회원가입, 아이디 중복 검사 테스트에서 아이디 길이나, 비밀번호 길이와 같은 부분은 검증되어 있지 않아요.(다른 도메인의 서비스에서도 마찬가지)
아마 Controller에서 검증을 하고 있기 때문에 작성되어 있지 않는 것 같아요.

그런데 읽다보니 어색하더라구요~
저는 아이디 길이나, 비밀번호 길이이도 우리 애플리케이션 고유의 요구사항이 비즈니스 로직이라고 생각해요.
따라서 우리 서비스에서 검증이 필요하다고 생각해요.
(그렇다고 null 체크도 검사하자는 것은 아닙니다. null 체크는 어떤 애플리케이션이든 필요하고 우리 서비스만의 규칙이 아님. 근데 아이디 길이를 8~20자로 제한한다든지, 비밀번호에 특수문자를 꼭 넣어야 한다든지 하는 건 우리 서비스만의 특별한 규칙이라고 생각)

여러분들의 의견이 궁금한 것이 있어요. (의견이 많이 갈릴 것 같은데 쓰읍)

  1. Controller 에서 검증을 하고 있지만, 이 부분은 Service에서 검증이 필요하지 않을까요?
  2. (않다면) 이 부분은 도메인 규칙이 아닐까요?0?
  3. (않다면2) 여러분이 생각하는 Controller와 Service의 역할, 나아가 Presentation Layer, Business Layer의 역할은 무엇일까요?

Copy link
Contributor

Choose a reason for hiding this comment

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

@jminkkk
오, 저도 비슷한 생각을 헀다가 놓쳤네요.
저도 동일하게 도메인 규칙이고, 비즈니스 로직이라 생각해요.
Service 부분에서의 검증이 진행되는 것이 이상적이며, 우리 서비스에서도 동일하게 적용할 수 있다고 생각합니다.

+)
저는 Controller 의 검증 (즉, DTO 가 스스로 검증) 과 도메인에서의 검증 중 하나만 선택하라면 후자를 선택할 것 같아요.
도메인 검증은 추가하는 것이 좋겠네요

Copy link
Contributor

Choose a reason for hiding this comment

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

@jminkkk
@zangsu

저는 조금 다르게 생각하는 부분이 있어요.

  1. Service에서 검증할 필요 없다고 생각합니다.
  2. 도메인 규칙이 맞습니다.

Service 계층에서 도메인 검증을 테스트할 필요 없다고 생각해요.

도메인 객체의 검증 로직은 도메인의 테스트 코드를 만들어서 테스트해야 하지 않을까요?

DTO에 검증 로직이 들어갔다면 DTO 테스트 코드를 만들어야 한다고 생각해요.

그렇게 하면 서비스 테스트의 부담이 줄어들 것 같네요.

결론은 도메인 테스트와 DTO 테스트를 추가해야 한다 입니다~!

다른 의견 있다면 얼마든지 부탁해용

Copy link
Contributor Author

Choose a reason for hiding this comment

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

모두 맞는말이라 어렵네요 ~
이런 모든 고민은 'Service 단에서 어디까지 테스트할 것인가'의 고민인 것 같네요.

레포지토리 예외처리도 처리한다, 하위 서비스 예외처리도 처리한다, 도메인 검증도 처리한다.
이 모두 결국엔 현재 테스트에 파생되는 모든 검증, 예외 로직을 처리할 것인가에 대해 정확하기 이야기 안되서 인 것 같아요.

그래서 가장 큰 틀로 오늘 조금 이야기 한 주제인 '서비스 테스트는(이 외에도) 어디까지 테스트를 고려할 것인가?'에 대해 이야기해보면 좋을 것 같아요 ~

  1. 해당 서비스 단에서 하는 로직만 테스트 구현한다 (하위 서비스 단 로직, 도메인 로직은 고려하지 않는다)
  2. 헤당 서비스와 연관된 클래스 로직도 고려해서 테스트 구현한다.
    • 레포지토리단 예외 처리
    • 하위 서비스 예외 처리
    • 도메인 검증

이번 계기로 생각해보니 저는 1번 방식을 선호합니다. 그렇지 않으면 로직을 분리할수록 중복되는 테스트 코드가 너무 많아질 것 같아요.
그래서 블랙박스 테스트로 연관된 클래스를 고려하지 않고, 해당 클래스와 해당 클래스의 용도만 생각했을 때 실패 케이스를 작성하는 방식으로 테스트를 구현하는 것이 좋을 것 같아요 ~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Controller 에서 검증을 하고 있지만, 이 부분은 Service에서 검증이 필요하지 않을까요?

직접적으로 도메인 검증 로직을 호출하는 서비스에서는 처리해야하고, 위에 말했듯이 직접 사용하지 않는 상위 서비스에서는 하지 않아도 될 것 같습니다.

(않다면) 이 부분은 도메인 규칙이 아닐까요?0?

도메인 규칙은 맞는 것 같아요
Dto와 도메인 두군데에서 중복 검증해야할 것 같네요

(않다면2) 여러분이 생각하는 Controller와 Service의 역할, 나아가 Presentation Layer, Business Layer의 역할은 무엇일까요?

컨트롤러는 요청을 받아 넘겨주고 반환하는 역할, 서비스는 받은 요청에 따른 비즈니스 로직을 처리하는 역할이라고 생각합니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

앗 저도 제우스 의견에 더 가깝네요.
도메인의 예외 처리에 대한 테스트는 도메인 테스트에서 확인을 하면 될 것 같군요.
추가적인 예외처리가 있는 경우에만 서비스 테스트에서 추가 확인을 해 주어도 될 것 같아요~

켬미가 고정해 준 이야기 주제에선 저도 1번 방법을 선호합니다!

zangsu marked this conversation as resolved.
Show resolved Hide resolved
}

@Nested
@DisplayName("템플릿을 소유한 멤버 조회")
class GetByTemplateId {
@Test
jminkkk marked this conversation as resolved.
Show resolved Hide resolved
@DisplayName("템플릿을 소유한 멤버 조회 성공")
void getByTemplateId() {
Member member = memberRepository.save(MemberFixture.memberFixture());
Category category = categoryRepository.save(CategoryFixture.getFirstCategory());
Template template = templateRepository.save(TemplateFixture.get(member, category));

Member actual = memberService.getByTemplateId(template.getId());

assertThat(actual).isEqualTo(member);
}

@Test
@DisplayName("템플릿을 소유한 멤버 조회 실패 : DB에 없는 템플릿인 경우")
void getByTemplateId_Fail() {
assertThatCode(() -> memberService.getByTemplateId(100L))
kyum-q marked this conversation as resolved.
Show resolved Hide resolved
.isInstanceOf(CodeZapException.class)
.hasMessage("템플릿에 대한 멤버가 존재하지 않습니다.");
}
}

@Nested
@DisplayName("아이디로 멤버 조회")
class GetById {
@Test
@DisplayName("아이디로 멤버 조회 성공")
void getById() {
Member member = memberRepository.save(MemberFixture.memberFixture());

Member actual = memberService.getById(member.getId());

assertThat(actual).isEqualTo(member);
}

@Test
@DisplayName("아이디로 멤버 조회 실패 : 존재하지 않는 아이디")
void getById_Fail() {
Long notExitsId = 100L;

assertThatCode(() -> memberService.getById(notExitsId))
.isInstanceOf(CodeZapException.class)
.hasMessage("식별자 " + notExitsId + "에 해당하는 멤버가 존재하지 않습니다.");
}
}
}