좋지 않은 테스트 판별하기

좋지 않은 테스트를 작성하는 것보다는 테스트를 작성하지 않는 것이 좋다.

가치가 별로 없는 테스트는 좋지 않은 테스트다.

실제 프로젝트에서 사용하고 있는 테스트를 검토하며 좋은 테스트인지 좋지 않은 테스트인지 판별해보겠다.

---

 

첫번째 케이스

AS-IS

	@Test
	@DisplayName("현재시간이 요일에 맞는 영업시간에 포함하면 open이다")
	void contains_Nowtime_Then_Open() {
		//given
		LocalDateTime now = LocalDateTime.of(2024, 1, 29, 12, 0, 0);
		DayOfWeek dayOfWeek = now.getDayOfWeek();
		LocalTime startTime = LocalTime.of(9, 0);
		LocalTime endTime = LocalTime.of(21, 0);
		//when
		boolean isOpen = openChecker.checkByNowTime(dayOfWeek, startTime, endTime, now);
		//then
		assertThat(isOpen).isTrue();
	}

	@Test
	@DisplayName("현재시간이 요일에 맞는 영업 시작시간이랑 일치하면 open이다")
	void when_NowTime_Is_StartTime_Then_Open() {
		//given
		LocalDateTime now = LocalDateTime.of(2024, 1, 29, 9, 0, 0);
		DayOfWeek dayOfWeek = now.getDayOfWeek();
		LocalTime startTime = LocalTime.of(9, 0);
		LocalTime endTime = LocalTime.of(21, 0);
		//when
		boolean isOpen = openChecker.checkByNowTime(dayOfWeek, startTime, endTime, now);
		//then
		assertThat(isOpen).isTrue();
	}

 

위 두개의 단위 테스트에는 두가지의 단점이 존재한다.

• 테스트 간의 결합도가 높다.
• (꼭 단점이라고 볼 수는 없지만) 두 개의 테스트는 동일한 테스트다.

 

테스트 간의 결합도가 높다.

https://ghffu405.tistory.com/51

[단위 테스트] 테스트 간의 높은 결합도는 안티 패턴이다.

 

 

두 개의 테스트는 다음과 같은 필드를 공유하고 있다.

private OpenChecker openChecker = new OpenChecker();

 

필드로 선언된 openChecker 변수를 두 개의 테스트에서 각각 가져다 쓰고 있다.

 

이는 공유 필드를 통해 상태를 공유하고 있는 셈이다.

실제 코드에서는 상태를 바꾸는 코드는 존재하지 않지만, 지금 현재의 코드일뿐 미래에는 어떻게 변할지 모른다.

 

공유 필드 내 어떠한 코드가 수정이 된다면, 각각의 테스트에 영향을 주게 된다.

완전히 같지는 않지만, 테스트는 독립적으로 실행되어야 한다.

 

결합도를 낮추기 위해서 다음과 같이 코드를 수정할 수 있다.

	@Test
	@DisplayName("현재시간이 요일에 맞는 영업시간에 포함하면 open이다")
	void contains_Nowtime_Then_Open() {
		//given
        	OpenChecker openChecker = new OpenChecker();
		...
	}

	@Test
	@DisplayName("현재시간이 요일에 맞는 영업 시작시간이랑 일치하면 open이다")
	void when_NowTime_Is_StartTime_Then_Open() {
		//given
		OpenChecker openChecker = new OpenChecker();
		...
	}

 

테스트에서 각각의 OpenChecker 인스턴스를 생성한다.

 

두 개의 테스트는 동일한 테스트다.

• 현재시간이 요일에 맞는 영업시간에 포함하면 open이다.
• 현재시간이 요일에 맞는 영업 시간이랑 일치하면 open이다.

두개의 테스트는 위와같은 목적을 가지고 테스트를 진행한다.

openChecker.checkByNowTime(dayOfWeek, startTime, endTime, now);

 

checkByNowTime 메서드는 현재시간이 요일에 맞는 영업시간 내에 존재하는지 판별하는 메서드이다.

영업시간이랑 일치 는 요일에 맞는 영업시간 내 포함 하는 것과 같은 케이스이므로 동일한 테스트로 볼 수 있다.

따라서, 두 개의 테스트는 하나의 테스트로 묶을 수 있다.

 

Junit5의 @ParameterizedTest 와 @MethodSource 어노테이션을 이용해서 하나의 테스트로 묶을 수 있다.

 

매개변수화된 테스트를 사용하면 장단점이 있다.

장점

• 테스트 코드의 양을 크게 줄여 유지보수성이 좋아진다.

단점

• 매개변수가 많을수록 테스트가 나타내는 사실을 파악하기 어려워진다.

매개변수화된 테스트를 사용하면 유지보수성이 좋아지지만 비용이 발생한다.

 

