-
Notifications
You must be signed in to change notification settings - Fork 8
refactor: 성적 관련 통합 테스트 데이터 fixture 메서드로 변경 #325
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: 성적 관련 통합 테스트 데이터 fixture 메서드로 변경 #325
Conversation
Walkthrough
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 30th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (7)
src/test/java/com/example/solidconnection/score/fixture/GpaScoreFixtureBuilder.java (1)
40-45: 중복된 siteUser 설정이 있습니다.GpaScore 생성자에서 이미 siteUser를 설정하고 있는데, 42번 라인에서 다시 setSiteUser 메소드를 호출하고 있습니다. 이는 중복 코드입니다.
아래와 같이 수정해보세요:
public GpaScore create() { GpaScore gpaScore = new GpaScore(gpa, siteUser); - gpaScore.setSiteUser(siteUser); gpaScore.setVerifyStatus(verifyStatus); return gpaScoreRepository.save(gpaScore); }src/test/java/com/example/solidconnection/score/fixture/LanguageTestScoreFixture.java (3)
13-15: 테스트 컴포넌트 주석과 Javadoc 추가 필요테스트 픽스처 클래스의 용도와 주요 기능을 설명하는 Javadoc을 다음과 같이 추가하는 것이 좋습니다:
클래스 용도 설명
- 테스트에 필요한 LanguageTestScore 엔티티 생성 역할
- 재사용 가능한 테스트 데이터 제공 방법
메소드 설명
- 각 메소드의 역할과 파라미터 설명
- 반환값의 특성과 사용 방법
/** * 어학 점수 테스트 데이터를 생성하는 픽스처 클래스. * 테스트에서 일관된 어학 점수 데이터를 쉽게 생성할 수 있도록 도와줍니다. */ @TestComponent @RequiredArgsConstructor public class LanguageTestScoreFixture {
19-25: 메소드 어학_점수의 명확한 Javadoc 추가 필요어학_점수 메소드에 대한 설명이 없어 사용 방법을 이해하기 어렵습니다. 다음과 같이 메소드에 대한 Javadoc을 추가해주세요:
/** * 기본값(TOEIC, 점수 "500", 보고서 경로 "/language-report.pdf")으로 설정된 * 어학 점수 엔티티를 생성하고 저장합니다. * * @param verifyStatus 생성할 어학 점수의 검증 상태 * @param siteUser 어학 점수와 연결할 사용자 * @return 저장된 LanguageTestScore 엔티티 */ public LanguageTestScore 어학_점수(VerifyStatus verifyStatus, SiteUser siteUser) {
20-24: 어학 점수 생성 로직 파라미터화 고려현재 코드는 TOEIC 점수 "500"만 생성할 수 있어 테스트 다양성이 제한됩니다. 더 유연한 테스트를 위해 다음과 같이 개선을 고려해보세요:
- 언어 시험 타입, 점수, 파일 경로를 파라미터로 받는 오버로드 메소드 추가
- 기본 메소드는 현재처럼 표준 값 사용
- 오버로드 메소드는 사용자 정의 값 허용
public LanguageTestScore 어학_점수(VerifyStatus verifyStatus, SiteUser siteUser, LanguageTestType testType, String score, String reportPath) { return languageTestScoreFixtureBuilder.languageTestScore() .languageTest(new LanguageTest(testType, score, reportPath)) .verifyStatus(verifyStatus) .siteUser(siteUser) .create(); }src/test/java/com/example/solidconnection/score/fixture/LanguageTestScoreFixtureBuilder.java (3)
11-13: 빌더 클래스 문서화 필요빌더 클래스의 목적과 사용법을 설명하는 Javadoc을 추가하는 것이 좋습니다:
클래스 용도 설명
- 테스트용 LanguageTestScore 엔티티 생성 빌더
- 유연한 테스트 데이터 구성 방법
사용 예시 추가
- 일반적인 사용 패턴 제시
/** * 테스트에서 LanguageTestScore 엔티티를 쉽게 생성하기 위한 빌더 클래스. * 유연한 테스트 데이터 구성과 일관된 저장 프로세스를 제공합니다. * * 사용 예시: * <pre> * LanguageTestScore score = languageTestScoreFixtureBuilder.languageTestScore() * .languageTest(new LanguageTest(TOEIC, "500", "/report.pdf")) * .verifyStatus(VerifyStatus.APPROVED) * .siteUser(user) * .create(); * </pre> */ @TestComponent @RequiredArgsConstructor public class LanguageTestScoreFixtureBuilder {
21-23: languageTestScore 메소드 개선현재 메소드는 새 인스턴스를 생성하지만 목적이 명확하지 않습니다:
- 개선 사항
- 메소드 이름을
create()또는builder()로 변경- 내부 필드 초기화 로직 추가
- 메소드 목적을 설명하는 Javadoc 추가
/** * 새로운 LanguageTestScore 빌더 인스턴스를 생성합니다. * 이전 빌드 상태를 초기화하고 새로운 설정을 시작합니다. * * @return 새로운 LanguageTestScoreFixtureBuilder 인스턴스 */ public LanguageTestScoreFixtureBuilder builder() { return new LanguageTestScoreFixtureBuilder(languageTestScoreRepository); }
25-38: 빌더 메소드 문서화 필요각 빌더 메소드에 대한 Javadoc이 없어 사용법 이해가 어렵습니다:
- 필요한 문서화
- 각 메소드의 역할과 파라미터 설명
- 메소드 체이닝 방식 명시
각 메소드에 간단한 Javadoc을 추가해주세요. 예:
/** * 어학 시험 정보를 설정합니다. * * @param languageTest 설정할 어학 시험 정보 * @return 메소드 체이닝을 위한 빌더 인스턴스 */ public LanguageTestScoreFixtureBuilder languageTest(LanguageTest languageTest) { this.languageTest = languageTest; return this; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (7)
src/test/java/com/example/solidconnection/admin/service/AdminGpaScoreServiceTest.java(6 hunks)src/test/java/com/example/solidconnection/admin/service/AdminLanguageTestScoreServiceTest.java(6 hunks)src/test/java/com/example/solidconnection/score/fixture/GpaScoreFixture.java(1 hunks)src/test/java/com/example/solidconnection/score/fixture/GpaScoreFixtureBuilder.java(1 hunks)src/test/java/com/example/solidconnection/score/fixture/LanguageTestScoreFixture.java(1 hunks)src/test/java/com/example/solidconnection/score/fixture/LanguageTestScoreFixtureBuilder.java(1 hunks)src/test/java/com/example/solidconnection/score/service/ScoreServiceTest.java(7 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/test/java/com/example/solidconnection/score/service/ScoreServiceTest.java (1)
src/test/java/com/example/solidconnection/support/integration/BaseIntegrationTest.java (1)
TestContainerSpringBootTest(51-527)
src/test/java/com/example/solidconnection/score/fixture/LanguageTestScoreFixtureBuilder.java (1)
src/test/java/com/example/solidconnection/score/fixture/LanguageTestScoreFixture.java (1)
TestComponent(13-26)
src/test/java/com/example/solidconnection/score/fixture/GpaScoreFixture.java (1)
src/test/java/com/example/solidconnection/score/fixture/GpaScoreFixtureBuilder.java (1)
TestComponent(11-46)
src/test/java/com/example/solidconnection/score/fixture/GpaScoreFixtureBuilder.java (1)
src/test/java/com/example/solidconnection/score/fixture/GpaScoreFixture.java (1)
TestComponent(10-23)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (24)
src/test/java/com/example/solidconnection/score/fixture/GpaScoreFixtureBuilder.java (2)
11-13: 잘 설계된 테스트 컴포넌트 구조입니다!테스트 컴포넌트로 GpaScoreFixtureBuilder를 구현한 것은 좋은 접근법입니다. 스프링 DI를 활용하여 테스트에서 재사용 가능한 구성요소를 만들었습니다.
21-23: 팩토리 메소드 패턴이 잘 적용되었습니다.새 인스턴스를 생성하는 gpaScore() 메소드는 빌더 패턴을 사용할 때 상태 초기화를 보장하는 좋은 방법입니다.
src/test/java/com/example/solidconnection/admin/service/AdminGpaScoreServiceTest.java (6)
10-13: 테스트 환경 설정 개선이 잘 되었습니다.
- GpaScoreFixture 임포트 추가
- TestContainerSpringBootTest로 전환
- 이전 BaseIntegrationTest에서 더 명확한 의도를 가진 어노테이션으로 변경되었습니다.
38-42: 의존성 주입 방식이 개선되었습니다.픽스처 기반 접근법으로 리팩토링하면서 SiteUserFixture와 GpaScoreFixture를 주입받도록 변경한 것은 좋은 개선입니다. 이렇게 하면 테스트 데이터 생성 로직이 중앙화되고 일관성이 유지됩니다.
53-55: 테스트 데이터 생성 방식이 간소화되었습니다.픽스처 메서드를 사용하여 테스트 데이터를 생성하도록 변경한 것은 코드의 가독성과 유지보수성을 크게 향상시킵니다.
72-73: 단순화된 검증 로직이 적용되었습니다.PR 설명에서 언급한 대로 단순히 size()만 확인하도록 어설션을 단순화했습니다. 이 변경은 테스트의 의도를 명확히 하고 불필요한 세부 검증을 제거했습니다.
86-87: 일관된 검증 패턴이 적용되었습니다.단순화된 어설션 패턴이 일관되게 적용되어 테스트 코드의 일관성이 향상되었습니다.
100-101: 검증 로직의 명확성이 향상되었습니다.모든 조건으로 조회하는 테스트에서도 동일한 패턴의 검증 로직이 적용되어 일관성이 유지되었습니다.
src/test/java/com/example/solidconnection/admin/service/AdminLanguageTestScoreServiceTest.java (6)
10-13: 테스트 환경 설정 개선이 잘 이루어졌습니다.
- LanguageTestScoreFixture 임포트 추가
- TestContainerSpringBootTest 전환
- 이러한 변경은 GpaScoreServiceTest의 변경과 일관성 있게 적용되었습니다.
39-43: 의존성 주입 방식이 개선되었습니다.SiteUserFixture와 LanguageTestScoreFixture를 주입받도록 변경한 것은 테스트 데이터 생성의 일관성과 재사용성을 향상시킵니다.
54-56: 테스트 데이터 생성 방식이 단순화되었습니다.픽스처 메서드를 사용하여 테스트 데이터를 생성하도록 변경한 것은 코드를 더 읽기 쉽고 유지보수하기 쉽게 만듭니다.
72-74: 검증 로직이 단순화되었습니다.사이즈만 확인하는 방식으로 어설션을 단순화한 것은 테스트의 의도를 명확히 하고 불필요한 상세 검증을 제거했습니다.
87-89: 이전 검증 로직의 복잡성이 제거되었습니다.닉네임으로 조회하는 테스트에서도 단순화된 검증 로직이 적용되어 일관성이 유지되었습니다.
102-103: 일관된 검증 패턴이 적용되었습니다.모든 조건으로 조회하는 테스트에서도 동일한 패턴의 검증 로직이 적용되어 테스트 코드의 일관성이 향상되었습니다.
src/test/java/com/example/solidconnection/score/fixture/GpaScoreFixture.java (2)
10-15: 잘 설계된 테스트 픽스처 구조입니다!
- 테스트 컴포넌트 어노테이션 적용
- 생성자 주입 패턴 사용
- GpaScoreFixtureBuilder 의존성 주입
이러한 접근 방식은 테스트 데이터 생성을 중앙화하고 일관성 있게 만드는 좋은 방법입니다.
16-22: 명확하고 간결한 테스트 데이터 생성 메서드입니다.한국어 메서드명(GPA_점수)을 사용한 것은 테스트 코드의 가독성을 높이는 좋은 방법입니다. 테스트에서 검증 상태와 사용자만 입력하면 되어 사용이 쉽고, 내부적으로는 빌더 패턴을 통해 유연한 구현을 제공합니다.
src/test/java/com/example/solidconnection/score/service/ScoreServiceTest.java (8)
13-14: 테스트 애노테이션 변경 및 픽스처 클래스 활용 적용테스트 클래스 구조가 개선되었습니다! 다음과 같은 변화가 있습니다:
애노테이션 변경
@BaseIntegrationTest→@TestContainerSpringBootTest로 변경- 더 명확한 테스트 컨테이너 설정 방식 적용
픽스처 활용
- GpaScoreFixture, LanguageTestScoreFixture 임포트 추가
- 테스트 데이터 설정 방식의 일관성 향상
이러한 변경으로 테스트 코드의 가독성과 유지보수성이 향상되었습니다!
Also applies to: 19-19, 33-33
52-56: 픽스처 의존성 주입 적용픽스처 클래스들을
@Autowired로 주입하여 테스트 설정을 체계화했습니다. 이는 다음과 같은 장점이 있습니다:
테스트 데이터 생성 로직 중앙화
- 테스트 데이터 생성 방식 일관성 확보
- 코드 중복 감소
테스트 가독성 향상
- 복잡한 엔티티 생성 코드가 간결한 메소드 호출로 대체
- 테스트의 의도가 더 명확하게 표현됨
이 접근 방식은 테스트 코드의 품질을 크게 향상시킵니다!
68-70: GPA 점수 생성 방식의 개선직접 엔티티를 생성하는 대신 픽스처 메소드를 사용하여 테스트 데이터를 설정하도록 변경했습니다. 이는 다음과 같은 이점이 있습니다:
코드 간결화
- 복잡한 엔티티 생성 로직이 간단한 메소드 호출로 대체
- 테스트 의도에 집중할 수 있는 구조
유지보수성 개선
- 엔티티 구조 변경 시 픽스처 클래스만 수정하면 됨
- 여러 테스트에서 일관된 테스트 데이터 사용 가능
좋은 리팩토링입니다!
77-77: 검증 로직 간소화이전 코드에서 엔티티 필드 단위 비교에서 크기 검증으로 단순화되었습니다. 이는 다음과 같은 효과가 있습니다:
테스트 유연성 향상
- 구체적인 필드 값보다 목록 크기에 집중
- 테스트 데이터 변경에 덜 취약한 구조
핵심 기능 검증에 집중
- 불필요한 세부 사항 검증 제거
- 실제 확인해야 할 동작(목록 크기)에 집중
테스트의 의도가 더 명확해졌습니다!
92-94: 어학 점수 생성 방식의 개선어학 점수 생성도 픽스처 메소드를 사용하여 개선되었습니다:
코드 통일성
- GPA 점수와 동일한 패턴으로 테스트 데이터 생성
- 일관된 코드 스타일 유지
가독성 향상
- 복잡한 엔티티 생성 코드 대신 의미 있는 메소드명 사용
- 테스트 의도가 더 명확하게 드러남
잘 구현된 리팩토링입니다!
101-101: 어학 점수 검증 로직 간소화GPA 점수와 같은 방식으로 어학 점수 검증 로직도 간소화되었습니다:
일관된 검증 스타일
- 모든 테스트에서 동일한 검증 패턴 사용
- 테스트 코드 이해가 더 쉬워짐
유지보수성 향상
- 세부 구현 변경에 덜 민감한 테스트
- 핵심 기능(목록 크기)에 집중한 검증
좋은 변경입니다!
126-126: GPA 점수 등록 테스트 검증 간소화GPA 점수 등록 테스트에서 검증 로직이 간소화되었습니다:
검증 범위 축소
- 모든 필드 대신 ID만 검증
- 핵심 기능(저장 성공)에 집중
관심사 분리
- 등록 기능과 필드 값 검증을 분리
- 테스트의 주요 목적이 더 명확해짐
테스트의 의도가 더 명확해졌습니다!
142-142: 어학 점수 등록 테스트 검증 간소화어학 점수 등록 테스트도 GPA 점수와 동일한 방식으로 간소화되었습니다:
일관된 테스트 스타일
- 모든 등록 테스트에서 같은 검증 패턴 사용
- 코드 일관성 확보
테스트 의도 명확화
- 등록 성공 여부만 확인하도록 집중
- 세부 구현에 덜 의존적인 검증 방식
좋은 리팩토링입니다!
src/test/java/com/example/solidconnection/score/fixture/LanguageTestScoreFixtureBuilder.java
Show resolved
Hide resolved
| () -> assertThat(actual.siteUserResponse().profileImageUrl()).isEqualTo(expected.getSiteUser().getProfileImageUrl()), | ||
| () -> assertThat(actual.siteUserResponse().nickname()).isEqualTo(expected.getSiteUser().getNickname()) | ||
| )); | ||
| assertThat(response.getContent()).hasSize(expectedGpaScores.size()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
어드민쪽 조회 테스트 시 너무 자세히 검증하던 then절을 좀 간소화했습니다! size()정도만 검증해도 괜찮을까요?
이 테스트 코드에서 검증하려는 것은, "verifyStatus으로 조회했을 때 잘 되는가?" 라고 생각해요.
아래와 같이 응답의 모든 status가 PENDIND임을 확인해주는게 필요할 것 같습니다.
다른곳들도 마찬가지이고요! 🫨
| assertThat(response.getContent()).hasSize(expectedGpaScores.size()); | |
| assertThat(response.getContent()).hasSize(expectedGpaScores.size()); | |
| assertThat(response.getContent()).hasSize(expectedGpaScores.size()); | |
| assertThat(response.getContent()) | |
| .extracting(content -> content.gpaScoreStatusResponse().verifyStatus()) | |
| .containsOnly(VerifyStatus.PENDING); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그렇네요 좋은 거 같습니다! 추가로 닉네임으로 검증하는 것도 필요할 거 같아서 그것만 추가해주었습니다!
(8ca6823)
| languageTestScore3 = languageTestScoreFixture.어학_점수(VerifyStatus.REJECTED, user3); | ||
| languageTestScore2 = languageTestScoreFixture.어학_점수(VerifyStatus.PENDING, user2); | ||
| languageTestScore1 = languageTestScoreFixture.어학_점수(VerifyStatus.PENDING, user1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
지금 보니 묘~하게 헷갈리는데 이것도 1,2,3 으로 순서를 맞추는건 어떤가요? 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
반영했습니다! (e5f5aa2)
nayonsoso
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다😊
관련 이슈
작업 내용
성적 관련 통합 테스트 데이터 fixture 메서드로 변경했습니다!
이제 커뮤니티랑 모의지원만 해결하면 끝이네요!
특이 사항
리뷰 요구사항 (선택)
어드민쪽 조회 테스트 시 너무 자세히 검증하던 then절을 좀 간소화했습니다! size()정도만 검증해도 괜찮을까요?