테스트하기 안 좋은 코드 리팩토링하기

삼성청년SW아카데미 11기에서 진행한 도서 공유 프로젝트가 마무리 되었습니다.

 

프로젝트가 마무리 되었으니 문제점이라고 생각한 부분을 리팩토링을 하였습니다.

 

리팩토링의 목표는 테스트 하기 좋지 않은 코드를 테스트하기 좋은 코드로 바꾸는 것이었습니다.

 

💡 테스트하기 좋지 않은 코드란?

테스트 코드는 항상 같은 결과를 return 하는 멱등성이 보장되어야 합니다.
만약 Rental을 생성하는 테스트 코드를 작성했는데, 이 코드에 멱등성이 보장되지 않은 코드가 있고,
이 코드로 인해 어떤 경우에는 테스트가 성공하고, 어떤 경우에는 테스트가 실패한다면 이는 테스트로서의 기능을 못하고 있는 것입니다.
따라서 멱등성을 보장하지 못하는 코드는 테스트 비용을 지불하여 멱등성을 보장해주어야 합니다.
이때의 방법은 Mocking, Stubbing, 테스트 데이터베이스 사전 준비 등등이 있습니다.

따라서 테스트하기 좋은 코드란 멱등성을 보장하는 순수함수에 가까운 코드가 테스트하기 좋은 코드라고 생각합니다.

---

문제의 코드

    @Transactional
	public RentalOfferResponse createRental(Long userbooksId) {
		Userbook userbook = validateAndFindUserbook(userbooksId);
		User currentUser = validateCanRentalAndFindCurrentUser();
		checkIsNotDuplicated(currentUser, userbook, OFFERING);

		checkIsNotOwner(userbook, currentUser);

		Rental rental = Rental.builder().user(currentUser).userbook(userbook).rentalStatus(OFFERING).build();
		Rental savedRental = this.rentalRepository.save(rental);

		return new RentalOfferResponse(savedRental.getId());
	}
	
	private Userbook validateAndFindUserbook(Long userbooksId) {
		Userbook userbook = this.userbookRepository.findById(userbooksId)
			.orElseThrow(() -> new ErrorException(ErrorCode.NotFound));
		if (!userbook.isAvailable()) {
			throw new ErrorException(ErrorCode.InvalidAccess);
		}
		return userbook;
	}
	
	private User validateCanRentalAndFindCurrentUser() {
		User currentUser = this.userHelper.findCurrentUser();
		if (!this.userPersonalFacade.checkPointToRental(currentUser.getId())) {
			throw new ErrorException(ErrorCode.NotEnoughPoint);
		}

		return currentUser;
	}
	
	private void checkIsNotDuplicated(User currentUser, Userbook userbook, RentalStatus status) {
		boolean existsRentalByRentalStatus = this.rentalRepository.existsRentalByRentalStatus(currentUser.getId(),
			userbook.getId(), status);

		if (existsRentalByRentalStatus) {
			throw new ErrorException(ErrorCode.NotAcceptDuplicate);
		}
	}
	
	private void checkIsNotOwner(Userbook userbook, User currentUser) {
		if (userbook.getUser().equals(currentUser)) {
			throw new ErrorException(ErrorCode.BadRequest);
		}
	}

 

조금 길 수도 있지만 해당 코드는 Userbook의 상태와 User의 포인트 잔여액에 따라 Rental을 생성하는 코드입니다.

도메인 규칙은 다음과 같습니다.

• Userbook 이 거래 가능 상태가 아니라면 Rental 을 생성할 수 없다.
• Userbook 의 소유자는 자신의 Userbook 에 대해 Rental 을 생성할 수 없다.
• 이미 대여 신청을 보낸 책이라면 중복으로 Rental 을 생성할 수 없다.
• 회원의 포인트가 부족하면 Rental 을 생성할 수 없다.

 