절충안으로 긍정적인 테스트와 부정적인 테스트 두개로 나누자.

입력 매개변수만으로 테스트 케이스를 판단할 수 있다면, 긍정적인 테스트 케이스와 부정적인 테스트 케이스 모두 하나의 메서드로 두는것이 좋다.

동작이 너무 복잡하면 매개변수화된 테스트를 조금도 사용하지 말자.

TO-BE

	@ParameterizedTest
	@MethodSource("provideLocalDateTime")
	@DisplayName("요일에 맞는 영업시간사이에 현재시간이 포함하면 open이다")
	void the_current_time_is_within_business_hours_on_a_specific_day(LocalDateTime now) {
		//given
		DayOfWeek dayOfWeek = now.getDayOfWeek();
		LocalTime startTime = LocalTime.of(9, 0);
		LocalTime endTime = LocalTime.of(21, 0);
		BusinessHourOpenChecker openChecker = new BusinessHourOpenChecker();
		//when
		boolean isOpen = openChecker.checkByNowTime(dayOfWeek, startTime, endTime, now);
		//then
		assertThat(isOpen).isTrue();
	}

	private static Stream<Arguments> provideLocalDateTime() {
		return Stream.of(
			Arguments.of(LocalDateTime.of(2024, 1, 29, 12, 0, 0)),
			Arguments.of(LocalDateTime.of(2024, 1, 29, 9, 0, 0)
			)
		);
	}

---

두번째 케이스

AS-IS

	@DisplayName("영업 중 여부를 판단한다")
	void determines_if_it_is_during_business_hours() {
    		//given
		List<BusinessHour> businessHours = new ArrayList<>();
		BusinessHour monday = 
        	TestBusinessHourFactory.createBusinessHourWithDayAndTime(
            		"MONDAY", LocalTime.of(9, 0), LocalTime.of(21, 0)
                	);
		businessHours.add(monday);
		BusinessHourOpenChecker sut = new BusinessHourOpenChecker();
		//when
		boolean isOpen = sut.checkWithBusinessHours(
        		businessHours, LocalDateTime.of(2024, 1, 29, 9, 0, 0)
            		);
		//then
		assertThat(isOpen).isTrue();
	}

 

sut : 테스트 하는 대상 (System Under Test)

 

위 테스트는 checkWithBusinessHours 메서드를 통해서 영업시간 체크를 한다.

첫번째 테스트는 어땠는가?

checkByNowTime 메서드를 통해서 영업시간 체크를 했다.

 

checkWithBusinessHours 메서드의 구현 세부사항을 보자.

public boolean checkWithBusinessHours(List<BusinessHour> businessHours, LocalDateTime now) {
		if (!hasMatchingDayOfWeek(businessHours, now)) {
			throw new CafegoryException(ExceptionType.CAFE_NOT_FOUND_DAY_OF_WEEK);
		}
		return businessHours.stream()
			.anyMatch(
            	// checkWithBusinessHours에서 checkByNowTime 메서드를 호출하고 있다.
				hour -> checkByNowTime(
                		DayOfWeek.valueOf(hour.getDay()),
                    		hour.getStartTime(),
                    		hour.getEndTime(),
                    		now)
                		);
	}
 
 
 public boolean checkByNowTime(DayOfWeek dayOfWeek, LocalTime businessStartTime, LocalTime businessEndTime,
		LocalDateTime now) {
		LocalTime currentTime = now.toLocalTime();

		boolean isOpenOvernight = isOpenOvernight(businessStartTime, businessEndTime);

		if (!isTodayBusinessDay(dayOfWeek, now, isOpenOvernight)) {
			return false;
		}
		// 24시간 영업인 경우 항상 참
		if (is24HourBusiness(businessStartTime, businessEndTime)) {
			return true;
		}
		// 새벽까지 영업하는 경우
		if (isOpenOvernight) {
			return isOpenOvernightNow(businessStartTime, businessEndTime, currentTime);
		}
		// 같은 날에 영업을 시작하고 종료하는 경우
		return isOpenDuringDay(businessStartTime, businessEndTime, currentTime);
	}

 

위 코드는 실제 프로젝트에서 사용하는 실제 코드이다. 자세히 볼 필요는 없다.

 

checkWithBusinessHours 메서드의 내부를 보면

checkWithBusinessHours에서 checkByNowTime 메서드를 호출하고 있다.

checkByNowTime 메서드는 checkWithBusinessHours 메서드와 테스트에서만 호출하고 있다.

 

checkByNowTime은 비공개 메서드로 되어 있어야 하는데 테스트를 위해 public 메서드로 되어 있다.

이는 테스트는 구현 세부사항을 테스트하지 말아야 한다는 원칙에 위반한다.

 

구현 세부사항을 테스트하고 있는 첫번째 테스트 케이스를 지우고, 두번째 테스트 케이스를 통해서 첫번째 영업여부를 판단해야 한다.

