refactor: 대학(추천, 좋아요) 통합 테스트 데이터 Fixture 메서드로 변경 #313
Conversation
Walkthrough
Suggested reviewers
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/test/java/com/example/solidconnection/university/service/GeneralUniversityRecommendServiceTest.java (1)
31-44: @beforeeach 설정이 체계적으로 구성되었습니다!
- 변경사항:
- 테스트 실행 전 필요한 대학 정보 데이터를 미리 생성
- 여러 종류의 대학 정보를 Fixture 메서드로 생성
- generalUniversityRecommendService.init() 호출을 중앙화
이 접근 방식은 PR 목표에 맞게 잘 구현되었습니다. 6개 이상의 대학 데이터를 생성하여 "Out of Range" 오류를 방지하는 요구사항도 충족했습니다.
추가 제안:
- 주석으로 6개 이상의 대학 데이터가 필요한 이유를 설명하면 더 명확할 것 같습니다
- 향후 필요한 대학 수가 변경될 경우를 대비해 상수로 분리하는 것도 고려해 볼 수 있습니다
@BeforeEach void setUp() { + // generalUniversityRecommendService.init()는 최소 6개 이상의 대학 데이터가 필요함 + // "Out of Range" 오류 방지를 위해 충분한 수의 대학 데이터 생성 universityInfoForApplyFixture.괌대학_A_지원_정보(); universityInfoForApplyFixture.괌대학_B_지원_정보(); universityInfoForApplyFixture.네바다주립대학_라스베이거스_지원_정보(); universityInfoForApplyFixture.메모리얼대학_세인트존스_A_지원_정보(); universityInfoForApplyFixture.서던덴마크대학교_지원_정보(); universityInfoForApplyFixture.코펜하겐IT대학_지원_정보(); universityInfoForApplyFixture.그라츠대학_지원_정보(); universityInfoForApplyFixture.그라츠공과대학_지원_정보(); universityInfoForApplyFixture.린츠_카톨릭대학_지원_정보(); universityInfoForApplyFixture.메이지대학_지원_정보(); generalUniversityRecommendService.init(); }src/test/java/com/example/solidconnection/university/service/UniversityRecommendServiceTest.java (1)
172-173: TODO 주석 추가사용자 생성 메서드를 Fixture로 대체하라는 TODO 주석 추가는 향후 개선 방향을 명확히 제시합니다. 현재는 createSiteUser() 메서드를 사용하지만, 일관성을 위해 이것도 Fixture로 변경하는 것이 좋습니다.
다음 단계로 SiteUserFixture를 구현하여 사용자 생성 코드도 Fixture 패턴으로 통일하시면 좋겠습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/test/java/com/example/solidconnection/university/service/GeneralUniversityRecommendServiceTest.java(2 hunks)src/test/java/com/example/solidconnection/university/service/UniversityLikeServiceTest.java(4 hunks)src/test/java/com/example/solidconnection/university/service/UniversityRecommendServiceTest.java(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (20)
src/test/java/com/example/solidconnection/university/service/UniversityLikeServiceTest.java (6)
7-7: TestContainerSpringBootTest로 상속 관계 변경BaseIntegrationTest에서 TestContainerSpringBootTest로 상속 관계를 변경했습니다. 이는 PR 설명에 언급된 대로 Fixture 기반 테스트 데이터 생성 방식으로 전환하는 전체 리팩토링의 일부입니다.
14-15: 필요한 임포트 추가
- UniversityInfoForApplyFixture를 임포트하여 대학 지원 정보 테스트 데이터를 생성할 수 있게 되었습니다.
- BeforeEach 어노테이션을 임포트하여 각 테스트 실행 전 공통 설정을 수행할 수 있게 되었습니다.
30-32: 클래스 상속 구조 변경BaseIntegrationTest 상속에서 TestContainerSpringBootTest 어노테이션 사용으로 변경했습니다. 이는 테스트 설정을 더 명시적으로 만들고, Fixture 패턴 도입에 맞게 리팩토링하는 과정의 일부입니다.
43-48: Fixture 및 테스트 데이터 필드 추가
- UniversityInfoForApplyFixture를 주입받아 재사용 가능한 테스트 데이터를 생성할 수 있게 되었습니다.
- 모든 테스트에서 공통으로 사용할 testUser와 괌대학_A_지원_정보 필드를 선언했습니다.
이러한 변경은 각 테스트 메서드에서 중복해서 데이터를 생성하는 대신, 일관된 테스트 데이터를 사용할 수 있게 해줍니다.
49-54: @beforeeach 메서드 추가각 테스트 실행 전에 testUser와 괌대학_A_지원_정보를 생성하는 setUp 메서드를 추가했습니다. 이 변경으로:
- 각 테스트 메서드에서 중복 코드가 제거되었습니다.
- 테스트 데이터 생성 로직이 중앙화되었습니다.
- 테스트 간 일관성이 향상되었습니다.
156-156: TO-DO 주석 추가현재 직접 구현된 createSiteUser() 메서드를 추후 Fixture로 대체해야 한다는 TO-DO 주석을 추가했습니다. 이는 리팩토링이 아직 완료되지 않았음을 나타내며, 일관된 Fixture 사용 패턴을 완성하기 위한 향후 작업을 명확히 합니다.
현재 코드에서 createSiteUser() 메서드를 직접 구현하고 있는데, 이를 나중에 SiteUserFixture와 같은 클래스로 분리하는 것이 좋겠습니다. 아래와 같은 후속 작업이 필요합니다:
- SiteUserFixture 클래스 생성하기
- 표준 테스트 사용자 생성 메서드 구현하기
- 현재 메서드를 해당 Fixture로 대체하기
src/test/java/com/example/solidconnection/university/service/GeneralUniversityRecommendServiceTest.java (4)
5-6: 필요한 import 추가가 잘 되었습니다!
- 변경사항:
- Fixture 클래스 import 추가
- BeforeEach 어노테이션 import 추가
위 import들은 새로운 테스트 접근 방식에 필요한 요소들로, 적절하게 추가되었습니다.
19-20: 클래스 구조 개선이 잘 이루어졌습니다!
- 변경사항:
- DisplayName 어노테이션 추가로 테스트 목적이 명확해졌습니다
- BaseIntegrationTest 상속 제거로 불필요한 의존성이 제거되었습니다
이러한 변경은 테스트 코드의 가독성과 목적성을 높이는 좋은 리팩토링입니다.
25-27: Fixture 주입 방식으로의 전환이 훌륭합니다!
- 변경사항:
- UniversityInfoForApplyFixture를 주입받아 테스트 데이터 생성 방식 개선
테스트 데이터 생성을 중앙화된 Fixture로 관리하는 것은:
- 테스트 데이터의 일관성 유지에 도움이 됩니다
- 코드 중복을 줄여줍니다
- 테스트 가독성을 높여줍니다
46-58: 테스트 메서드의 가독성이 향상되었습니다!
- 변경사항:
- 테스트 메서드 내에서 더 이상 init() 메서드를 호출하지 않아 간결해짐
- 테스트 로직은 동일하게 유지하여 안정성 확보
테스트 코드의 설정 부분과 검증 부분이 명확히 분리되어 Single Responsibility Principle을 잘 따르고 있습니다. 이는 테스트 코드의 유지보수성과 가독성을 높여줍니다.
src/test/java/com/example/solidconnection/university/service/UniversityRecommendServiceTest.java (10)
3-17: 테스트 코드 가독성을 위한 import 개선
추가된 Fixture 관련 import
- RegionFixture, CountryFixture, UniversityInfoForApplyFixture 추가
- 테스트 데이터를 중앙화하여 관리할 수 있게 됨
테스트 기반 클래스 변경
- BaseIntegrationTest에서 TestContainerSpringBootTest로 변경됨
- 컨테이너 기반 테스트로 격리성 향상
적절한 fixture 사용으로 테스트 코드의 가독성과 유지보수성이 향상되었습니다.
28-30: 테스트 클래스 선언부 변경
테스트 클래스 접근 제어자 변경
- public에서 default(package-private)로 변경됨
- 불필요한 노출 감소
테스트 프레임워크 적용 방식 변경
- 상속 대신 애노테이션 기반 구성으로 변경
- 좋은 변경점: 상속보다 구성(composition)이 더 유연한 디자인 패턴
접근 제어자와 테스트 구성 방식의 변경으로 좀 더 현대적인 테스트 코드 스타일을 적용했습니다.
47-54: Fixture 의존성 주입 추가
- 테스트 데이터 생성 방식 개선
- RegionFixture, CountryFixture, UniversityInfoForApplyFixture 주입
- 하드코딩된 데이터 생성 로직 제거
의존성 주입을 통한 Fixture 활용은 테스트 데이터 준비 과정을 명확하게 하고, 재사용성을 높이는 좋은 변경입니다.
56-67: 테스트 데이터 필드 추가
공유 필드 생성
- testUser 필드 추가로 테스트 간 사용자 객체 공유
- 모든 테스트 케이스에서 일관된 사용자 사용 가능
대학 지원 정보 필드 다수 추가
- 한글 명명 규칙으로 테스트 의도 명확화
- 다양한 지역의 대학 정보 보유
테스트 데이터를 필드로 관리하는 방식은 데이터 재사용성을 높이고 테스트 간 일관성을 유지하는 데 도움이 됩니다.
68-82: setUp 메서드 개선
사용자 생성 및 초기화
- 모든 테스트에서 공통으로 사용할 testUser 생성
- 테스트 간 사용자 생성 중복 제거
대학 지원 정보 초기화
- Fixture를 활용한 일관된 대학 정보 초기화
- 10개의 대학 정보 생성으로 추천 시스템 테스트 데이터 준비
추천 서비스 초기화
- generalUniversityRecommendService.init() 호출 중앙화
- PR 설명대로 "Out of Range" 오류 방지를 위해 6개 이상의 데이터 준비
Fixture를 활용한 데이터 준비와 서비스 초기화 방식이 개선되었습니다. 각 테스트 전에 필요한 상태가 일관되게 준비됩니다.
84-101: 관심 지역 기반 추천 테스트 개선
Fixture 활용
- regionFixture를 사용하여 영미권 지역 데이터 생성
- 테스트 가독성 향상 및 하드코딩 제거
공유 사용자 사용
- 각 테스트마다 사용자를 생성하지 않고 공유 testUser 사용
- 테스트 셋업 간소화
테스트 데이터 생성 방식이 개선되어 테스트 의도가 더 명확하게 드러납니다.
103-118: 관심 국가 기반 추천 테스트 개선
Fixture 활용
- countryFixture를 사용하여 덴마크 국가 데이터 생성
- 테스트 가독성 향상 및 하드코딩 제거
공유 테스트 객체 사용
- 미리 생성된 university 객체와 testUser 활용
- 테스트 코드의 일관성 향상
국가 기반 추천 테스트에서도 Fixture를 활용하여 테스트 데이터 관리가 개선되었습니다.
120-140: 복합 관심사 추천 테스트 개선
Fixture 활용 확대
- regionFixture와 countryFixture 동시 사용
- 복합 관심사 테스트 케이스의 데이터 준비 간소화
명확한 테스트 결과 검증
- containsExactlyInAnyOrder를 사용한 정확한 결과 세트 검증
- 예상 결과와 실제 결과 간의 명확한 비교
지역과 국가를 모두 고려한 복합 추천 테스트에서도 Fixture를 일관되게 활용하고 있습니다.
142-155: 관심사 미설정 사용자 테스트 개선
사용자 생성 로직 제거
- 이전 코드에서 메서드 내 사용자 생성을 제거
- setUp에서 생성된 공유 testUser 사용
일반 추천 결과 검증 유지
- 일반 추천 서비스의 결과와 일치하는지 검증 로직 유지
- 기능 동작의 정확성 보장
관심사가 없는 사용자에 대한 테스트도 공통 사용자 객체를 활용하도록 개선되었습니다.
1-183: 전체적인 리팩토링 평가
Fixture 패턴 도입
- 테스트 데이터 생성 방식의 일관성 확보
- 테스트 코드의 가독성 및 유지보수성 향상
테스트 설정 중앙화
- setUp 메서드에서 모든 공통 초기화 수행
- 테스트 간 중복 코드 제거
추천 시스템 초기화 방식 개선
- PR 설명에 언급된 "Out of Range" 오류 방지
- 6개 이상의 데이터를 미리 준비하여 안정적인 테스트 환경 구성
질문에 대한 답변: 추천 테스트를 위해 모든 대학을 setUp에서 초기화하는 접근 방식은 적절합니다. 다만, 테스트 관심사에 따라 필요한 최소한의 데이터만 생성하는 방식도 고려해볼 수 있습니다. 현재 방식은 일반적인 사용 시나리오를 테스트하기에 충분합니다.
전체적으로 테스트 코드의 품질이 향상된 훌륭한 리팩토링입니다.
|
generalUniversityRecommendService.init() 관련 문제는 generalUniversityRecommendService 자체를 손봐야 할 것 같은데 어떻게 생각하시나요? |
|
저도 안전하게 바꿔놓는 게 좋을 거 같네요. RECOMMEND_UNIVERSITY_NUM보다 적게 들어있으면 그냥 있는 만큼만 넣어놓은다거나 그러는 게 좋겠네요! 혹시 위백님은 테스트 데이터 세팅 방식이 바뀐 게 어떠신 거 같나요? |
저는 좋아 보입니다. 기존에 상속받는건 너무 무겁고 E2E 느낌이 났었던 것 같습니다. 이제 필요한 모듈만 분리해서 불러올 수 있게 되었네요! |
| @BeforeEach | ||
| void setUp() { | ||
| testUser = createSiteUser(); | ||
| 괌대학_A_지원_정보 = universityInfoForApplyFixture.괌대학_A_지원_정보(); | ||
| } |
| .hasMessage(UNIVERSITY_INFO_FOR_APPLY_NOT_FOUND.getMessage()); | ||
| } | ||
|
|
||
| // todo : 추후 Fixture로 대체 필요 |
There was a problem hiding this comment.
todo로 앞으로의 계획 표시 좋습니다! 🤩
| @BeforeEach | ||
| void setUp() { | ||
| universityInfoForApplyFixture.괌대학_A_지원_정보(); | ||
| universityInfoForApplyFixture.괌대학_B_지원_정보(); | ||
| universityInfoForApplyFixture.네바다주립대학_라스베이거스_지원_정보(); | ||
| universityInfoForApplyFixture.메모리얼대학_세인트존스_A_지원_정보(); | ||
| universityInfoForApplyFixture.서던덴마크대학교_지원_정보(); | ||
| universityInfoForApplyFixture.코펜하겐IT대학_지원_정보(); | ||
| universityInfoForApplyFixture.그라츠대학_지원_정보(); | ||
| universityInfoForApplyFixture.그라츠공과대학_지원_정보(); | ||
| universityInfoForApplyFixture.린츠_카톨릭대학_지원_정보(); | ||
| universityInfoForApplyFixture.메이지대학_지원_정보(); | ||
| generalUniversityRecommendService.init(); | ||
| } |
There was a problem hiding this comment.
generalUniversityRecommendService.init(); 이거 호출 전에 테스트 데이터가 6개 이상 없으면 바로 Out of Range 터지더라구요
혹시 다시 확인해주실 수 있나요? 😲
제가 break point 찍어서 확인해봤을 때는 generalUniversityRecommendService.init() 호출 전에
테스트 데이터가 0개이면,
List<UniversityInfoForApply> universities =
generalUniversityRecommendService.getRecommendUniversities();에서 universities는 비어있는 리스트가 됩니다.
out of range 예외는 발생하지 않고요!
그리고 데이터베이스에 대학 지원 정보가 6개보다 적은 "2개"만 있더라도,
out of range 예외는 발생하지 않고, "크기가 2인 리스트"가 반환됩니다.
RECOMMEND_UNIVERSITY_NUM보다 적게 들어있으면 그냥 있는 만큼만 넣어놓은다거나 그러는 게 좋겠네요!
만약 수정한다면 별도 pr에서 하겠습니다!
말씀하신 부분은 이미 구현되어있습니다~
제가 테스트를 모호하게 작성한게 있어 이런 오해가 발생했으니,
“목표 추천 대학보다 적은 수의 대학이 들어있다면 그만큼을 반환한다” 는 것을 검증하는 테스트 코드를 추가하여 여러가지 케이스를 보여주도록 할 필요는 있어보입니다.
There was a problem hiding this comment.
아 제가 조금 명확하게 말씀을 안드렸네요...
UniversityRecommendService에서 getGeneralRecommendsExcludingSelected 함수가 있는데
private List<UniversityInfoForApply> getGeneralRecommendsExcludingSelected(List<UniversityInfoForApply> alreadyPicked) {
List<UniversityInfoForApply> generalRecommend = new ArrayList<>(generalUniversityRecommendService.getRecommendUniversities());
generalRecommend.removeAll(alreadyPicked);
Collections.shuffle(generalRecommend);
return generalRecommend.subList(0, RECOMMEND_UNIVERSITY_NUM - alreadyPicked.size());
}
여기서 문제가 발생하는 거 같습니다
예시로 alreadyPicked.size()이라는 맞춤 추천 대학 개수가 2개로 가지고 있으면 부족한 개수를 이번 term에 열리는 학교들을 불러오고 있습니다. return으로 결국 subList(0, 4)를 호출하게 되는데 이때 말씀해주신 universities 에 3개의 대학만 들어가있다면 IndexOutOfBoundsException를 발생시킵니다!
There was a problem hiding this comment.
아하 "General" UniversityRecommendService의 테스트 코드가 아니라
UniversityRecommendService의 테스트코드 말씀이셨군요!
저는 뭔가가 잘못된건가.. 했습니다😅
...test/java/com/example/solidconnection/university/service/UniversityRecommendServiceTest.java
Show resolved
Hide resolved
3 participants
관련 이슈
작업 내용
기존 BaseIntegrationTest로 생성하던 테스트 데이터를 Fixture 메서드로 생성하도록 변경하였습니다.
특이 사항
리뷰 요구사항 (선택)
추천 관련 통합테스트는 모든 대학들을 세팅해주는 게 좋은 거 같은데 어떻게 생각하시나요?
generalUniversityRecommendService.init(); 이거 호출 전에 테스트 데이터가 6개 이상 없으면 바로 Out of Range 터지더라구요 그래서 BeforeEach로 지금 한 번에 생성해주고 있긴한데 더 좋은 방식이 있을까요? 🥲