[TEST/#299] 배너 생성 컨트롤러 테스트코드 작성

[sung-silver]

Related Issue 🚀

• closed [TEST] 배너 생성 테스트 작성 #299

Work Description ✏️

• 배너 생성 작업에 대한 컨트롤러 단 테스트코드를 작성했습니다

PR Point 📸

• 오빠들이 작업한 API는 모두 when(bannerService.getBannerDetail(MOCK_BANNER_ID)).thenReturn(mockBannerDetail); 부분에서 모킹한 getBannerDetail 메서드를 사용하고 있는데요 이 부분이 현재 모든 테스트 실행 이전에 동작하는 @BeforeEach에 정의되어 있습니다
• 그러나 제 API는 생성과 관련되어 있기 때문에 해당 부분이 필요 없다는 생각이 들었습니다 그래서 서비스 단 모킹 코드를 각 테스트에서 진행하는 것은 어떤지 궁금합니다
• 현재는 제 코드에서 이미 모킹이 진행된다는 가정하에 getBannerDetail을 사용하긴 했는데, 이 과정 없이 givenBannerDetail의 값과 비교하는 과정만으로도 충분한 테스트가 진행된다고 생각해요 오빠들의 생각이 궁금합니다!
• 그리고 현욱오빠 테스트코드에서 (DELETE) Banner Detail 부분에 mockBannerDetail 변수는 필요없는 코드인 것 같은데 어떻게 생각하시나요?

[height]

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

[yummygyudon]

고생하셨습니다 :)

몇 가지 답변을 드려볼게요!

그러나 제 API는 생성과 관련되어 있기 때문에 해당 부분이 필요 없다는 생각이 들었습니다 그래서 서비스 단 모킹 코드를 각 테스트에서 진행하는 것은 어떤지 궁금합니다

우선 부분적으로 전체의 BeforeEach가 불필요한 케이스도 다수 존재할 수 있다고 생각합니다.
다만 반대로 불필요한 케이스로 인해 중복되는 설정 코드를 테스트마다 작성하게 되면 중복 코드가 늘어날 것이라고 생각합니다.

이에 대해선 동일한 도메인/서비스에 대한 상위 Test 클래스에서 @Nested를 적용한 하위 Sub Test로 관심사를 분리해서 진행한다면
충분히 아래와 같이 관리할 수 있을 것 같습니다

• 공통의 BeforeEach가 필요한 관심사 = 속한 Nested 클래스 내부에서 공통의 BeforeEach 적용
• 공통의 BeforeEach가 필요없는 관심사 = 속한 Nested 클래스 내부에서 공통의 BeforeEach 적용없이 각 테스트 내부에서 설정

또한 상수 값은 BeforeEach와는 별개로 활용될 수 있기 때문에 최상위 클래스에 static final로 선언해두는 것이 좋을 것 같고
"저장" 혹은 "모킹"은 위에 말한 방식으로 상수를 활용해 적용하는 방식으로 진행하면 좋을 것 같아요!

[yummygyudon]

현재는 제 코드에서 이미 모킹이 진행된다는 가정하에 getBannerDetail을 사용하긴 했는데, 이 과정 없이 givenBannerDetail의 값과 비교하는 과정만으로도 충분한 테스트가 진행된다고 생각해요 오빠들의 생각이 궁금합니다!

저는 생각이 조금 다른 것 같습니다.
테스트는 테스트마다의 목적과 의미가 다르다고 생각하는데요!

사실 본 테스트의 경우, "API"(Controller)에 대한 테스트라고 생각이 되는데 해당 단계에서는 가장 중요한 목적이 "Client는 Request에 대해 의도한 형식의 Response를 수신하는가" 검증하는 것이기 때문에
앞서 말씀해주신 "givenBannerDetail의 값과 비교"는 다소 의도와는 다른 검증이라고 생각이 들어요!
(예를 들어, Service에서는 의도한 결과가 반환되었지만 DTO 변환 과정에서 문제가 발생해 의도치 않은 결과가 반환되거나 메시지와 같이 의도한 데이터가 반환되지 않는 경우를 위해 테스트한다고 생각해요)

더불어 "givenBannerDetail의 값과 비교"는 "해당 어플리케이션은 의도한대로 구현되어 의도한 결과값이 반환되는가"가 목적이고 "Service" 레이어를 검증하기위한 의도에 적합하다고 생각합니다.

[sung-silver]

현재는 제 코드에서 이미 모킹이 진행된다는 가정하에 getBannerDetail을 사용하긴 했는데, 이 과정 없이 givenBannerDetail의 값과 비교하는 과정만으로도 충분한 테스트가 진행된다고 생각해요 오빠들의 생각이 궁금합니다!

저는 생각이 조금 다른 것 같습니다. 테스트는 테스트마다의 목적과 의미가 다르다고 생각하는데요!