중요한 것은 두번째 테스트 케이스에서 구현 세부사항을 테스트하는 것이 아니라, 테스트 대상의 동작 단위를 테스트하여야 한다.

 

구현 세부사항을 테스트 할 경우, 좋은 단위 테스트의 4대 요소 중 하나인 리팩토링 내성이 낮아진다. 

https://ghffu405.tistory.com/54

[단위 테스트] 리팩토링 내성 (좋은 단위 테스트의 요소)

 

 

TO-BE

//	@ParameterizedTest
//	@MethodSource("provideLocalDateTime")
//	@DisplayName("요일에 맞는 영업시간사이에 현재시간이 포함하면 open이다")
//	void the_current_time_is_within_business_hours_on_a_specific_day(LocalDateTime now) {
//		//given
//		DayOfWeek dayOfWeek = now.getDayOfWeek();
//		LocalTime startTime = LocalTime.of(9, 0);
//		LocalTime endTime = LocalTime.of(21, 0);
//		BusinessHourOpenChecker openChecker = new BusinessHourOpenChecker();
//		//when
//		boolean isOpen = openChecker.checkByNowTime(dayOfWeek, startTime, endTime, now);
//		//then
//		assertThat(isOpen).isTrue();
//	}

//	private static Stream<Arguments> provideLocalDateTime() {
//		return Stream.of(
//			Arguments.of(LocalDateTime.of(2024, 1, 29, 12, 0, 0)),
//			Arguments.of(LocalDateTime.of(2024, 1, 29, 9, 0, 0)
//			)
//		);
//	}
    
    
	@DisplayName("영업 중 여부를 판단한다")
	void determines_if_it_is_during_business_hours() {
    		//given
		List<BusinessHour> businessHours = new ArrayList<>();
		BusinessHour monday = 
        	TestBusinessHourFactory.createBusinessHourWithDayAndTime(
            		"MONDAY", LocalTime.of(9, 0), LocalTime.of(21, 0)
                	);
		businessHours.add(monday);
		BusinessHourOpenChecker sut = new BusinessHourOpenChecker();
		//when
		boolean isOpen = sut.checkWithBusinessHours(
        		businessHours, LocalDateTime.of(2024, 1, 29, 9, 0, 0)
            		);
		//then
		assertThat(isOpen).isTrue();
	}

 

첫번째 테스트 케이스는 삭제하고, 두번째 테스트 케이스만 살려놓자.

두번째 케이스는 이미 테스트 대상의 동작 단위를 테스트하고 있다.

---

세번째 케이스

AS-IS

	@ParameterizedTest
	@MethodSource("provideDayOfWeekAndBusinessHour")
	@DisplayName("요일에 맞는 영업시간을 확인한다.")
	void find_business_hour(String dayOfWeek, LocalTime startTime, LocalTime endTime) {
		//given
		List<BusinessHour> businessHours = List.of(
			createBusinessHourWithDayAndTime("MONDAY",
				LocalTime.of(9, 0), LocalTime.of(21, 0)),
			createBusinessHourWithDayAndTime("TUESDAY",
				LocalTime.of(10, 0), LocalTime.of(22, 0))
		);
		Cafe sut = createCafeWithBusinessHours(businessHours);
		//when
		BusinessHour businessHour = sut.findBusinessHour(DayOfWeek.valueOf(dayOfWeek));
		//then
		assertAll(
			() -> assertThat(businessHour.getStartTime()).isEqualTo(startTime),
			() -> assertThat(businessHour.getEndTime()).isEqualTo(endTime)
		);
	}

	private static Stream<Arguments> provideDayOfWeekAndBusinessHour() {
		return Stream.of(
			Arguments.of("MONDAY", LocalTime.of(9, 0), LocalTime.of(21, 0)),
			Arguments.of("TUESDAY", LocalTime.of(10, 0), LocalTime.of(22, 0))
		);
	}

 

위 테스트는 MONDAY, TUESDAY와 같이 요일에 맞는 영업시간를 제대로 찾아오는지 확인하는 테스트이다.

 

이 테스트가 문제가 되는것은 코드의 복잡도이다.

복잡도는 좋은 단위 테스트의 요소 중 하나인 회귀 방지와 관련이 있다.

https://ghffu405.tistory.com/53

[단위 테스트] 회귀 방지 (좋은 단위테스트의 요소)

 

이 테스트에서 확인하는 것은 요일에 맞는 영업시간을 찾아오는 것이다.

어떻게 생각하는가?

요일에 맞는 영업시간을 찾아오는 로직이 복잡할까?

 

한번 구현 세부사항을 보자.

