티스토리 뷰

지난 4월 3일부터 8.5주간 NEXTSTEPTDD, 클린 코드 with Java를 수강하게 되었다. 여타 강의와 다르게 해당 강의는 수동적으로 강의를 듣고 끝나는 것이 아니라, 스스로 미션을 수행하고 코드 리뷰를 받으며 진행되는 커리큘럼이다.

 

이 과정을 그냥 흘려보내기엔 아쉬울 것 같다는 생각이 들어, 코드 리뷰를 받은 내용 위주로 포스팅을 하려고 한다. 회사에서 어떻게 하면 코드 리뷰 문화를 정착시킬 수 있을까를 고민하고 있는데, 이 과정을 통해 그에 대한 힌트를 얻을 수 있기를 🔥

 

 

 

1. Collection 검증

리뷰 전

// size()
int size = numbers.size();
assertThat(size).isEqualTo(3);

// contains
assertThat(numbers.contains(value)).isTrue();

 

개선

// size()
assertThat(numbers).hasSize(3);

// contains()
assertThat(numbers).contains(value);

 

assertj에는 Collection에 대한 검증 기능도 지원하고 있는데, 기계적으로 Set의 size()와 contains() 메소드를 직접 사용하여 검증을 했다 🤦‍♂️

 

리뷰 전의 테스트 코드가 에러는 아니지만, 라이브러리에서 제공해주는 기능을 활용하면 코드가 간결해지고 가독성을 높일 수 있으니 참고하자.

 

 

2. 테스트 픽스처

리뷰 코드

private Set<Integer> numbers;

@BeforeEach
void setUp() {
	  numbers = new HashSet<>();
	  numbers.add(1);
	  numbers.add(1);
	  numbers.add(2);
	  numbers.add(3);
}

 

요구사항에서 주어진 샘플 코드를 그대로 사용했는데 그에 대해 테스트 픽스처 구성에 대한 참고 자료를 피드백으로 주셨고, 내용은 아래에 간략히 정리해두었다. JUnit을 학습하기 위한 실습이었기에, 따로 수정은 하지 않았지만 추후에 본격적인 과제를 하게 된다면 테스트 픽스처 구성에 대한 의식적인 연습을 해야겠다.

 

 

테스트 픽스처란?

테스트 대상 시스템(SUT: System Under Test)을 실행하기 위해 해줘야 하는 모든 것을 테스트 픽스처라고 한다.

 

잘못된 테스트 픽스처의 문제점

  1. 테스트 메서드를 확인하려면 테스트 픽스처(예시에서는 @BeforeEach)의 코드를 살펴봐야 한다.
  2. 테스트 픽스처에 모든 테스트가 결합되어 버린다. 즉, 테스트 픽스처를 수정할 경우 모든 테스트에 영향을 미친다. → 회사에서 비슷한 경험이 있었다 😲

해결

  1. 클래스 내부에 private 팩토리 메소드를 만들어 활용
  2. 클래스 외부에 static 팩토리 메소드를 만들어 활용

 

 

3. @Parameterized

리뷰 전

@Test
void split1() {
    String str = "1";
    String[] split = str.split(",");
    assertThat(split).containsExactly("1");
}

@Test
void split2() {
    String str = "1,2";
    String[] split = str.split(",");
    assertThat(split).containsExactly("1", "2");
}

 

개선

@ParameterizedTest
@MethodSource("stringInputAndSplitArray")
void split(String input, String[] expected) {
    String[] split = input.split(",");
    assertThat(split).isEqualTo(expected);
}

static Stream<Arguments> stringInputAndSplitArray() {
    return Stream.of(
            Arguments.arguments("1", new String[]{"1"}),
            Arguments.arguments("1,2", new String[]{"1","2"})
    );
}

 

비슷한 유형의 테스트에서는 @ParameterizedTest 어노테이션 사용 제안을 해주셨다. 요구사항에 있는 그대로 구현을 하다보니, 멍청이처럼 수동적으로만 생각했다 🤦‍♂️

 

 

4. 하나의 테스트 케이스는 하나의 시나리오만 테스트

리뷰 전

@Test
void charAt() {
    String str = "abc";

    assertThat(str.charAt(0)).isEqualTo('a');
    assertThat(str.charAt(1)).isEqualTo('b');
    assertThat(str.charAt(2)).isEqualTo('c');

    assertThatThrownBy(() -> str.charAt(3))
            .isInstanceOf(IndexOutOfBoundsException.class)
            .hasMessage("String index out of range: 3");
}

 

개선

@ParameterizedTest
@CsvSource(value = {"abc,0,a", "abc,1,b", "abc,2,c"})
void charAt(String input, int index, char expected) {
    assertThat(input.charAt(index)).isEqualTo(expected);
}

@Test
void charAt_IndexOutOfBoundsException() {
    String str = "abc";
    assertThatThrownBy(() -> str.charAt(3))
            .isInstanceOf(IndexOutOfBoundsException.class)
            .hasMessage("String index out of range: 3");
}

 

정상 케이스에 대한 테스트와 에러 케이스에 대한 테스트를 한번에 작성 했는데, 테스트 코드에 여러 시나리오를 작성할 경우 테스트 케이스가 실패했을 때 어떤 시나리오에서 문제가 발생했는지 파악하기 어려워지므로 리뷰어님의 제안에 따라 테스트를 분리했다.

 

추가로 테스트를 분리함에 따라 정상 케이스에 대한 테스트에 @ParameterizedTest 어노테이션을 활용할 수 있다.

 

 

 

pull request 🙇‍♂️

참고 자료 🙇‍♂️

댓글