카카오테크캠퍼스 4주차 코드리뷰

PR

안녕하세요 멘토님! :)
작성한 코드에 대해 부족한 점, 잘못된 점, 잘한 점 등 아낌없이 조언해주시면 감사드리겠습니다! 😊

 

 

코드 작성하면서 어려웠던 점

• BDDMockito 사용시 given 패턴에서 인자 설정 오류으로 인해 Stubbing이 되지 않는 문제가 발생하였고,
  any()와 anyInt()를 이용해 해결했습니다.
• Controller 테스트에서 Spring Security의 userDetails 값을 불러오지 못해 문제가 발생하였고, @WithMockUser로는 해결이 되지 않았습니다.
  • 직접 Authentication 객체를 생성하여 주입하고, SecurityContextHolder의 Context에 설정하여 해결했습니다.
• Bean Validation에서 @NotEmpty, @NotBlank가 문자열에만 적용됨을 알았고, @NotNull은 기본 타입에서는 적용되지 못한다는 것을 알았습니다.
• Controller에서 처리하지 않는 요청에 대해 Filter를 이용해 응답을 처리하려고 하였으나 filter chain 부분에서 오류를 해결하지 못했습니다. 다음주 aop를 배울때 filter도 같이 공부할 예정입니다.

 

3주차 과제 개선 중 Question

✚ 코드 리뷰를 반영하여 3주차 과제를 개선했습니다. 😼

• 유연한 테스트 코드 작성을 위해 코드를 수정하다가 궁금한 점이 생겼습니다.위 부분에서id를 repository에서 조회해서 가져와 하드코딩된 id가 존재하지 않을 경우를 방지하였습니다.

@Test
@DisplayName("2. 개별 상품 상세 조회 : Lazy")
public void option_mFindByProductId_lazy_test() throws JsonProcessingException {
    // given
    Option option = optionJPARepository.findAll().get(0);
    int id = option.getProduct().getId();

    // when
    List<Option> optionListPS = optionJPARepository.mFindByProductId(id); // Lazy, join fetch 사용해서 연관관계 객체 가져오기

    System.out.println("json 직렬화 직전========================");
    String responseBody = om.writeValueAsString(optionListPS);
    System.out.println("테스트 : "+responseBody);

    // then
}

그런데 이렇게 코드를 작성할 경우 DB에 2번이나 쿼리를 날려야하므로 비효율적이라고 생각했습니다. 🥲
좀 더 개선할 수 있는 부분이 있을지 여쭙고 싶습니다.

 

Answer

테스트 코드에서 쿼리를 1번 더 사용하는 부분보다 테스트 코드를 유연하게 만드는 것이 우선순위가 높다고 생각합니다.
테스트 코드는 테스트 실행시에만 실행되기 때문에 괜찮다고 생각다고 생각됩니다. 
개선한다고 하면 @BeforeAll 이나 @beforeeach 에서 ID를 찾아와서 변수로 선언하여 사용해보는 것은 어떨까요?

 

✅ 반영하여 수정하였습니다.

 

코드 리뷰시 중심적으로 리뷰해주셨으면 하는 부분

stub을 @BeforeEach에 모아서 처리하는 방안

• Controller 단위 테스트시 각 테스트마다 중복되는 대량의 stub 코드가 있음을 발견했습니다.
• @BeforeEach로 모아서 작성하는 것이 코드의 중복을 줄일 수 있겠지만, 테스트간의 격리성이 떨어질까 고민이 됩니다.
• 또한 모든 테스트가 같은 stub을 사용하는 것이 아니라 5개의 테스트 메서드에서 3개만 사용하는 경우도 있습니다.
• 그리고 @BeforeEach로 모아 작성할 경우 Mockito의 given을 함께 작성해야하는데,
  테스트시불필요한 로딩이 발생할까 걱정이 됩니다. 🥹
• 멘토님의 생각을 듣고 싶습니다!

 

Answer

현재 구현한 @beforeeach 에 중복되는 코드를 stub 코드로 잘 구현 하였습니다~!
stub에서 데이터를 저장하고 id를 저장 하는 경우 저장된 id 를 사용하면 격리성있게 잘 실행될것으로 보입니다.

일부 메소드만 같은 stub를 사용하는 경우 test class 를 구분 하여 사용 하는 것도 방법이 될꺼같습니다.
1번만 실행되어야 하는 경우 @beforeall 도 사용합니다.
@beforeeach로 모아 작성할 경우 Mockito의 given 은 격리성 있게 각 테스트코드마다 실행되어야 하므로 테스트 실행시 실행되는것이 맞는것으로 생각됩니다.

 