public BusinessHour findBusinessHour(DayOfWeek dayOfWeek) {
		return businessHours.stream()
			.filter(businessHour -> businessHour.matchesDayOfWeek(dayOfWeek))
			.findFirst()
			.orElseThrow(() -> new CafegoryException(CAFE_NOT_FOUND_DAY_OF_WEEK));
	}

 

실제 사용하고 있는 코드를 가져왔다.

BusinessHour 객체에게 DayOfWeek(요일) 에 맞는 BusinessHour 객체를 반환해달라고 하는 로직이다.

로직을 보면 복잡한것이 하나도 없다.

 

좋은 단위 테스트 요소 중 하나인 회귀 방지를 위해서 이 테스트가 존재하는게 맞을까?

필자는 처음에는 이 테스트가 필요없는 테스트라고 생각했었지만, 이 테스트는 필요한 테스트다.

코드는 복잡하지 않지만 비즈니스적으로 무조건 검증해야하기 때문이다.

 

이 로직이 구현된 프로젝트에 대해서 간단히 설명하자면, 

카페가 존재하고 카페에 대한 영업시간과 같은 정보가 존재한다.

카페의 영업시간 기준으로 스터디가 존재한다.

 

카페의 영업시간은 비즈니스적으로 중요하고, 영업시간을 기준으로 스터디가 존재한다.

요일에 맞는 영업시간이 정확하지 않다면 엄청난 버그가 발생할 것이다.

 

그렇기 때문에 이 테스트는 코드의 복잡도와 무관하게 비즈니스적으로 굉장히 중요한 역할을 하고 있기 때문에 좋은 테스트라고 볼 수 있다. 

---

네번째 케이스

AS-IS

	@Test
	@DisplayName("잘못된 시간으로 스터디를 생성할 수 없다.")
	void creating_studyOnce_fails_with_invalid_time() {
		Assertions.assertThatThrownBy(
			() -> {
				LocalDateTime startDateTime = 
                	LocalDateTime.now().plusHours(3).minusSeconds(1);
                    
				StudyOnce.builder()
					.startDateTime(startDateTime)
					.endDateTime(startDateTime.plusHours(4))
					.build();
			})
		.isInstanceOf(CafegoryException.class)
		.hasMessage(STUDY_ONCE_WRONG_START_TIME.getErrorMessage());
	}

 

이 테스트는 잘못된 시간으로 스터디를 생성할 수 있는지 확인하는 테스트이다.

스터디 객체를 만들기 위해서는 다음과 같은 비즈니스 규칙이 존재한다.

• 스터디 시작 시간은 스터디를 생성하는 시점으로부터 최소 3시간 이후로 설정할 수 있다.

현재시간 기준에서 2시간 59분 59초에서 스터디 객체가 생성되는지 테스트를 한다.

 

이 테스트는 좋은 단위 테스트 요소 중 하나인 리팩토링 내성의 특성인 도메인 유의성에 부합한다.

최소 3시간 이후로 스터디를 생성할 수 있다고 했으니까 2시간 59분 59초에 테스트하는것은 마땅하다.

 

하지만 이 테스트는 심각한 단점이 있다.

위에서 살펴봤듯이, 좋은 단위 테스트 요소 중 하나인 리팩토링 내성이 좋지 못하다.

 

StudyOnce.builder()
	.startDateTime(startDateTime)
	.endDateTime(startDateTime.plusHours(4))
	.build();

 

StudyOnce 객체를 생성하는 과정에서 비즈니스 규칙에 포함하는 구현 세부 사항인 startDateTime 과 endDateTim 을 드러내고 있다.

 

그러면 이런 의문을 가질 수 있다.

 

리팩토링 내성을 높히기 위해서는 구현 세부 사항을 테스트 하는것이 아니라, 테스트 하는 대상, 즉 sut 의 식별할 수 있는 동작을 테스트하라고 했다.

객체를 생성하는 것은 객체를 생성하는것 그뿐이라 식별할 수 있는 동작으로 바꿀수 없다.

 

그럼 리팩토링 내성을 위해서 비즈니스 규칙을 테스트하지 않고 테스트를 지우라는 건가?

 

아니다.

 

정확히는 위의 객체 생성을 테스트하는 테스트 케이스는 지우고, 상위 계층에서 비즈니스 규칙을 테스트를 하면 된다.

 

다음 코드는 상위 계층인 Service 계층의 메서드를 테스트 한 것이다.

 

TO-BE

	@Test
	@DisplayName("잘못된 시간으로 스터디를 생성할 수 없다.")
	void creating_studyOnce_fails_with_invalid_time() {
  		//given
		LocalDateTime start = LocalDateTime.now().plusHours(3).minusSeconds(1);
		LocalDateTime end = start.plusHours(3);
        	Long leaderId = createMember().getId();
		StudyOnceCreateRequest studyOnceCreateRequest = makeStudyOnceCreateRequest(start, end);
		//then
		assertThatThrownBy(
        	() -> studyOnceService.createStudy(leaderId, studyOnceCreateRequest, LocalDate.now()))
			.isInstanceOf(CafegoryException.class)
			.hasMessage(STUDY_ONCE_WRONG_START_TIME.getErrorMessage());
	}

 

