Merged
Conversation
📝 WalkthroughWalkthrough채팅 API용 통합 테스트 클래스 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/test/java/gg/agit/konect/integration/domain/chat/ChatApiTest.java`:
- Around line 64-75: setUp() currently creates broad fixtures
(UniversityFixture, UserFixture, ClubFixture, ClubMemberFixture and persists
adminUser, normalUser, targetUser, outsiderUser, club) for every test which
hides true dependencies; narrow the fixture scope by removing club and club
member creation from the global setUp() and instead create helper methods (e.g.,
createClubWithMembers(ClubFixture, ClubMemberFixture)) or move club/member
persists into the specific tests or nested test classes that require them (use
`@Nested` or individual `@BeforeEach` for chat/message vs admin-room scenarios),
while keeping shared user/university persists (UniversityFixture, UserFixture,
adminUser, normalUser, outsiderUser) in setUp(); ensure clear references to
setUp(), ClubFixture, ClubMemberFixture, and any new helper methods so tests
only prepare the data they actually depend on.
- Around line 153-163: The test currently assumes the admin room is at rooms[0],
which is brittle; change it to capture the chatRoomId returned by
performPost("/chats/rooms/admin") (inspect its JSON response for "chatRoomId")
and then use that id to locate the correct room in the
performGet("/chats/rooms") response (or alternatively assert the response
contains the expected number of rooms before indexing). Concretely: after
calling performPost("/chats/rooms/admin") extract the chatRoomId from the
response, then in the performGet("/chats/rooms") assertions locate the room
whose chatRoomId equals that value and assert its chatType, roomName,
lastMessage absence, and isMuted flag instead of hardcoding rooms[0].
- Around line 92-94: The test in ChatApiTest currently only asserts presence of
a chat room via chatRoomRepository.findByTwoUsers(normalUser.getId(),
targetUser.getId()) after creation; update both affected test blocks (including
the one around lines 132-139) to also assert that no duplicate room was created
by recording the count or ensuring exactly one room exists for that user pair
before and after the create flow—e.g., query the repository for rooms between
normalUser and targetUser (using chatRoomRepository.findByTwoUsers or a count
method) before the operation, perform the creation, call
clearPersistenceContext(), then re-query and assert the result is present and
that the total count for that pair is unchanged/equals one so the contract
“exactly one room for the pair” is enforced.
- Around line 272-284: The test is too tightly coupled to an internal storage
representation by insisting a NotificationMuteSetting row exists with isMuted ==
false; update the assertion around
notificationMuteSettingRepository.findByTargetTypeAndTargetIdAndUserId(NotificationTargetType.CHAT_ROOM,
chatRoom.getId(), normalUser.getId()) to allow either the row to be absent or,
if present, to have getIsMuted() == false (e.g., assert that Optional.isEmpty()
OR Optional.get().getIsMuted() == false); keep the earlier performPost and
jsonPath assertions unchanged and only change the repository verification to the
permissive OR check so the test will pass whether unmute is implemented as
deletion or flag flip.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: caa77b7b-8b68-412c-8ee9-a6ad7e3d0d32
📒 Files selected for processing (1)
src/test/java/gg/agit/konect/integration/domain/chat/ChatApiTest.java
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/test/java/gg/agit/konect/integration/domain/chat/ChatApiTest.java
src/test/java/gg/agit/konect/integration/domain/chat/ChatApiTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/test/java/gg/agit/konect/integration/domain/chat/ChatApiTest.java (1)
161-171:⚠️ Potential issue | 🟡 Minor[LEVEL: MEDIUM] 목록 검증을
rooms[0]에 고정하지 마세요.정렬 기준이 바뀌거나 기본 데이터가 추가되면 기능은 정상이더라도 이 테스트만 깨집니다.
POST /chats/rooms/admin응답의chatRoomId를 받아 그 ID로 목록에서 대상을 찾아 검증하는 방식이 더 안전합니다. As per coding guidelines, '가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/gg/agit/konect/integration/domain/chat/ChatApiTest.java` around lines 161 - 171, The test currently assumes the new room is at rooms[0], which is brittle; instead capture the chatRoomId returned by performPost("/chats/rooms/admin") and use that ID to locate the matching room in the GET /chats/rooms response before asserting its fields. Concretely: call performPost("/chats/rooms/admin") and extract the chatRoomId from the response, then call performGet("/chats/rooms"), find the room whose chatRoomId equals that extracted ID, and assert that its chatType is "DIRECT", roomName equals adminUser.getName(), lastMessage does not exist, and isMuted is false (use performPost, performGet, adminUser, and the chatRoomId value to find the correct room instead of hardcoding rooms[0]).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/test/java/gg/agit/konect/integration/domain/chat/ChatApiTest.java`:
- Around line 154-172: Update the test createAdminChatRoomAndGetRoomsSuccess to
also exercise the "existing room returned" branch by calling the POST
/chats/rooms/admin endpoint twice: capture the chatRoomId from the first
performPost response, call performPost a second time and assert the returned
chatRoomId equals the first one, then performGet("/chats/rooms") and assert the
rooms list size did not increase (e.g., still one room) and other expectations
(chatType, roomName, no lastMessage, isMuted false) still hold; use the existing
performPost/performGet helpers and JSON path extraction from the responses to
compare IDs and verify room count.
---
Duplicate comments:
In `@src/test/java/gg/agit/konect/integration/domain/chat/ChatApiTest.java`:
- Around line 161-171: The test currently assumes the new room is at rooms[0],
which is brittle; instead capture the chatRoomId returned by
performPost("/chats/rooms/admin") and use that ID to locate the matching room in
the GET /chats/rooms response before asserting its fields. Concretely: call
performPost("/chats/rooms/admin") and extract the chatRoomId from the response,
then call performGet("/chats/rooms"), find the room whose chatRoomId equals that
extracted ID, and assert that its chatType is "DIRECT", roomName equals
adminUser.getName(), lastMessage does not exist, and isMuted is false (use
performPost, performGet, adminUser, and the chatRoomId value to find the correct
room instead of hardcoding rooms[0]).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c55c8a67-adac-4ddb-80ef-2ac4a001d1f3
📒 Files selected for processing (1)
src/test/java/gg/agit/konect/integration/domain/chat/ChatApiTest.java
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/test/java/gg/agit/konect/integration/domain/chat/ChatApiTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔍 개요
🚀 주요 변경 내용
💬 참고 사항
✅ Checklist (완료 조건)