-
Notifications
You must be signed in to change notification settings - Fork 0
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
feature/#122 연구실 테스트 코드 작성 #124
Conversation
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough이 풀 리퀘스트에서는 Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 5
🧹 Outside diff range and nitpick comments (6)
aics-api/src/main/java/kgu/developers/api/lab/application/LabService.java (1)
24-26
: 변수명 개선이 필요합니다.
createLab
이라는 변수명은 메서드명과 유사하여 혼란을 줄 수 있습니다. 저장된 엔티티임을 더 명확히 표현하는 것이 좋겠습니다.다음과 같이 변경을 제안드립니다:
- Lab createLab = labRepository.save(lab); - return LabPersistResponse.of(createLab.getId()); + Lab savedLab = labRepository.save(lab); + return LabPersistResponse.of(savedLab.getId());aics-domain/src/testFixtures/java/mock/FakeLabRepository.java (1)
19-26
: 유효성 검증 메시지를 더 구체적으로 개선해주세요.현재 예외 메시지가 다소 일반적입니다. 각 필드별로 구체적인 검증 메시지를 제공하면 테스트 실패 시 원인 파악이 더 용이할 것 같습니다.
다음과 같이 개선을 제안드립니다:
- if (lab.getName() == null - || lab.getLoc() == null - || lab.getSite() == null) { - throw new IllegalArgumentException("이름, 위치, 사이트는 필수 입력값입니다."); + if (lab.getName() == null) { + throw new IllegalArgumentException("연구실 이름은 필수 입력값입니다."); + } + if (lab.getLoc() == null) { + throw new IllegalArgumentException("연구실 위치는 필수 입력값입니다."); + } + if (lab.getSite() == null) { + throw new IllegalArgumentException("연구실 사이트는 필수 입력값입니다.");aics-domain/src/testFixtures/java/lab/domain/LabDomainTest.java (2)
11-27
: 테스트 케이스 보완이 필요합니다현재 테스트는 기본적인 성공 케이스만 다루고 있습니다. 다음과 같은 케이스들을 추가하면 좋을 것 같습니다:
- 잘못된 입력값 (빈 문자열, null 등)에 대한 검증
- 실제 연구실 데이터와 유사한 테스트 데이터 사용
31-31
: 오타를 수정해 주세요DisplayName의 "얀구실명"을 "연구실명"으로 수정해 주세요.
aics-api/src/testFixtures/java/lab/application/LabServiceTest.java (2)
21-42
: 테스트 데이터를 상수로 분리하면 좋을 것 같습니다테스트에서 사용되는 문자열 값들을 클래스 상수로 분리하면 유지보수성이 향상될 것 같습니다.
예시:
public class LabServiceTest { + private static final String LAB_A_NAME = "Lab A"; + private static final String LAB_A_LOC = "8500"; + private static final String LAB_A_SITE = "http://lab1.kyonggi.ac.kr"; + private static final String LAB_B_NAME = "Lab B"; + private static final String LAB_B_LOC = "8520"; + private static final String LAB_B_SITE = "https://lab2.kyonggi.ac.kr";
90-112
: 정렬 의존성을 명시적으로 테스트해 주세요현재 테스트는 이름순 정렬에 의존하고 있습니다. 정렬 로직이 변경될 경우 테스트가 실패할 수 있으므로, 정렬과 무관하게 특정 Lab을 찾아서 검증하는 것이 좋을 것 같습니다.
-//then - 이름순이기 때문에 순서까지 바뀌어서 리턴 -assertEquals(newName, contents.get(1).name()); -assertEquals(newLoc, contents.get(1).loc()); -assertEquals(newSite, contents.get(1).site()); +//then +LabDetailResponse updatedLab = contents.stream() + .filter(lab -> lab.name().equals(newName)) + .findFirst() + .orElseThrow(); +assertEquals(newLoc, updatedLab.loc()); +assertEquals(newSite, updatedLab.site());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
aics-api/src/main/java/kgu/developers/api/lab/application/LabService.java
(1 hunks)aics-api/src/main/java/kgu/developers/api/lab/presentation/request/LabRequest.java
(1 hunks)aics-api/src/testFixtures/java/lab/application/LabServiceTest.java
(1 hunks)aics-api/src/testFixtures/java/lab/presentation/LabControllerTest.java
(1 hunks)aics-domain/src/testFixtures/java/lab/application/LabServiceTest.java
(1 hunks)aics-domain/src/testFixtures/java/lab/domain/LabDomainTest.java
(1 hunks)aics-domain/src/testFixtures/java/mock/FakeLabRepository.java
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- aics-domain/src/testFixtures/java/lab/application/LabServiceTest.java
- aics-api/src/testFixtures/java/lab/presentation/LabControllerTest.java
🔇 Additional comments (2)
aics-api/src/main/java/kgu/developers/api/lab/presentation/request/LabRequest.java (1)
9-11
: 빌더 패턴 적용이 적절합니다!
테스트 코드 작성을 위한 @Builder
어노테이션 추가가 적절합니다. 이를 통해 테스트 픽스처 생성이 더욱 용이해질 것입니다.
aics-api/src/testFixtures/java/lab/application/LabServiceTest.java (1)
114-127
: 테스트가 잘 작성되었습니다
삭제 후 컬렉션의 크기와 남은 항목을 모두 검증하고 있어 완성도가 높습니다.
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.
깔끔한 테코 감사합니다 👍
한가지 코멘트만 검토 부탁드려요~
aics-domain/src/testFixtures/java/lab/domain/LabDomainTest.java
Outdated
Show resolved
Hide resolved
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.
LGTM👍
Summary
testFixtures 기반 연구실 테스트 코드 작성
Tasks