이제는 StudyOnce 객체를 생성하는 과정에서 비즈니스 규칙에 포함하는 구현 세부사항을 드러내지 않고 비즈니스 규칙을 테스트 했다.

---

다섯번째 케이스

AS-IS

	@Test
	@DisplayName("스터디 시작시간 1시간 전(이하)에 스터디 참여신청이 가능하다.")
	void allows_joining_1hour_before_start() {
		//given
		Member leader = createMember();
		Member member = createMember();
		Cafe cafe = createCafe();
		StudyOnce sut = createStudyOnceWithTime(cafe, leader, NOW.plusHours(4), NOW.plusHours(6));
		//when
		sut.tryJoin(member, NOW.plusHours(3));
		//then
		List<Member> members = sut.getStudyMembers().stream()
			.map(StudyMember::getMember)
			.collect(Collectors.toList());
		assertThat(members).contains(member);
	}

 

스터디 시작시간 1시간 전에 스터디 참여신청이 가능하다 라는 비즈니스를 가지고 있는 테스트이다.

비즈니스 규칙을 테스트 하고 있으므로 좋은 단위 테스트의 요소인 회귀 방지는 만족한다.

 

그러면 리팩토링 내성이 문제인가?

맞다.

하지만 위의 테스트는 상태 기반 테스트 스타일이므로, 상태를 검증하는 과정에서 구현 세부 사항이 어느정도 노출이 될 수 밖에 없다.

이러한 문제때문에 상태 기반 테스트는 출력 기반 테스트보다 리팩토링 내성이 낮다.

https://ghffu405.tistory.com/55

[단위 테스트] 단위 테스트의 세 가지 스타일

 

상태 기반 테스트는 구현 세부 사항이 어느정도 노출된다고 했다.

그러면 테스트에서 뭐가 문제인데??

 

사실 문제라기 보다는 구현 세부 사항 노출을 좀 줄이고자 이 테스트 케이스를 다섯번째로 선택했다.

구현 세부 사항 노출을 좀 줄여보자.

 

TO-BE

	@Test
	@DisplayName("스터디 시작시간 1시간 전(이하)에 스터디 참여신청이 가능하다.")
	void allows_joining_1hour_before_start() {
		//given
		Member leader = createMember();
		Member member = createMember();
		Cafe cafe = createCafe();
		StudyOnce sut = createStudyOnceWithTime(cafe, leader, NOW.plusHours(4), NOW.plusHours(6));
		//when
		sut.tryJoin(member, NOW.plusHours(3));
		//then
		assertThat(sut.getStudyMembers().size()).isEqualTo(2);
	}

 

비교

// 변경 전
		List<Member> members = sut.getStudyMembers().stream()
			.map(StudyMember::getMember)
			.collect(Collectors.toList());
		assertThat(members).contains(member);
        
// 변경 후
		assertThat(sut.getStudyMembers().size()).isEqualTo(2);

 

 

변경 전에는 StudyMember 객체에서 getMember 메서드를 통해서 스터디에 참여한 모든 멤버를 List로 매핑하고 있다.

그리고 나서 member가 스터디에 참여한 멤버에 포함하는 지 검증하고 있다.

 

변경 후에는 스터디에 참여한 멤버의 인원 수만 검증하고 있다.

 

물론 변경 전의 테스트가 참여 신청한 멤버가 들어갔는지 확실하게 검증할 수 있다.

하지만 이 테스트는 다른 테스트와 격리되어 있고 서로 영향을 미치지 않는다.

변경 후의 테스트처럼 인원수만 검증해도 충분하다. 

인원수만 검증하면 구현 세부 사항을 변경 전보다 적게 드러내므로, 리팩토링 내성이 좋아진다.

---

여섯번째 케이스

AS-IS

	@Test
	@DisplayName("카공이 시작되기전에는 카공원은 참여자들의 프로필을 조회할 수 없다.")
	void member_cannot_view_profiles_of_participants_before_study_begins() {
		//given
        	ProfileService sut = new ProfileService(의존 객체 주입);
            
		ThumbnailImage thumbnailImage = thumbnailImageSaveHelper.saveThumbnailImage();
		Member leader = memberSaveHelper.saveMemberWithName(thumbnailImage, "카공장");
		Member member = memberSaveHelper.saveMemberWithName(thumbnailImage, "카공원");
		Cafe cafe = cafeSaveHelper.saveCafe();
		StudyOnce studyOnce = 
        		studyOnceSaveHelper.saveStudyOnceWithTime(cafe, leader, 
           			NOW.plusHours(4), NOW.plusHours(5));
		studyOnceService.tryJoin(member.getId(), studyOnce.getId());
        
		//then
		assertThatThrownBy(() -> sut.getProfile(member.getId(), leader.getId(), NOW))
			.isInstanceOf(CafegoryException.class)
			.hasMessage(PROFILE_GET_PERMISSION_DENIED.getErrorMessage());
	}

 

