-
Notifications
You must be signed in to change notification settings - Fork 0
feature/#109 Post 테스트 코드 작성 #117
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
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 labels
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: 8
🧹 Outside diff range and nitpick comments (16)
aics-domain/src/testFixtures/java/mock/TestEntityUtils.java (3)
1-7
: 클래스 문서화 추가 필요이 유틸리티 클래스의 목적과 사용 방법에 대한 JavaDoc 문서를 추가하면 좋겠습니다.
다음과 같은 문서 추가를 제안합니다:
+/** + * BaseTimeEntity의 테스트를 위한 유틸리티 클래스입니다. + * 리플렉션을 사용하여 BaseTimeEntity의 private 필드에 접근하고 수정합니다. + * 테스트 환경에서만 사용되어야 합니다. + */ public class TestEntityUtils {
8-8
: 유틸리티 클래스 final 선언 추가유틸리티 클래스는 상속을 방지하기 위해 final로 선언하는 것이 좋습니다.
-public class TestEntityUtils { +public final class TestEntityUtils {
1-19
: 테스트 유틸리티 사용에 대한 아키텍처 조언리플렉션을 사용하는 테스트 유틸리티는 다음 사항을 고려해야 합니다:
- 테스트 격리: 이 유틸리티는 테스트 환경에서만 사용되어야 합니다.
- 유지보수성: BaseTimeEntity 클래스가 변경될 경우 이 유틸리티도 업데이트가 필요합니다.
- 문서화: 사용 방법과 주의사항을 명확히 문서화해야 합니다.
README.md
또는 테스트 문서에 이 유틸리티의 사용 지침을 추가하는 것을 추천드립니다.aics-api/src/main/java/kgu/developers/api/post/presentation/request/PostRequest.java (1)
제목 길이 제한에 대한 테스트 케이스가 누락되어 있습니다
현재 테스트 코드에는 게시글 제목의 길이 제한(100자)에 대한 유효성 검증 테스트가 누락되어 있습니다. 다음 위치에 테스트 케이스 추가가 필요합니다:
aics-api/src/testFixtures/java/post/presentation/PostControllerTest.java
에 제목 길이 초과 시 예외 발생 테스트aics-api/src/testFixtures/java/post/application/PostServiceTest.java
에 제목 길이 제한 경계값 테스트🔗 Analysis chain
Line range hint
13-15
: 제목 길이 제한에 대한 테스트 케이스 추가 제안
@Size(max = 100)
제약 조건에 대한 검증이 필요합니다. 제목 길이가 100자를 초과할 때 적절한 예외가 발생하는지 확인하는 테스트 케이스를 추가하면 좋을 것 같습니다.테스트 케이스 작성에 도움이 필요하시다면 말씀해 주세요.
Let me search for existing test files to verify if there are any test cases for the title length validation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: 제목 길이 제한 관련 테스트 케이스 존재 여부 확인 # PostControllerTest나 PostServiceTest에서 제목 길이 제한 테스트 검색 rg -l "title.*100|제목.*100" --type javaLength of output: 125
Script:
#!/bin/bash # Search for test files related to Post fd "Post.*Test\.java$" --type f # Then let's look at the content of these test files to check for title validation tests fd "Post.*Test\.java$" --type f --exec cat {}Length of output: 7956
aics-api/src/testFixtures/java/post/application/FakeUserService.java (3)
9-12
: 필드 중복 선언 검토 필요상위 클래스인
UserService
에 이미 동일한 필드들이 존재할 가능성이 높습니다. 하위 클래스에서 동일한 필드를 다시 선언하는 것은 불필요한 중복일 수 있습니다.다음과 같이 필드 선언을 제거하는 것을 고려해보세요:
public class FakeUserService extends UserService { - private final UserRepository userRepository; - private final BCryptPasswordEncoder bCryptPasswordEncoder;
13-17
: 생성자 구현 단순화 필요필드 할당이 중복되어 있습니다.
super()
호출만으로 충분할 것 같습니다.다음과 같이 수정하는 것을 추천드립니다:
public FakeUserService(UserRepository userRepository, BCryptPasswordEncoder bCryptPasswordEncoder) { super(bCryptPasswordEncoder, userRepository); - this.userRepository = userRepository; - this.bCryptPasswordEncoder = bCryptPasswordEncoder; }
1-29
: 테스트 데이터 생성 로직 분리 고려현재 테스트용 사용자 생성 로직이
me()
메소드에 직접 구현되어 있습니다. 다양한 테스트 시나리오를 지원하기 위해서는 더 유연한 구조가 필요할 것 같습니다.다음과 같은 개선을 고려해보세요:
- 테스트 데이터 생성을 위한 별도의 빌더 메소드 추가
- 테스트용 사용자 속성을 커스터마이징할 수 있는 메소드 제공
TestEntityUtils
와 통합하여 일관된 테스트 데이터 생성 방식 구현예시 코드를 생성해드릴까요?
aics-domain/src/testFixtures/java/mock/FakePostRepository.java (3)
43-66
: 스트림 체이닝 최적화 필요현재 구현에서는 중간 리스트를 여러 번 생성하여 메모리 사용량이 증가할 수 있습니다. 다음과 같이 최적화하는 것을 추천드립니다:
- List<Post> filteredPosts = data.stream() - .filter( - post -> post.getTitle().contains(keyword) && post.getCategory().equals(category) - ) - .sorted(Comparator.comparing(Post::getCreatedAt).reversed()) - .collect(Collectors.toList()); - - int start = (int) pageable.getOffset(); - int end = Math.min(start + pageable.getPageSize(), filteredPosts.size()); - - List<Post> paginatedPosts = start > filteredPosts.size() ? - Collections.emptyList() : - filteredPosts.subList(start, end); + List<Post> paginatedPosts = data.stream() + .filter(post -> post.getTitle().contains(keyword) && post.getCategory().equals(category)) + .sorted(Comparator.comparing(Post::getCreatedAt).reversed()) + .skip(pageable.getOffset()) + .limit(pageable.getPageSize()) + .collect(Collectors.toList());
68-73
: findById 성능 개선 제안스트림 대신 일반 반복문을 사용하면 성능이 개선될 수 있습니다. ID 검색은 첫 번째 일치하는 항목을 찾으면 즉시 반환할 수 있기 때문입니다.
@Override public Optional<Post> findById(Long postId) { - return data.stream() - .filter(post -> post.getId().equals(postId)) - .findAny(); + for (Post post : data) { + if (post.getId().equals(postId)) { + return Optional.of(post); + } + } + return Optional.empty(); }
75-79
: 삭제 작업 로깅 추가 권장대량 삭제 작업의 경우, 삭제된 게시물의 수를 로깅하면 테스트 디버깅에 도움이 될 수 있습니다.
@Override public void deleteAllByDeletedAtBefore(int retentionDays) { LocalDateTime threshold = LocalDateTime.now().minusDays(retentionDays); + int initialSize = data.size(); data.removeIf(post -> post.getDeletedAt() != null && post.getDeletedAt().isBefore(threshold)); + int deletedCount = initialSize - data.size(); + System.out.println("삭제된 게시물 수: " + deletedCount); }aics-api/src/main/java/kgu/developers/api/post/application/PostService.java (2)
37-38
: createPost 메서드에 입력값 검증 로직 추가를 고려해주세요.현재 구현은 명확하고 잘 작동하지만, 다음과 같은 개선사항을 제안드립니다:
- 제목과 내용의 길이 제한 검증
- 카테고리 null 체크
@Transactional public PostPersistResponse createPost(PostRequest request, Category category) { + validatePostRequest(request, category); User author = userService.me(); Post createPost = Post.create(request.title(), request.content(), category, author); Long id = postRepository.save(createPost).getId(); return PostPersistResponse.from(id); } + +private void validatePostRequest(PostRequest request, Category category) { + if (category == null) { + throw new IllegalArgumentException("카테고리는 필수입니다."); + } + if (request.title() == null || request.title().trim().isEmpty()) { + throw new IllegalArgumentException("제목은 필수입니다."); + } + if (request.content() == null || request.content().trim().isEmpty()) { + throw new IllegalArgumentException("내용은 필수입니다."); + } +}
스케줄된 정리 작업에 대한 테스트 케이스 추가가 필요합니다.
현재 테스트 코드에서 다음과 같은 문제점들이 발견되었습니다:
cleanupOldDeletedPosts
메서드에 대한 테스트가 누락되어 있습니다.FakePostRepository
에는deleteAllByDeletedAtBefore
구현이 있으나, 이를 검증하는 테스트가 없습니다.lastScheduledRun
업데이트 검증도 누락되어 있습니다.다음 테스트 케이스들을 추가해주세요:
- 삭제된 게시물이 보존 기간을 초과했을 때 정상적으로 제거되는지 확인
- 보존 기간 이내의 삭제된 게시물은 유지되는지 확인
lastScheduledRun
시간이 정확히 업데이트되는지 확인🔗 Analysis chain
Line range hint
74-83
: 스케줄된 정리 작업의 테스트 검증이 필요합니다.
cleanupOldDeletedPosts
메서드에 대한 테스트 케이스가 필요해 보입니다. 다음 사항들을 확인해주세요:
- 정리 작업의 정상 동작 여부
- lastScheduledRun 업데이트 검증
- POST_RETENTION_DAYS 기간 이전/이후 삭제된 게시물 처리
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: 테스트 코드에서 cleanup 관련 테스트 케이스 확인 # PostServiceTest에서 cleanup 관련 테스트 검색 rg -t java "cleanup|deleteAllByDeletedAtBefore" "src/test"Length of output: 107
Script:
#!/bin/bash # 전체 테스트 디렉토리 구조 확인 fd -t d "test" # 테스트 파일 검색 fd -t f "PostService.*Test.*\.java$" # cleanup 관련 코드 검색 (전체 코드베이스) rg -t java "cleanup|deleteAllByDeletedAtBefore"Length of output: 2417
Script:
#!/bin/bash # PostServiceTest 파일의 내용 확인 cat aics-api/src/testFixtures/java/post/application/PostServiceTest.java cat aics-domain/src/testFixtures/java/post/application/PostServiceTest.java # FakePostRepository 구현 확인 cat aics-domain/src/testFixtures/java/mock/FakePostRepository.javaLength of output: 7102
aics-domain/src/testFixtures/java/post/domain/PostDomainTest.java (2)
1-15
: 테스트 클래스 구성에 대한 제안테스트 클래스가 잘 구성되어 있습니다만, 다음과 같은 개선을 고려해보시기 바랍니다:
- 테스트 케이스들을 논리적 그룹으로 구분하기 위해
@Nested
클래스 사용- 공통 테스트 데이터 설정을 위한
@BeforeEach
메서드 추가
116-125
: 테스트 사용자 생성 메서드 개선 제안현재 고정된 값으로 사용자를 생성하고 있습니다. 테스트의 유연성을 위해 다음과 같이 개선해보세요:
- public User getUser() { + public User getUser(String id, String name, Major major) { - String id = "202411345"; String password = "password"; - String name = "홍길동"; String email = "valid@kyonggi.ac.kr"; String phone = "010-1234-5678"; - Major major = Major.CSE; return User.create(id, password, name, email, phone, major); } + public User getDefaultUser() { + return getUser("202411345", "홍길동", Major.CSE); + }aics-api/src/testFixtures/java/post/application/PostServiceTest.java (2)
28-66
: 테스트 데이터 설정 개선이 필요합니다테스트 데이터 설정에 다음과 같은 개선사항을 제안합니다:
- 테스트 데이터용 상수를 정의하여 매직 넘버/문자열 제거
- 테스트 데이터 설정의 목적을 설명하는 주석 추가
다음과 같이 개선할 수 있습니다:
+ private static final String TEST_USER1_ID = "202411345"; + private static final String TEST_USER2_ID = "202411346"; + private static final String TEST_POST1_TITLE = "테스트용 제목1"; + private static final String TEST_POST2_TITLE = "테스트용 제목2"; @BeforeEach public void init() { + // 테스트를 위한 기본 사용자 2명과 게시글 2개 설정 BCryptPasswordEncoder bCryptPasswordEncoder = new BCryptPasswordEncoder(); // ... 나머지 코드 ... User user1 = User.create( - "202411345", + TEST_USER1_ID, // ... 나머지 코드 ... );
111-127
: 검색 테스트의 검증을 강화해 주세요현재
getPostsByKeywordAndCategory
테스트는 결과 개수만 확인하고 있습니다. 다음 항목들도 검증하면 좋을 것 같습니다:
- 반환된 게시글들의 제목에 검색어가 포함되어 있는지
- 반환된 게시글들의 카테고리가 일치하는지
- 페이징이 올바르게 동작하는지
// then assertEquals(2, posts.contents().size()); +assertTrue(posts.contents().stream() + .allMatch(post -> post.title().contains(keyword))); +assertTrue(posts.contents().stream() + .allMatch(post -> post.category().equals(category))); +assertEquals(0, posts.pageNumber()); +assertEquals(size, posts.pageSize());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
aics-api/src/main/java/kgu/developers/api/post/application/PostService.java
(2 hunks)aics-api/src/main/java/kgu/developers/api/post/presentation/request/PostRequest.java
(1 hunks)aics-api/src/testFixtures/java/post/application/FakeUserService.java
(1 hunks)aics-api/src/testFixtures/java/post/application/PostServiceTest.java
(1 hunks)aics-api/src/testFixtures/java/post/presentation/PostControllerTest.java
(1 hunks)aics-domain/src/testFixtures/java/mock/FakePostRepository.java
(1 hunks)aics-domain/src/testFixtures/java/mock/TestEntityUtils.java
(1 hunks)aics-domain/src/testFixtures/java/post/application/PostServiceTest.java
(1 hunks)aics-domain/src/testFixtures/java/post/domain/PostDomainTest.java
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- aics-api/src/testFixtures/java/post/presentation/PostControllerTest.java
- aics-domain/src/testFixtures/java/post/application/PostServiceTest.java
🔇 Additional comments (6)
aics-api/src/main/java/kgu/developers/api/post/presentation/request/PostRequest.java (1)
8-8
: @builder 어노테이션 추가가 적절합니다.
Builder 패턴을 사용함으로써 객체 생성이 더욱 유연해지고 가독성이 향상될 것입니다. 특히 테스트 코드 작성 시 객체 생성이 더욱 편리해질 것으로 예상됩니다.
Also applies to: 10-10
aics-domain/src/testFixtures/java/mock/FakePostRepository.java (1)
19-21
: 스레드 안전성이 잘 구현되어 있습니다!
Collections.synchronizedList
와 AtomicLong
을 사용하여 멀티스레드 환경에서의 안전성을 보장하고 있습니다.
aics-api/src/main/java/kgu/developers/api/post/application/PostService.java (2)
15-21
: 임포트 구문 정리가 잘 되었습니다!
코드의 가독성이 향상되었습니다.
42-42
: 메서드 파라미터 정렬이 개선되었습니다.
코드의 일관성과 가독성이 향상되었습니다.
aics-domain/src/testFixtures/java/post/domain/PostDomainTest.java (2)
57-74
: 내용 업데이트 테스트도 동일한 개선 필요
앞서 제목 업데이트 테스트와 동일한 문제점이 있습니다:
- 테스트 데이터 설정 중복
- 유효성 검사 케이스 누락
94-114
: 고정 상태 토글 테스트 승인
고정 상태 변경에 대한 테스트가 잘 작성되었습니다. 두 상태 모두 정확하게 검증하고 있습니다.
aics-api/src/testFixtures/java/post/application/FakeUserService.java
Outdated
Show resolved
Hide resolved
aics-domain/src/testFixtures/java/post/domain/PostDomainTest.java
Outdated
Show resolved
Hide resolved
aics-api/src/testFixtures/java/post/application/PostServiceTest.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.
크리티컬한 이슈 발견해주셔 감사합니다!
우선 테스트 전략에 대한 코멘트만 달았는데 확인 부탁드립니다 👍
aics-api/src/testFixtures/java/post/application/FakeUserService.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.
수정 감사합니다~ 마지막으로 코멘트 확인 부탁드려요~
User user1 = User.create( | ||
"202411345", | ||
bCryptPasswordEncoder.encode("password1234"), | ||
"홍길동", | ||
"test@kyonggi.ac.kr", | ||
"010-1234-5678", | ||
Major.CSE); | ||
|
||
User user2 = User.create( | ||
"202411346", | ||
bCryptPasswordEncoder.encode("password5678"), | ||
"신짱구", | ||
"shin@kyonggi.ac.kr", | ||
"010-5678-1234", | ||
Major.AIT); | ||
|
||
fakeUserRepository.save(user1); | ||
fakeUserRepository.save(user2); | ||
|
||
UserDetails user = fakeUserRepository.findById(user1.getId()).orElseThrow(); | ||
SecurityContext context = SecurityContextHolder.getContext(); | ||
context.setAuthentication( | ||
new UsernamePasswordAuthenticationToken(user, user.getPassword(), user.getAuthorities()) | ||
); |
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.
지금 PostServiceTest에서 FakeUserRepository에 유저 객체를 저장하고 있는데요,
해당 코드 UserServiceTest에서도 동일하게 존재하기 때문에 이를 재사용할 수 있는 방법이 있을까요?
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.
회의 내용 추가해서 반영했습니다!
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
Tasks