현재의 코드를 테스트 하기 위해 간단한 테스트 코드를 작성하였습니다.

	@DisplayName("회원도서에 대해 대여를 신청한다")
	@Test
	void createRental() {
		// given
		User customer = cretaeCustomer();
		createPoint(customer);
		User owner = createOwner();
		Userbook userbook = createUserbook(owner);
		
		given(this.userHelper.findCurrentUser()).willReturn(customer);

		// when
		RentalOfferResponse rentalOfferResponse = this.rentalService.rentalOffer(userbook.getId());

		// then
		assertThat(rentalOfferResponse.rentalId()).isNotNull();
	}

 

• userHelper : SecurityContext 에 저장된 Authentication 을 가지고 UserRepository 에서 User를 조회해주는 헬퍼 클래스입니다. 
  • Mocking 을 하여 도서를 대여하고자 하는 customer 를 return 하게 해주었습니다.
• createCustomer, createPoint, createOwner, createUserbook : 테스트에 필요한 데이터를 데이터베이스에 저장한 뒤 return 하는 함수입니다. 테스트 환경에서는 인메모리 DB인 H2 를 사용하고 있습니다. 
  • 이 과정에서 매번 테스트마다 데이터베이스에 데이터를 생성하며 이는 테스트 속도를 저하시키는 원인이 될 수 있습니다.

테스트 실행 속도

 

실행 속도를 보시면 성공하는 하나의 케이스를 테스트하는데 252ms 가 소요되었습니다.

하지만 여기서 만약 비즈니스 조건에 맞게 실패하는 케이스를 작성해야 한다면 어떻게 될까요?

위의 코드처럼 테스트 비용이 높은 코드를 작성할 수 밖에 없고, 테스트 속도는 점점 늘어나게 될 것입니다.

이는 배포시에 더 많은 시간이 소요된다는 의미이기도 합니다.

 

따라서 저는 순수함수로 분리할 수 있는 코드는 분리하기로 했습니다.

---

팩토리 패턴

Rental이 생성되는 규칙은 다음과 같습니다.

• Userbook 이 isAvailble 일 경우
• Userbook의 소유자가 아닌 경우

저는 이 부분이 Userbook의 도메인으로 들어가면 테스트 하기 조금은 용이해질 것이라고 생각하여 Userbook이 Rental을 생성하는 팩토리 클래스가 되도록 리팩토링을 했습니다. 

그리고 도메인 주도 설계 관점에서 Rental은 Userbook의 하위도메인이기에 이는 자연스러운 코드 스타일이라고 생각했습니다.

 

public Rental createRental(User user) {
    Assert.notNull(user, "유저가 없습니다.");

    if (this.user.getId().equals(user.getId())) {
       throw new IllegalStateException("동일한 유저입니다");
    }

    if (!isAvailable()) {
       throw new IllegalStateException("접근할 수 없는 상태입니다");
    }
    return Rental.builder()
       .user(user)
       .userbook(this)
       .rentalStatus(OFFERING)
       .build();
}

 

Userbook 의 상태와 관련된 로직을 Userbook 안에 위치함으로써 도메인 응집도는 올라가고, 테스트 하기 용이한 코드가 되었습니다.

이를 바탕으로 Userbook의 상태에 따른 Rental 을 생성하는 테스트 코드를 작성해보았습니다.

 

class UserbookTest {

    @DisplayName("회원도서의 상태가 접근불가한 상태가 아니라면 대여를 생성한다")
    @ParameterizedTest
    @MethodSource
    void createRental(RegisterType registerType, TradeStatus tradeStatus) {
       // given
       Book book = createBook();
       User owner = createUser(1L);
       User customer = createUser(2L);

       Userbook userbook = createUserbook(book, owner, registerType, tradeStatus);

       // when
       Rental rental = userbook.createRental(customer);

       // then
       assertAll(
          () -> assertThat(rental.getRentalStatus()).isEqualTo(RentalStatus.OFFERING),
          () -> assertThat(rental.getUser().getId()).isEqualTo(customer.getId()),
          () -> assertThat(rental.getStartDate()).isNull(),
          () -> assertThat(rental.getEndDate()).isNull()
       );
    }