sut인 ProfileService 객체의 sut.getProfile 메서드는 프로필을 조회하는 메서드이다.

getProfile은 다음과 같은 비즈니스를 담고 있다.

• 카공에 참여 신청한 멤버의 프로필을 볼 수 있다.
• 카공이 시작되기전에는 , 카공장만이 참여 신청한 멤버의 프로필을 볼 수 있다.
• 카공이 시작되면, 다른 참여자들도 참여 신청한 멤버의 프로필을 볼 수 있다

카공(스터디)가 시작되어야만 참여 멤버들은 다른 사람들의 프로필을 볼 수 있다.

 

위 테스트는 두번째 비즈니스의 부정 케이스를 검증하고 있다.

 

getProfile의 구현 세부사항을 보자.

public ProfileGetResponse getProfile(
	Long requestMemberId, Long targetMemberId, LocalDateTime baseDateTime) {
    
		if (isOwnerOfProfile(requestMemberId, targetMemberId)) {
			return makeProfileGetResponse(targetMemberId);
		}
		if (isAllowedCauseStudyLeader(requestMemberId, targetMemberId)) {
			return makeProfileGetResponse(targetMemberId);
		}
		if (isAllowedCauseSameStudyOnceMember(requestMemberId, targetMemberId, baseDateTime)) {
			return makeProfileGetResponse(targetMemberId);
		}
		throw new CafegoryException(PROFILE_GET_PERMISSION_DENIED);
	}

 

첫번째 분기 : 자신의 프로필을 조회한다.

두번째 분기 : 카공이 시작되기전에는 , 카공장만이 참여 신청한 멤버의 프로필을 볼 수 있다.

세번째 분기 : 카공이 시작되면, 다른 참여자들도 참여 신청한 멤버의 프로필을 볼 수 있다

 

각각의 분기마다 비즈니스 규칙이 맞는지 확인하고 응답값을 리턴해준다.

 

이 세가지의 비즈니스 규칙에 부합하지 않는다면, 예외를 터트린다.

 

다시 테스트의 검증부분을 확인해보자.

//then
assertThatThrownBy(() -> sut.getProfile(member.getId(), leader.getId(), NOW))
	.isInstanceOf(CafegoryException.class)
	.hasMessage(PROFILE_GET_PERMISSION_DENIED.getErrorMessage());

 

어느부분이 문제일까?

이제는 위에서 봐온 여러 케이스를 잘 이해했다면 쉽게 찾을 수 있을것이다.

 

바로 구현 세부사항을 드러냈다는 점이다.

PROFILE_GET_PERMISSION_DENIED.getErrorMessage()

 

이 예외 메시지를 통해서 구현 세부사항을 드러내고 있다.

위에서 봐왔듯이 구현 세부사항이 테스트에 드러나면 리팩토링 내성은 낮아진다.

비즈니스 규칙은 같은데, 예외가 변경된다면 예외를 드러낸 테스트 케이스들은 모두 다 컴파일 오류가 날것이다.

 

그럼 저 예외를 드러낸 부분을 지우는게 좋을까?

 

필자는 이 예외부분을 그대로 냅두는 것이 더 낫다고 생각했다.

리팩토링 내성은 낮아지지만, 이 예외를 통해서 sut의 메서드가 원하는 분기를 테스트하고 있는지 확실히 알 수 있다.

 

sut.getProfile의 로직을 좀 변경해보겠다.

public ProfileGetResponse getProfile(
	Long requestMemberId, Long targetMemberId, LocalDateTime baseDateTime) {
    
		if (!isOwnerOfProfile(requestMemberId, targetMemberId)) {
			throw new CafegoryException(자신의 프로필만 조회 가능합니다)
		}
		if (!isAllowedCauseStudyLeader(requestMemberId, targetMemberId)) {
			throw new CafegoryException(카공장이 아니면 참여자들의 프로필을 조회할 수 없습니다)
		}
		if (!isAllowedCauseSameStudyOnceMember(requestMemberId, targetMemberId, baseDateTime)) {
			throw new CafegoryException(카공이 시작하기전에는 참여자들의 프로필을 조회할 수 없습니다)
		}
		return makeProfileGetResponse(targetMemberId);
	}

 

수정 전 코드에는 예외를 하나만 터뜨렸다.

throw new CafegoryException(PROFILE_GET_PERMISSION_DENIED);

 

수정 후에는 케이스에 따라서 다른 예외를 터뜨렸다.

