[#250]크루 알람,게임 알람, 통합 알람을 테스트코드 작성 #251
Conversation
jay-so
requested review from
charlesuu and
Hchanghyeon
and removed request for
a team
| @SpringBootTest | ||
| @AutoConfigureMockMvc | ||
| @AutoConfigureRestDocs | ||
| public abstract class IntegrationCrewAlarmTest { |
There was a problem hiding this comment.
p1
IntegrationAlarmTest가 하나 있기 때문에 IntegrationCrewAlarmTest와 IntegrationGameAlramTest는 삭제해주셔도 될 것 같습니다.
추상 클래스로 묶는 이유는 테스트 코드 실행시 매번 스프링 컨텍스트가 올라가지 않게끔 하는 것인데 하나의 도메인에서 여러개를 띄우는 방식으로 사용한다면 사용하는 의미가 사라질 것 같습니다!
CrewAlram과 GameAlram이 모두 Alram도메인에 속하고 있으니 IntegrationAlarmTest 안에 의존이 필요한 객체들을 적어주시고 ControllerTest와 DocumentTest는 모두 IntegrationAlramTest를 상속받아서 처리해주시면 될 것 같습니다!
There was a problem hiding this comment.
아뇨. 제 생각은 다릅니다~
IntegrationAlarmTest가 있어, 해당 IntegrationCrewAlarmTest와 IntegrationGameAlramTest을 삭제하다니요?!
테스트 코드 상으로 비슷하게 보이지만, 서로 CrewAlarm과 GameAlarm에서 각각 필요한 데이터 타입은 다릅니다.
그렇게 되면 AlarmControllerTest와 CrewAlarmControllerTest,GameAlarmControllerTest 모두 동일한 데이터를 가진 다는 것인데, 구조적으로는 비슷하나 서로 각각 다른 데이터 타입을 비교합니다.
때문에 저는 해당 부분은 분리되어야 한다는 생각입니다~
There was a problem hiding this comment.
위에서도 말씀드렸지만 별도의 추상클래스로 분리해서 통합테스트를 사용하는 이유는 테스트 코드 실행시 매번 스프링 컨텍스트가 올라가지 않게끔 하기 위함입니다. 물론 서로 다른 종류의 "알림"이지만 하나의 "알림" 도메인이라고 볼 수 있고 필요한 데이터 타입이 다른 것과는 별개로 알림 도메인에 속하는 모든 통합테스트를 하나의 상위 통합테스트로 부터 상속 받아 알림 도메인에 속한 통합테스트 클래스들의 스프링 컨텍스트를 하나로 띄우기 위함입니다.
모두 동일한 데이터를 가진다는 말씀이 어떤 의미인지 잘 이해하지 못했는데 이 부분에 대해서 조금 더 자세하게 설명해주실 수 있으실까요? 추가로 데이터 타입과 통합테스트가 어떠한 연관이 있어서 말씀해주신건지 조금 더 자세하게 말씀해주시면 이해할 수 있을 것 같습니다! 👍👍
There was a problem hiding this comment.
해당 부분을 IntegrationAlarmTest로 통일하고 CrewAlarmControllerTest와 GameAlarmControllerTest 및 Document를 모두 IntegrationAlarmTest를 사용하는 AlarmConrollerTest,AlarmDocumnetTest로 만들어야 하는 피드백인줄 알았습니다.
해당 부분 반영하여 수정하였습니다~
반영 커밋: accdb6d
src/main/java/kr/pickple/back/alarm/dto/request/CrewAlarmUpdateStatusRequest.java
Outdated
Show resolved
Hide resolved
src/main/java/kr/pickple/back/alarm/dto/request/GameAlarmUpdateStatusRequest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
@Hchanghyeon
코멘트 달아두신 부분에 대해서 수정하고, 의견 남겨두었습니다! 확인해주시면 감사하겠습니다!😀😀
src/main/java/kr/pickple/back/alarm/dto/request/GameAlarmUpdateStatusRequest.java
Outdated
Show resolved
Hide resolved
src/main/java/kr/pickple/back/alarm/dto/request/CrewAlarmUpdateStatusRequest.java
Outdated
Show resolved
Hide resolved
| @SpringBootTest | ||
| @AutoConfigureMockMvc | ||
| @AutoConfigureRestDocs | ||
| public abstract class IntegrationCrewAlarmTest { |
There was a problem hiding this comment.
아뇨. 제 생각은 다릅니다~
IntegrationAlarmTest가 있어, 해당 IntegrationCrewAlarmTest와 IntegrationGameAlramTest을 삭제하다니요?!
테스트 코드 상으로 비슷하게 보이지만, 서로 CrewAlarm과 GameAlarm에서 각각 필요한 데이터 타입은 다릅니다.
그렇게 되면 AlarmControllerTest와 CrewAlarmControllerTest,GameAlarmControllerTest 모두 동일한 데이터를 가진 다는 것인데, 구조적으로는 비슷하나 서로 각각 다른 데이터 타입을 비교합니다.
때문에 저는 해당 부분은 분리되어야 한다는 생각입니다~
| import lombok.Getter; | ||
| import lombok.NoArgsConstructor; | ||
|
|
||
| @Getter | ||
| @AllArgsConstructor(access = AccessLevel.PRIVATE) |
There was a problem hiding this comment.
p3
- 롬복을 사용하고 있으니 from메서드는 지우고 아래와 같이 조금 더 간결하게 표현할 수 있을 것 같아요! 👍 👍
| @AllArgsConstructor(access = AccessLevel.PRIVATE) | |
| @AllArgsConstructor(access = AccessLevel.PRIVATE, staticName = "from") |
There was a problem hiding this comment.
그렇다면 이전 리뷰를 남기신 access = AccessLevel.PRVIATE는 삭제 해야 됩니다! (테스트 환경에서 사용해야 하니깐요!)
해당 부분 access삭제하고 staticName = from으로 수정하였습니다~
반영 커밋: 79bea8d
| import lombok.Getter; | ||
| import lombok.NoArgsConstructor; | ||
|
|
||
| @Getter | ||
| @AllArgsConstructor(access = AccessLevel.PRIVATE) |
There was a problem hiding this comment.
그렇다면 이전 리뷰를 남기신 access = AccessLevel.PRVIATE는 삭제 해야 됩니다! (테스트 환경에서 사용해야 하니깐요!)
해당 부분 access삭제하고 staticName = from으로 수정하였습니다~
반영 커밋: 79bea8d
| @SpringBootTest | ||
| @AutoConfigureMockMvc | ||
| @AutoConfigureRestDocs | ||
| public abstract class IntegrationCrewAlarmTest { |
There was a problem hiding this comment.
해당 부분을 IntegrationAlarmTest로 통일하고 CrewAlarmControllerTest와 GameAlarmControllerTest 및 Document를 모두 IntegrationAlarmTest를 사용하는 AlarmConrollerTest,AlarmDocumnetTest로 만들어야 하는 피드백인줄 알았습니다.
해당 부분 반영하여 수정하였습니다~
반영 커밋: accdb6d
|
Kudos, SonarCloud Quality Gate passed! |
👨💻 작업 사항
📑 PR 개요
✅ 작업 목록