✅ 불필요한 로딩을 걱정하기보다 테스트마다 격리된 데이터가 더 중요하다.

✅ 테스트마다 테스트클래스를 만들어 각각에 맞는 데이터로 사용할 수 있다.

 

stub code를 각각 적용한 테스트 코드

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import java.util.stream.Stream;

public class ExampleTest {

    @BeforeEach
    void setup() {
        // 초기 설정 코드
    }

    @ParameterizedTest
    @MethodSource("provideTestData1") //stub 메서드 1 적용
    @DisplayName("Test with Stub Data 1")
    public void testWithStubData1(int number, String text) {
        System.out.println("Test with Stub Data 1 - Number: " + number + ", Text: " + text);
    }

    @ParameterizedTest
    @MethodSource("provideTestData2") //stub 메서드 2 적용
    @DisplayName("Test with Stub Data 2")
    public void testWithStubData2(int number, String text) {
        System.out.println("Test with Stub Data 2 - Number: " + number + ", Text: " + text);
    }
	//stub 메서드 1
    private static Stream<Arguments> provideTestData1() {
        return Stream.of(
                Arguments.of(1, "One"),
                Arguments.of(2, "Two"),
                Arguments.of(3, "Three")
        );
    }
	//stub 메서드 2
    private static Stream<Arguments> provideTestData2() {
        return Stream.of(
                Arguments.of(4, "Four"),
                Arguments.of(5, "Five"),
                Arguments.of(6, "Six")
        );
    }
}

@ParameterizedTest으로 매개변수를 순서대로 넘겨줄 수 있고 테스트가 매개변수를 포함한 테스트임을 알린다. @MethodSource("provideTestData2")로 stub 메서드를 지정할 수 있다.

📌 위 코드에서는 ParameterizedTest에 인자를 제공하지 않고 메서드에서 제공한다.

 

@ParameterizedTest에서 어노테이션을 사용하여 직접 인자를 넘겨줄 수도 있다.

@ParameterizedTest
@ValueSource(ints = {1, 3, 5, -3, 15, Integer.MAX_VALUE})
void isOdd_ShouldReturnTrueForOddNumbers(int number) {
    // Test logic
    // ...
}

 

자세하게 정리된 블로그를 참고하여 공부하면 좋을 것 같다.

https://velog.io/@ohzzi/junit5-parameterizedtest

[JUnit5] @ParameterizedTest로 한 번에 테스트하자

 

400 에러 vs 404 에러

• 400(Bad Request)는 잘못된 요청(문법 오류)을, 404(Not Found)는 요청 리소스를 찾을 수 없음을 뜻한다고 알고 있습니다.
• 만약 클라이언트가 DB에 저장되어있지 않은 id로 API 요청시 ex)/orders/1
  400과 404중 어떤 에러를 표시해야하는지 궁금합니다.
• 차이를 알아보던 중 404 에러는 DB 자원을 의미하는 것이 아니라 API URI 요청 경로를 의미한다라는 글을 보았습니다.
  해당 글
• 404 에러는 경로로 해석하는 경우가 더 많아 400번 에러를 사용한다고 하였습니다.
• 하지만 제 생각으로는 API 요청 경로로 DB 조회를 하니 이 둘을 같지 않을까라는 생각이 듭니다. 😭..🤯
• 멘토님께서는 400 에러와 404 에러를 어떤 기준으로 구분하여 사용하시는지 궁금합니다.

 

Answer

말씀해주신대로 일반적으로 404는 API URI 경로가 없는 경우만 주로 사용합니다. 
API를 이용하는 입장에서 생각해보면 API 경로가 없는 것인지 데이터가 없는 것인지 구분이 필요합니다.
구분하는 방법으로는 400 또는 404 status code에 response 에 errorCode 또는 메시지를 추가하여 응답을 합니다.

일부 API 서버에서는 데이터가 없더라도 200 status 에 errorCode로 데이터가 없을때의 코드를 응답해주기도 합니다.

저는 수정 하는 API라고 했을때 수정할수 없는 ID 를 요청한 경우 잘못된 ID를 요청했다고 판단하여 400 에러를 사용하고 있습니다.

 

 

항상 감사드립니다! 😊

 

코드 리뷰 반영

✅ 코드 리팩토링

✅  @ParameterizedTest, @MethodSource 공부

✅ 변수명 수정

 

이번주도 완료 :)