throw new CafegoryException(자신의 프로필만 조회 가능합니다)
throw new CafegoryException(카공장이 아니면 참여자들의 프로필을 조회할 수 없습니다)
throw new CafegoryException(카공이 시작하기전에는 참여자들의 프로필을 조회할 수 없습니다)

 

케이스에 따라서 예외를 터뜨릴 경우, 테스트 코드에서 세부적으로 예외를 드러낼 수 있다.

//then
assertThatThrownBy(() -> sut.getProfile(member.getId(), leader.getId(), NOW))
	.isInstanceOf(CafegoryException.class)
	.hasMessage(여러 예외를 넣을 수 있다);

 

이렇게 여러 예외를 넣어서 검증하게 된다면, 다음과 같은 장점이 있다.

• 특정 비즈니스 규칙을 검증하고 있는지 알 수 있다.
• 메서드 내부에서 특정 비즈니스 규칙의 로직을 타고 있음을 확인 할 수 있다. 즉, 원하는 로직을 검증하고 있음을 증명한다.

이러한 장점 때문에 리팩토링 내성이 낮아지더라도, 테스트는 가치있는 테스트가 됐다.

 

---

일곱번째 케이스

AS-IS

	@Test
	@DisplayName("리더만이 스터디 세부사항을 수정할 수 있다.")
	void only_leader_updates_study_details() {
		//given
		테스트 픽스쳐 생성
        	...
		StudyOnceUpdateRequest request = 
        		new StudyOnceUpdateRequest(cafe2.getId(), "변경된카공이름", start.plusHours(5),
			start.plusHours(6), 5, false, "오픈채팅방 링크");
		//when
		sut.updateStudyOnce(leader.getId(), studyOnce.getId(), request, LocalDateTime.now());
		StudyOnce result = studyOnceRepository.findById(studyOnce.getId()).get();
		//then
		assertAll(
			() -> assertThat(result.getName()).isEqualTo(request.getName()),
			() -> assertThat(result.getCafe().getId()).isEqualTo(request.getCafeId()),
			() -> assertThat(result.getStartDateTime()).isEqualTo(request.getStartDateTime()),
			() -> assertThat(result.getEndDateTime()).isEqualTo(request.getEndDateTime()),
			() -> assertThat(result.getMaxMemberCount()).isEqualTo(request.getMaxMemberCount()),
			() -> assertThat(result.isAbleToTalk()).isEqualTo(request.isCanTalk()),
			() -> assertThat(result.getOpenChatUrl()).isEqualTo(request.getOpenChatUrl())
		);
	}

 

이 테스트는 무엇이 문제일까?

스터디의 세부사항을 수정하고 수정 내용이 반영됐는지 검증하고 있다.

// 여기서 sut는 StudyOnceService 객체
sut.updateStudyOnce(leader.getId(), studyOnce.getId(), request, LocalDateTime.now());

 

sut의 동작 단위를 실행하고 검증단에서도 수정 내용에 대해서 검증하기 때문에 별 문제 없는 테스트처럼 보인다.

sut의 updateStudyOnce의 구현 세부사항을 보자.

public void updateStudyOnce(long requestedMemberId, long studyOnceId, StudyOnceUpdateRequest request,
		LocalDateTime now) {
		StudyOnce studyOnce = findStudyOnceById(studyOnceId);
		if (!isStudyOnceLeader(requestedMemberId, studyOnceId)) {
			throw new CafegoryException(STUDY_ONCE_LEADER_PERMISSION_DENIED);
		}
		if (request.getCafeId() != null) {
			studyOnce.changeCafe(findCafeById(request.getCafeId()));
		}
		if (request.getName() != null) {
			studyOnce.changeName(request.getName());
		}
		if (request.getStartDateTime() != null && request.getEndDateTime() != null) {
			Cafe cafe = studyOnce.getCafe();
			validateBetweenBusinessHour(request.getStartDateTime().toLocalTime(),
				request.getEndDateTime().toLocalTime(), cafe.findBusinessHour(now.getDayOfWeek()));
			studyOnce.changeStudyOnceTime(request.getStartDateTime(), request.getEndDateTime());
		}
		if (request.getOpenChatUrl() != null) {
			studyOnce.changeOpenChatUrl(request.getOpenChatUrl());
		}
		studyOnce.changeMaxMemberCount(request.getMaxMemberCount());
		studyOnce.changeCanTalk(request.isCanTalk());
	}

 

복잡해보이지만 간단히 설명하자면, null이 아닌 값들만 수정하는 로직이다.

 

구현 세부사항을 봤으니 이제 어떤점이 문제인지 보이는가?

테스트 코드에 구현 세부사항이 드러난 것도 아니다.

 

이번엔 StudyOnce의 여러 change 메서드들을 보자.

public void changeName(String name) {
		validateEmptyOrWhiteSpace(name, STUDY_ONCE_NAME_EMPTY_OR_WHITESPACE);
		this.name = name;
	}

