Member Service 테스트 #646
Member Service 테스트 #646
Conversation
backend/src/test/java/codezap/member/service/MemberServiceTest.java
Outdated
Show resolved
Hide resolved
backend/src/test/java/codezap/member/service/MemberServiceTest.java
Outdated
Show resolved
Hide resolved
| @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 + "에 해당하는 멤버가 존재하지 않습니다."); | ||
| } |
There was a problem hiding this comment.
Repository 단에서 나는 에러 처리도 서비스 테스트에서 확인하는게 맞을지 의문이 들긴합니다.
일단 추가하긴했는데 함께 고민하면 좋을 것 같아요 여러분들의 의견은 어떤가요?
이 부분인가요??
저는 예외 발생이 해당 메서드의 명세가 되는가 / 그렇지 않은가 에 따라 달라질 것이라 생각해요.
이 부분은 "저장되지 않은 멤버의 정보를 조회하면 예외가 발생한다" 라는 명세를 테스트 하는 것으로 느껴지네요.
테스트가 있는 것이 자연스럽게 느껴집니다!
There was a problem hiding this comment.
네 해당 부분은 서비스 단에서는 예외처리 로직이 존재하지 않고, 레포지토리 단에 예외 로직이 있어요 !
그래서 추가할까 고민했었습니다 !
짱수 의견 말해줘서 고마워요 ~ 저도 동의합니다 !
There was a problem hiding this comment.
저도 짱수 의견에 동의합니다.
테스트 코드는 문서화와 같다고 생각해요. 해당 메서드를 실행했을 때 어떤 오류들이 발생할 수 있는지를 나타내는 것 또한 문서화의 일종이기 때문에 해당 테스트 코드가 전혀 어색하지 않다고 생각합니다.
There was a problem hiding this comment.
저도요~
모두와 동일하게 생각해요~
[고민]
그래서 테스트를 작성하고 리뷰를 작성하다가 의문이 드는 게 있어요.
테스트에 대한 이야기는 아니지만,,, 테스트를 보다가 생긴 의문이라 공유할게요~
바로 이전 회원가입, 아이디 중복 검사 테스트에서 아이디 길이나, 비밀번호 길이와 같은 부분은 검증되어 있지 않아요.(다른 도메인의 서비스에서도 마찬가지)
아마 Controller에서 검증을 하고 있기 때문에 작성되어 있지 않는 것 같아요.
그런데 읽다보니 어색하더라구요~
저는 아이디 길이나, 비밀번호 길이이도 우리 애플리케이션 고유의 요구사항이 비즈니스 로직이라고 생각해요.
따라서 우리 서비스에서 검증이 필요하다고 생각해요.
(그렇다고 null 체크도 검사하자는 것은 아닙니다. null 체크는 어떤 애플리케이션이든 필요하고 우리 서비스만의 규칙이 아님. 근데 아이디 길이를 8~20자로 제한한다든지, 비밀번호에 특수문자를 꼭 넣어야 한다든지 하는 건 우리 서비스만의 특별한 규칙이라고 생각)
여러분들의 의견이 궁금한 것이 있어요. (의견이 많이 갈릴 것 같은데 쓰읍)
- Controller 에서 검증을 하고 있지만, 이 부분은 Service에서 검증이 필요하지 않을까요?
- (않다면) 이 부분은 도메인 규칙이 아닐까요?0?
- (않다면2) 여러분이 생각하는 Controller와 Service의 역할, 나아가 Presentation Layer, Business Layer의 역할은 무엇일까요?
There was a problem hiding this comment.
@jminkkk
오, 저도 비슷한 생각을 헀다가 놓쳤네요.
저도 동일하게 도메인 규칙이고, 비즈니스 로직이라 생각해요.
Service 부분에서의 검증이 진행되는 것이 이상적이며, 우리 서비스에서도 동일하게 적용할 수 있다고 생각합니다.
+)
저는 Controller 의 검증 (즉, DTO 가 스스로 검증) 과 도메인에서의 검증 중 하나만 선택하라면 후자를 선택할 것 같아요.
도메인 검증은 추가하는 것이 좋겠네요
There was a problem hiding this comment.
저는 조금 다르게 생각하는 부분이 있어요.
- Service에서 검증할 필요 없다고 생각합니다.
- 도메인 규칙이 맞습니다.
Service 계층에서 도메인 검증을 테스트할 필요 없다고 생각해요.
도메인 객체의 검증 로직은 도메인의 테스트 코드를 만들어서 테스트해야 하지 않을까요?
DTO에 검증 로직이 들어갔다면 DTO 테스트 코드를 만들어야 한다고 생각해요.
그렇게 하면 서비스 테스트의 부담이 줄어들 것 같네요.
결론은 도메인 테스트와 DTO 테스트를 추가해야 한다 입니다~!
다른 의견 있다면 얼마든지 부탁해용
There was a problem hiding this comment.
모두 맞는말이라 어렵네요 ~
이런 모든 고민은 'Service 단에서 어디까지 테스트할 것인가'의 고민인 것 같네요.
레포지토리 예외처리도 처리한다, 하위 서비스 예외처리도 처리한다, 도메인 검증도 처리한다.
이 모두 결국엔 현재 테스트에 파생되는 모든 검증, 예외 로직을 처리할 것인가에 대해 정확하기 이야기 안되서 인 것 같아요.
그래서 가장 큰 틀로 오늘 조금 이야기 한 주제인 '서비스 테스트는(이 외에도) 어디까지 테스트를 고려할 것인가?'에 대해 이야기해보면 좋을 것 같아요 ~
- 해당 서비스 단에서 하는 로직만 테스트 구현한다 (하위 서비스 단 로직, 도메인 로직은 고려하지 않는다)
- 헤당 서비스와 연관된 클래스 로직도 고려해서 테스트 구현한다.
- 레포지토리단 예외 처리
- 하위 서비스 예외 처리
- 도메인 검증
이번 계기로 생각해보니 저는 1번 방식을 선호합니다. 그렇지 않으면 로직을 분리할수록 중복되는 테스트 코드가 너무 많아질 것 같아요.
그래서 블랙박스 테스트로 연관된 클래스를 고려하지 않고, 해당 클래스와 해당 클래스의 용도만 생각했을 때 실패 케이스를 작성하는 방식으로 테스트를 구현하는 것이 좋을 것 같아요 ~
There was a problem hiding this comment.
Controller 에서 검증을 하고 있지만, 이 부분은 Service에서 검증이 필요하지 않을까요?
직접적으로 도메인 검증 로직을 호출하는 서비스에서는 처리해야하고, 위에 말했듯이 직접 사용하지 않는 상위 서비스에서는 하지 않아도 될 것 같습니다.
(않다면) 이 부분은 도메인 규칙이 아닐까요?0?
도메인 규칙은 맞는 것 같아요
Dto와 도메인 두군데에서 중복 검증해야할 것 같네요
(않다면2) 여러분이 생각하는 Controller와 Service의 역할, 나아가 Presentation Layer, Business Layer의 역할은 무엇일까요?
컨트롤러는 요청을 받아 넘겨주고 반환하는 역할, 서비스는 받은 요청에 따른 비즈니스 로직을 처리하는 역할이라고 생각합니다.
There was a problem hiding this comment.
앗 저도 제우스 의견에 더 가깝네요.
도메인의 예외 처리에 대한 테스트는 도메인 테스트에서 확인을 하면 될 것 같군요.
추가적인 예외처리가 있는 경우에만 서비스 테스트에서 추가 확인을 해 주어도 될 것 같아요~
켬미가 고정해 준 이야기 주제에선 저도 1번 방법을 선호합니다!
backend/src/test/java/codezap/member/service/MemberServiceTest.java
Outdated
Show resolved
Hide resolved
| @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 + "에 해당하는 멤버가 존재하지 않습니다."); | ||
| } |
There was a problem hiding this comment.
저도 짱수 의견에 동의합니다.
테스트 코드는 문서화와 같다고 생각해요. 해당 메서드를 실행했을 때 어떤 오류들이 발생할 수 있는지를 나타내는 것 또한 문서화의 일종이기 때문에 해당 테스트 코드가 전혀 어색하지 않다고 생각합니다.
| public class MemberServiceTest { | ||
| @SpringBootTest | ||
| @DatabaseIsolation | ||
| @AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE) |
There was a problem hiding this comment.
정확한 이유는 아직 잘 모르겠기는 한데 해당 어노테이션 때문에 발생하는 오류로 보입니다.
해당 어노테이션은 어떤 어노테이션인가요?
간단히 찾아보았을 때는 테스트를 할 때 실제 데이터베이스를 사용하게 해주는 설정이라고 하는데, 해당 설정이 없어도 실제 데이터베이스를 사용하고 있지 않나요??
잘 몰라서 질문 남깁니다
There was a problem hiding this comment.
실제로 찾아보니 해당 어노테이션은 없어도 될 것 같아요 !
위에 실제 DB를 사용하지 않는다는 조건은 @DataJpaTest 테스트 시에만 유효하네요 ! 그래서 @DataJpaTest 이 아니라면 db.yml 설정에 맞게 실제 DB를 사용한다고 합니다 !
서치 해보면 해당 어노테이션은 @DataJpaTest에서 사용하더라고요 !
그래서 ChatGPT 선생 도움을 받아보니 다음과 같다고 하네요 !
@DataJpaTest와 @AutoConfigureTestDatabase
@DataJpaTest는 JPA 관련 테스트만을 실행하기 위해 데이터 레이어만 컨텍스트에 로드합니다. 테스트 환경에서 JPA 기능(리포지토리, 엔티티 등)만 테스트하려는 경우에 사용됩니다.
기본적으로 Spring Boot는 @DataJpaTest를 사용하면 임베디드 데이터베이스(예: H2)를 자동으로 사용하도록 설정합니다.
이때 실제 데이터베이스(MySQL, PostgreSQL 등)를 사용하고 싶으면 @AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE)를 추가하여 임베디드 데이터베이스로 대체되지 않도록 설정하는 것입니다.
@SpringBootTest에서는 왜 설정하지 않아도 되는가?
@SpringBootTest는 애플리케이션의 모든 빈을 로드하여 전체 애플리케이션을 통합적으로 테스트합니다.
이 경우 Spring Boot는 application.yml 또는 application.properties 파일에 정의된 실제 데이터베이스 설정을 그대로 사용합니다. 즉, @SpringBootTest는 임베디드 데이터베이스로 대체되지 않고, 실제 데이터베이스 설정을 따릅니다.
따라서, 별도로 @AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE)를 지정할 필요가 없습니다.
요약
@DataJpaTest: JPA 관련 테스트에서 임베디드 데이터베이스 사용을 방지하려면@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE)를 추가해야 함.@SpringBootTest: 실제 application.yml 설정을 따르기 때문에, 임베디드 DB로 대체되지 않고 별도의 설정이 필요 없음.
There was a problem hiding this comment.
오호 좋은 정보 감사합니다 👍
근데 그럼 CI 문제는 랜덤 포트 때문에 발생하는 건가요..?
backend/src/test/java/codezap/member/service/MemberServiceTest.java
Outdated
Show resolved
Hide resolved
backend/src/test/java/codezap/member/service/MemberServiceTest.java
Outdated
Show resolved
Hide resolved
backend/src/test/java/codezap/member/service/MemberServiceTest.java
Outdated
Show resolved
Hide resolved
| @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 + "에 해당하는 멤버가 존재하지 않습니다."); | ||
| } |
There was a problem hiding this comment.
저도요~
모두와 동일하게 생각해요~
[고민]
그래서 테스트를 작성하고 리뷰를 작성하다가 의문이 드는 게 있어요.
테스트에 대한 이야기는 아니지만,,, 테스트를 보다가 생긴 의문이라 공유할게요~
바로 이전 회원가입, 아이디 중복 검사 테스트에서 아이디 길이나, 비밀번호 길이와 같은 부분은 검증되어 있지 않아요.(다른 도메인의 서비스에서도 마찬가지)
아마 Controller에서 검증을 하고 있기 때문에 작성되어 있지 않는 것 같아요.
그런데 읽다보니 어색하더라구요~
저는 아이디 길이나, 비밀번호 길이이도 우리 애플리케이션 고유의 요구사항이 비즈니스 로직이라고 생각해요.
따라서 우리 서비스에서 검증이 필요하다고 생각해요.
(그렇다고 null 체크도 검사하자는 것은 아닙니다. null 체크는 어떤 애플리케이션이든 필요하고 우리 서비스만의 규칙이 아님. 근데 아이디 길이를 8~20자로 제한한다든지, 비밀번호에 특수문자를 꼭 넣어야 한다든지 하는 건 우리 서비스만의 특별한 규칙이라고 생각)
여러분들의 의견이 궁금한 것이 있어요. (의견이 많이 갈릴 것 같은데 쓰읍)
- Controller 에서 검증을 하고 있지만, 이 부분은 Service에서 검증이 필요하지 않을까요?
- (않다면) 이 부분은 도메인 규칙이 아닐까요?0?
- (않다면2) 여러분이 생각하는 Controller와 Service의 역할, 나아가 Presentation Layer, Business Layer의 역할은 무엇일까요?
backend/src/test/java/codezap/member/service/MemberServiceTest.java
Outdated
Show resolved
Hide resolved
backend/src/test/java/codezap/member/service/MemberServiceTest.java
Outdated
Show resolved
Hide resolved
|
이 브랜치 혹시 어디서 분기하신건가요?? |
dev/be 에서 분기했습니다 그리고 MemberFixture 의 변경 사항이 있다면 변경이 잡히지 않은 점이 이상하네요..! 어떤 부분에서 MemberFixture 가 다르다고 생각하셨나요? +MemberFixture는 나중에 하나로 통일하면 좋을 것 같으니 몰리가 파놓은 이슈에 해당 내용 추가해놓겠습니다 |
|
�아 member 패키지 안에 |
backend/src/test/java/codezap/member/service/MemberServiceTest.java
Outdated
Show resolved
Hide resolved
backend/src/test/java/codezap/member/service/MemberServiceTest.java
Outdated
Show resolved
Hide resolved
| @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 + "에 해당하는 멤버가 존재하지 않습니다."); | ||
| } |
There was a problem hiding this comment.
저는 조금 다르게 생각하는 부분이 있어요.
- Service에서 검증할 필요 없다고 생각합니다.
- 도메인 규칙이 맞습니다.
Service 계층에서 도메인 검증을 테스트할 필요 없다고 생각해요.
도메인 객체의 검증 로직은 도메인의 테스트 코드를 만들어서 테스트해야 하지 않을까요?
DTO에 검증 로직이 들어갔다면 DTO 테스트 코드를 만들어야 한다고 생각해요.
그렇게 하면 서비스 테스트의 부담이 줄어들 것 같네요.
결론은 도메인 테스트와 DTO 테스트를 추가해야 한다 입니다~!
다른 의견 있다면 얼마든지 부탁해용
backend/src/test/java/codezap/member/service/MemberServiceTest.java
Outdated
Show resolved
Hide resolved
⚡️ 관련 이슈
close #645
📍주요 변경 사항
MemberService Test 보충 및 MemberServiceTest 실제 DB 사용으로 변경
🎸기타
테스트 고민
Repository 단에서 나는 에러 처리도 서비스 테스트에서 확인하는게 맞을지 의문이 들긴합니다.
일단 추가하긴했는데 함께 고민하면 좋을 것 같아요 여러분들의 의견은 어떤가요?
CI 문제
문제 해결 방법 찾음, BUT 이게 맞을까? -> 동일하게 바꾸면 CI 통과함
몰리 PR을 보니 CI 문제가 발생 안함.
그래서 뭐가 다른지 확인해보니 몰리는 기본 설정인 MOCK 사용하고,
@AutoConfigureTestDatabase설정이 없음1.
@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT)랜덤 포트 문제그래서 바꿔보니 해결 되네..? 왜 RANDOM_PORT 에서 문제가 발생할까요? 실제 환경이랑 동일하게 테스트하고 싶은 저희 욕구에 맞추려면 RANDOM_PORT를 사용하는게 좋을 것같은데 문제가 발생해서 애매하네요.. ㅜ
2.@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE)설정 추가하면 문제실제 DB를 사용하려면 해당 어노테이션 추가해야하지 않나요? 그런데 추가하면 에러 발생함제가 해당 어노테이션에 대해 다음과 같이 이해했는데 잘못된 정보가 있거나 해당 어노테이션이 없어도 DB를 잘 사용한다는 게 보장된다면 지우겠습니다- 없을 때: Spring Boot는 테스트 실행 시 기본적으로 임베디드 데이터베이스를 사용하며, 빠르게 실행되지만 실제 DB 환경과 차이가 있을 수 있음.- 있을 때: 실제 데이터베이스를 그대로 사용하여 운영 환경과 더 유사한 테스트가 가능하지만, 속도가 느릴 수 있음.실제로 찾아보니 해당 어노테이션은 없어도 될 것 같아요 !
위에 실제 DB를 사용하지 않는다는 조건은
@DataJpaTest테스트 시에만 유효하네요 ! 그래서@DataJpaTest이 아니라면 db.yml 설정에 맞게 실제 DB를 사용한다고 합니다 !둘 다 추가했을 때 나는 에러는 뭘까?
flyway 설정 문제가 발생 문제 인가?
CI 작동 시 나는 에러를 chatGPT한테 물어보니 flyway 문제라고 진단하네요.
로컬에서는 잘 작동해서 문제가 무엇인지 의문..
로컬에서 CI 에러를 똑같이 만드는 법은 db.yml 파일에서 flyway 로직을 제거하면 동일한 에러 발생
그래서 CI의 db.yml 파일을 flyway 설정을 포함하도록 변경해봄
그래도 해결되지 않음..!