사실 본 테스트의 경우, "API"(Controller)에 대한 테스트라고 생각이 되는데 해당 단계에서는 가장 중요한 목적이 "Client는 Request에 대해 의도한 형식의 Response를 수신하는가" 검증하는 것이기 때문에 앞서 말씀해주신 "givenBannerDetail의 값과 비교"는 다소 의도와는 다른 검증이라고 생각이 들어요! (예를 들어, Service에서는 의도한 결과가 반환되었지만 DTO 변환 과정에서 문제가 발생해 의도치 않은 결과가 반환되거나 메시지와 같이 의도한 데이터가 반환되지 않는 경우를 위해 테스트한다고 생각해요)

더불어 "givenBannerDetail의 값과 비교"는 "해당 어플리케이션은 의도한대로 구현되어 의도한 결과값이 반환되는가"가 목적이고 "Service" 레이어를 검증하기위한 의도에 적합하다고 생각합니다.

그러면 제 코드에서 givenBannerDetail은 서비스 코드로부터 조회해오는 것이 아닌 service 레이어에서 생성 이후에 반환되는 값을 모킹하여 그 값이 반환된 형태를 담도록 수정하고, 현욱오빠 코드에서만 bannerService 모킹 부분을 deleteBanner로 수정한 후 머지하면 될까요?

제 코드만 수정하기 전에 아래 첨부해 미리 보여드리겠습니다!

@Nested
    class CreateBannerTests {
        @Test
        @DisplayName("(POST) New Banner")
        void createNewBanner() throws Exception {
            // given
            BannerCreate bannerCreate = new BannerCreate("pg_community", "product", "publisher",
                    LocalDate.of(2024, 1, 1), LocalDate.of(2024, 12, 31), "link", "image-url-pc", "image-url-mobile"
            );
            BannerResponse.BannerDetail mockBannerDetail = new BannerResponse.BannerDetail(
                    MOCK_BANNER_ID, "in_progress", "pg_community", "product", "publisher", "link",
                    LocalDate.of(2024, 1, 1), LocalDate.of(2024, 12, 31), "image-url-pc", "image-url-mobile"
            );
            String request = objectMapper.writeValueAsString(bannerCreate);
            when(bannerService.createBanner(bannerCreate))
                    .thenReturn(mockBannerDetail);
            BannerResponse.BannerDetail givenBannerDetail = bannerService.createBanner(bannerCreate);

            mockMvc.perform(
                            // when
                            post("/api/v1/banners")
                                    .contentType(APPLICATION_JSON)
                                    .content(request))
                    // then
                    .andExpect(status().isCreated())
                    .andExpect(jsonPath("$.success").value("true"))
                    .andExpect(jsonPath("$.message").value(BannerSuccessCode.SUCCESS_CREATE_BANNER.getMessage()))
                    .andExpect(jsonPath("$.data.id").value(givenBannerDetail.bannerId()))
                    .andExpect(jsonPath("$.data.status").value(givenBannerDetail.bannerStatus()))
                    .andExpect(jsonPath("$.data.location").value(givenBannerDetail.bannerLocation()))
                    .andExpect(jsonPath("$.data.content_type").value(givenBannerDetail.bannerType()))
                    .andExpect(jsonPath("$.data.publisher").value(givenBannerDetail.publisher()))
                    .andExpect(jsonPath("$.data.link").value(givenBannerDetail.link()))
                    .andExpect(jsonPath("$.data.start_date").value(givenBannerDetail.startDate().toString()))
                    .andExpect(jsonPath("$.data.end_date").value(givenBannerDetail.endDate().toString()))
                    .andExpect(jsonPath("$.data.image_url_pc").value(givenBannerDetail.pcImageUrl()))
                    .andExpect(jsonPath("$.data.image_url_mobile").value(givenBannerDetail.mobileImageUrl()));
        }
    }

[yummygyudon]

앗 제가 남긴 리뷰를 남겼던 이유는
성은이가 "mockMvc로 의도한 응답이 나오는지에 대해 검증하는 것은 무의미하고 service 단에서 return 객체에 대해서만 검증해도 무관하지 않는가"라고 질문했다고 판단했기 때문이에요!

그래서 테스트 코드에 대해서는 별도의 리뷰를 안달았던 것이었구요!!

[sung-silver]

앗 제가 남긴 리뷰를 남겼던 이유는 성은이가 "mockMvc로 의도한 응답이 나오는지에 대해 검증하는 것은 무의미하고 service 단에서 return 객체에 대해서만 검증해도 무관하지 않는가"라고 질문했다고 판단했기 때문이에요!

그래서 테스트 코드에 대해서는 별도의 리뷰를 안달았던 것이었구요!!

저의 의도는 mockMvc으로 반환되는 값의 검증은 필요하되 그 비교되는 값을 bannerSerivce.getBanner로 가져오지 않아도 될 것 같다는 의미였습니다! 텍스트로 좀 더 잘 전달해야했는데 부족했던 것 같네요 ^^.. 죄송합니다 Nested를 사용함으로써 beforeEach에서 모킹한 값을 꼭 사용해야할 것 같다는 생각이 없어진 부분이라 넘어가도 될 것 같습니다!