public void changeStudyOnceTime(LocalDateTime startDateTime, LocalDateTime endDateTime) {
	validateStartDateTime(startDateTime);
	validateStudyOnceTime(startDateTime, endDateTime);
	this.startDateTime = startDateTime;
	this.endDateTime = endDateTime;
}

public void changeMaxMemberCount(int maxMemberCount) {
	validateMaxMemberCount(maxMemberCount);
	validateNowMemberCountOverMaxLimit(this.nowMemberCount, maxMemberCount);
	this.maxMemberCount = maxMemberCount;
}

 

StudyOnceService에서 StudyOnce에게 스터디 세부사항에 대해서 변경을 요청하고, 그 요청에 대한 메서드이다.

StudyOnce 메서드의 구현 세부사항까지 봤다.

 

이제는 테스트에서 어떤것이 문제인지 보이는가?

아직도 보이지 않는다면 이번엔 StudyOnce에 대한 테스트를 보자.

	@ParameterizedTest
	@ValueSource(strings = {"", " "})
	@DisplayName("스터디 이름 변경, 빈값, 공백문자 검증")
	void name_is_empty_or_whitespace(String value) {
		//given
		Member leader = createMember();
		Cafe cafe = createCafe();
		StudyOnce sut = createStudyOnce(cafe, leader);
        	//then
		assertThatThrownBy(() -> sut.changeName(value))
			.isInstanceOf(CafegoryException.class)
			.hasMessage(STUDY_ONCE_NAME_EMPTY_OR_WHITESPACE.getErrorMessage());
	}

 

이제는 보이는가?

위 테스트는 스터디의 이름을 바꾸는 테스트이다.

 

아직도 보이지 않는다면, StudyOnceService 테스트와 비교해보자.

	@Test
	@DisplayName("리더만이 스터디 세부사항을 수정할 수 있다.")
	void only_leader_updates_study_details() {
		//given
		테스트 픽스쳐 생성
        	...
		StudyOnceUpdateRequest request = 
        		new StudyOnceUpdateRequest(cafe2.getId(), "변경된카공이름", start.plusHours(5),
			start.plusHours(6), 5, false, "오픈채팅방 링크");
		//when
		sut.updateStudyOnce(leader.getId(), studyOnce.getId(), request, LocalDateTime.now());
		StudyOnce result = studyOnceRepository.findById(studyOnce.getId()).get();
		//then
		assertAll(
			() -> assertThat(result.getName()).isEqualTo(request.getName()),
			() -> assertThat(result.getCafe().getId()).isEqualTo(request.getCafeId()),
			() -> assertThat(result.getStartDateTime()).isEqualTo(request.getStartDateTime()),
			() -> assertThat(result.getEndDateTime()).isEqualTo(request.getEndDateTime()),
			() -> assertThat(result.getMaxMemberCount()).isEqualTo(request.getMaxMemberCount()),
			() -> assertThat(result.isAbleToTalk()).isEqualTo(request.isCanTalk()),
			() -> assertThat(result.getOpenChatUrl()).isEqualTo(request.getOpenChatUrl())
		);
	}

 

위의 StudyOnceService 테스트의 문제점은

sut.updateStudyOnce 를 실행하고 난 뒤의 결과를 모두 검증하고 있다는 점이다.

 

수정된 결과의 검증은 이미 StudyOnce의 테스트에서 검증이 됐다.

그런데 또 다시 서비스 레이어에서 검증을 하고 있는 것이다.

 

이미 하위 레이어에서 검증한 내용을 상위 레이어에서 또 검증하고 있다.

 

정리를 해보자면

• 하위 레이어, 즉 StudyOnce에서는 변경사항에 내용을 검증하는 것이 맞다.
• 그 위의 상위 레이어 StudyOnceService에서는 변경사항까지 검증할 필요는 없다

이는 결국에는 상위 레이어에서 구현 세부사항을 드러내므로써 리팩토링 내성을 낮춘것과 같다.

 

그럼 이제 바뀐 테스트를 보자.

TO-BE

	@Test
	@DisplayName("리더만이 스터디 세부사항을 수정할 수 있다.")
	void only_leader_updates_study_details() {
		//given
		테스트 픽스쳐 생성
        	...
		StudyOnceUpdateRequest request = 
        		new StudyOnceUpdateRequest(cafe2.getId(), "변경된카공이름", start.plusHours(5),
			start.plusHours(6), 5, false, "오픈채팅방 링크");
		//when
		assertDoesNotThrow(
        		() -> sut.updateStudyOnce(
            			leader.getId(), studyOnce.getId(), request, LocalDateTime.now()
                	));
	}

 

시나리오에 맞게 예외가 터지지가 않는다면 성공하는 테스트로 바뀌었다.