    @DisplayName("회원도서의 상태가 접근불가한 상태라면 에러를 발생한다")
    @ParameterizedTest
    @MethodSource
    void createRental_fail(RegisterType registerType, TradeStatus tradeStatus) {
       // given
       String errorMessage = "접근할 수 없는 상태입니다";
       Book book = createBook();
       User owner = createUser(1L);
       User customer = createUser(2L);

       Userbook userbook = createUserbook(book, owner, registerType, tradeStatus);

       // expected
       IllegalStateException exception = assertThrows(IllegalStateException.class,
          () -> userbook.createRental(customer));
       assertThat(exception.getMessage()).isEqualTo(errorMessage);
    }

    private static Book createBook() {
       return Book.builder()
          .build();
    }

    private static Stream<Arguments> createRental() {
       return Stream.of(
          Arguments.of(RegisterType.RENTAL, RegisterType.RENTAL.getDefaultTradeStatus()),
          Arguments.of(RegisterType.EXCHANGE, RegisterType.EXCHANGE.getDefaultTradeStatus()),
          Arguments.of(RegisterType.RENTAL_EXCHANGE, RegisterType.RENTAL_EXCHANGE.getDefaultTradeStatus())
       );
    }

    private static Stream<Arguments> createRental_fail() {
       return Stream.of(
          Arguments.of(RegisterType.RENTAL, TradeStatus.RENTED),
          Arguments.of(RegisterType.EXCHANGE, TradeStatus.EXCHANGED),
          Arguments.of(RegisterType.RENTAL_EXCHANGE, TradeStatus.UNAVAILABLE),
          Arguments.arguments(RegisterType.INACTIVE, TradeStatus.RENTAL_AVAILABLE)
       );
    }

    private static Userbook createUserbook(Book book, User owner, RegisterType registerType, TradeStatus tradeStatus) {
       return Userbook.builder()
          .book(book)
          .user(owner)
          .registerType(registerType)
          .tradeStatus(tradeStatus)
          .build();
    }

    private static User createUser(long id) {
       return User.builder()
          .id(id)
          .build();
    }
}

 

• 기존의 테스트 코드는 @SpringBootTest 어노테이션이 붙은 통합테스트였습니다.
• @SpringBootTest 는 실제 스프링 어플리케이션이 실행되면서 모든 빈들이 등록됩니다.
• 하지만 리팩토링을 통한 현재 코드는 순수 자바 코드로 이루어져있어 @SpringBootTest 보다 실행속도가 빠릅니다.

테스트 실행 속도

리팩토링을 진행하고, Userbook 과 관련된 로직에 대한 검증으로 9건의 테스트를 진행한 결과 201ms 가 소요되었습니다.

정상적인 케이스 1건에 걸린 시간이 252ms 인 것에 비교하여 훨씬 빠른 속도를 보여주고 있습니다.

 

기존의 코드는 아래와 같이 변경되었습니다.

@Transactional
public RentalOfferResponse rentalOffer(Long userbooksId) {
    Userbook userbook = validateAndFindUserbook(userbooksId);
    User currentUser = validateCanRentalAndFindCurrentUser();
    checkIsNotDuplicated(currentUser, userbook, OFFERING);

    Rental rental = userbook.createRental(currentUser);

    Rental savedRental = this.rentalRepository.save(rental);

    return new RentalOfferResponse(savedRental.getId());
}

 

이제 서비스 클래스의 createRental은 Rental 을 생성하기 전에 User의 포인트만 체크하고, 중복된 Rental이 없는 책임만 갖게 되었습니다.

이는 곧 createRental 의 테스트 케이스가 줄어들게 되었고, 테스트 속도가 개선되었음을 뜻하기도 합니다.

---

참고자료

• 도메인 주도 개발 시작하기

도메인 주도 개발 시작하기: DDD 핵심 개념 정리부터 구현까지 | 최범균 - 교보문고

• https://jwchung.github.io/testing-oh-my

Testing, Oh my!