Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
import com.example.solidconnection.chat.domain.ChatReadStatus;
import com.example.solidconnection.chat.domain.ChatRoom;
import com.example.solidconnection.chat.dto.ChatMessageResponse;
import com.example.solidconnection.chat.dto.ChatMessageSendRequest;
import com.example.solidconnection.chat.dto.ChatMessageSendResponse;
import com.example.solidconnection.chat.dto.ChatRoomListResponse;
import com.example.solidconnection.chat.fixture.ChatAttachmentFixture;
import com.example.solidconnection.chat.fixture.ChatMessageFixture;
Expand All @@ -29,10 +31,14 @@
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.BDDMockito;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.data.domain.PageRequest;
import org.springframework.data.domain.Pageable;
import org.springframework.data.domain.Sort;
import org.springframework.messaging.simp.SimpMessagingTemplate;

@TestContainerSpringBootTest
@DisplayName("채팅 서비스 테스트")
Expand Down Expand Up @@ -62,6 +68,9 @@ class ChatServiceTest {
@Autowired
private ChatAttachmentFixture chatAttachmentFixture;

@MockBean
private SimpMessagingTemplate simpMessagingTemplate;

private SiteUser user;
private SiteUser mentor1;
private SiteUser mentor2;
Expand Down Expand Up @@ -363,4 +372,53 @@ void setUp() {
.hasMessage(CHAT_PARTICIPANT_NOT_FOUND.getMessage());
}
}

@Nested
class 채팅_메시지를_전송한다 {

private SiteUser sender;
private ChatParticipant senderParticipant;
private ChatRoom chatRoom;

@BeforeEach
void setUp() {
sender = siteUserFixture.사용자(111, "sender");
chatRoom = chatRoomFixture.채팅방(false);
senderParticipant = chatParticipantFixture.참여자(sender.getId(), chatRoom);
}

@Test
void 채팅방_참여자는_메시지를_전송할_수_있다() {
// given
final String content = "안녕하세요";
ChatMessageSendRequest request = new ChatMessageSendRequest(content);

// when
chatService.sendChatMessage(request, sender.getId(), chatRoom.getId());

// then
ArgumentCaptor<String> destinationCaptor = ArgumentCaptor.forClass(String.class);
ArgumentCaptor<ChatMessageSendResponse> payloadCaptor = ArgumentCaptor.forClass(ChatMessageSendResponse.class);

BDDMockito.verify(simpMessagingTemplate).convertAndSend(destinationCaptor.capture(), payloadCaptor.capture());

assertAll(
() -> assertThat(destinationCaptor.getValue()).isEqualTo("/topic/chat/" + chatRoom.getId()),
() -> assertThat(payloadCaptor.getValue().content()).isEqualTo(content),
() -> assertThat(payloadCaptor.getValue().senderId()).isEqualTo(senderParticipant.getId())
);
Comment on lines +399 to +409
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

검증을 인자 기반(BDD then + eq)으로 바꿔, 호출 횟수/추가 호출에 둔감하게 만듭니다.

  1. 현재 captor로 destination을 직접 캡처하면, convertAndSend가 추가로 더 호출되는 구조로 변경될 때 테스트가 불필요하게 깨질 수 있습니다.
  2. 목적지(destination)는 eq로 검증하고, payload만 captor로 캡처해 필드 단언으로 이어지게 하면 테스트 탄성이 좋아집니다.

아래처럼 수정 제안드립니다.

-            ArgumentCaptor<String> destinationCaptor = ArgumentCaptor.forClass(String.class);
-            ArgumentCaptor<ChatMessageSendResponse> payloadCaptor = ArgumentCaptor.forClass(ChatMessageSendResponse.class);
-
-            BDDMockito.verify(simpMessagingTemplate).convertAndSend(destinationCaptor.capture(), payloadCaptor.capture());
-
-            assertAll(
-                    () -> assertThat(destinationCaptor.getValue()).isEqualTo("/topic/chat/" + chatRoom.getId()),
-                    () -> assertThat(payloadCaptor.getValue().content()).isEqualTo(content),
-                    () -> assertThat(payloadCaptor.getValue().senderId()).isEqualTo(senderParticipant.getId())
-            );
+            ArgumentCaptor<ChatMessageSendResponse> payloadCaptor = ArgumentCaptor.forClass(ChatMessageSendResponse.class);
+            BDDMockito.then(simpMessagingTemplate)
+                    .should()
+                    .convertAndSend(
+                            org.mockito.ArgumentMatchers.eq("/topic/chat/" + chatRoom.getId()),
+                            payloadCaptor.capture()
+                    );
+            assertAll(
+                    () -> assertThat(payloadCaptor.getValue().content()).isEqualTo(content),
+                    () -> assertThat(payloadCaptor.getValue().senderId()).isEqualTo(senderParticipant.getId())
+            );

추가로, eq·any를 자주 쓰신다면 ArgumentMatchers에 대한 static import를 고려해 주세요.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// then
ArgumentCaptor<String> destinationCaptor = ArgumentCaptor.forClass(String.class);
ArgumentCaptor<ChatMessageSendResponse> payloadCaptor = ArgumentCaptor.forClass(ChatMessageSendResponse.class);
BDDMockito.verify(simpMessagingTemplate).convertAndSend(destinationCaptor.capture(), payloadCaptor.capture());
assertAll(
() -> assertThat(destinationCaptor.getValue()).isEqualTo("/topic/chat/" + chatRoom.getId()),
() -> assertThat(payloadCaptor.getValue().content()).isEqualTo(content),
() -> assertThat(payloadCaptor.getValue().senderId()).isEqualTo(senderParticipant.getId())
);
// then
ArgumentCaptor<ChatMessageSendResponse> payloadCaptor = ArgumentCaptor.forClass(ChatMessageSendResponse.class);
BDDMockito.then(simpMessagingTemplate)
.should()
.convertAndSend(
org.mockito.ArgumentMatchers.eq("/topic/chat/" + chatRoom.getId()),
payloadCaptor.capture()
);
assertAll(
() -> assertThat(payloadCaptor.getValue().content()).isEqualTo(content),
() -> assertThat(payloadCaptor.getValue().senderId()).isEqualTo(senderParticipant.getId())
);
🤖 Prompt for AI Agents
In src/test/java/com/example/solidconnection/chat/service/ChatServiceTest.java
around lines 399 to 409, change the verification so the destination is verified
with an eq matcher (eg. eq("/topic/chat/" + chatRoom.getId())) instead of
capturing it, and only capture the payload with
ArgumentCaptor<ChatMessageSendResponse>; update the verify/then call to use the
matcher for the first argument and payloadCaptor.capture() for the second so
extra convertAndSend calls won't break the test, and consider adding a static
import for ArgumentMatchers.eq (and any) to keep the test concise.

}

@Test
void 채팅_참여자가_아니면_메시지를_전송할_수_없다() {
// given
SiteUser nonParticipant = siteUserFixture.사용자(333, "nonParticipant");
ChatMessageSendRequest request = new ChatMessageSendRequest("안녕하세요");

// when & then
assertThatCode(() -> chatService.sendChatMessage(request, nonParticipant.getId(), chatRoom.getId()))
.isInstanceOf(CustomException.class)
.hasMessage(CHAT_PARTICIPANT_NOT_FOUND.getMessage());
}
Comment on lines +418 to +422
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

실패 케이스에서 메시지 브로커 호출이 전혀 없음을 보장하세요.

  1. 예외 단언 뒤에 no-interaction 검증을 추가하면, 회귀로 인한 오발신을 조기에 잡을 수 있습니다.
             assertThatCode(() -> chatService.sendChatMessage(request, nonParticipant.getId(), chatRoom.getId()))
                     .isInstanceOf(CustomException.class)
                     .hasMessage(CHAT_PARTICIPANT_NOT_FOUND.getMessage());
+            org.mockito.Mockito.verifyNoInteractions(simpMessagingTemplate);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// when & then
assertThatCode(() -> chatService.sendChatMessage(request, nonParticipant.getId(), chatRoom.getId()))
.isInstanceOf(CustomException.class)
.hasMessage(CHAT_PARTICIPANT_NOT_FOUND.getMessage());
}
// when & then
assertThatCode(() -> chatService.sendChatMessage(request, nonParticipant.getId(), chatRoom.getId()))
.isInstanceOf(CustomException.class)
.hasMessage(CHAT_PARTICIPANT_NOT_FOUND.getMessage());
org.mockito.Mockito.verifyNoInteractions(simpMessagingTemplate);
}
🤖 Prompt for AI Agents
In src/test/java/com/example/solidconnection/chat/service/ChatServiceTest.java
around lines 418 to 422, the failure case asserts the CustomException but does
not verify that the message broker was not called; add a no-interaction
verification for the mocked message broker (e.g., verifyNoInteractions or
verify(messageBroker, never()) semantics) immediately after the exception
assertion to ensure no messages were sent on failure and prevent
regression-induced stray calls.

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
package com.example.solidconnection.websocket;

import static java.util.concurrent.TimeUnit.SECONDS;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.ThrowableAssert.catchThrowable;
import static org.junit.jupiter.api.Assertions.assertAll;

import com.example.solidconnection.auth.service.AccessToken;
import com.example.solidconnection.auth.service.AuthTokenProvider;
import com.example.solidconnection.siteuser.domain.SiteUser;
import com.example.solidconnection.siteuser.fixture.SiteUserFixture;
import com.example.solidconnection.support.TestContainerSpringBootTest;
import java.util.List;
import java.util.concurrent.ArrayBlockingQueue;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.ExecutionException;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.web.server.LocalServerPort;
import org.springframework.messaging.converter.MappingJackson2MessageConverter;
import org.springframework.messaging.simp.stomp.StompHeaders;
import org.springframework.messaging.simp.stomp.StompSession;
import org.springframework.messaging.simp.stomp.StompSessionHandlerAdapter;
import org.springframework.web.client.HttpClientErrorException;
import org.springframework.web.socket.WebSocketHttpHeaders;
import org.springframework.web.socket.client.standard.StandardWebSocketClient;
import org.springframework.web.socket.messaging.WebSocketStompClient;
import org.springframework.web.socket.sockjs.client.SockJsClient;
import org.springframework.web.socket.sockjs.client.Transport;
import org.springframework.web.socket.sockjs.client.WebSocketTransport;

@TestContainerSpringBootTest
@DisplayName("WebSocket/STOMP 통합 테스트")
class WebSocketStompIntegrationTest {

@LocalServerPort
private int port;
private String url;
private WebSocketStompClient stompClient;
private StompSession stompSession;

@Autowired
private AuthTokenProvider authTokenProvider;

@Autowired
private SiteUserFixture siteUserFixture;

@BeforeEach
void setUp() {
this.url = String.format("ws://localhost:%d/connect", port);
List<Transport> transports = List.of(new WebSocketTransport(new StandardWebSocketClient()));
this.stompClient = new WebSocketStompClient(new SockJsClient(transports));
this.stompClient.setMessageConverter(new MappingJackson2MessageConverter());
}

@AfterEach
void tearDown() {
if (this.stompSession != null && this.stompSession.isConnected()) {
this.stompSession.disconnect();
}
}
Comment on lines +60 to +65
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

리소스 정리: 테스트 종료 시 stompClient.stop() 호출을 권장합니다.

  1. 내부 스케줄러/스레드가 남아 다른 테스트에 간섭하는 일을 예방합니다.
     void tearDown() {
         if (this.stompSession != null && this.stompSession.isConnected()) {
             this.stompSession.disconnect();
         }
+        if (this.stompClient != null) {
+            this.stompClient.stop();
+        }
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@AfterEach
void tearDown() {
if (this.stompSession != null && this.stompSession.isConnected()) {
this.stompSession.disconnect();
}
}
@AfterEach
void tearDown() {
if (this.stompSession != null && this.stompSession.isConnected()) {
this.stompSession.disconnect();
}
if (this.stompClient != null) {
this.stompClient.stop();
}
}
🤖 Prompt for AI Agents
In
src/test/java/com/example/solidconnection/websocket/WebSocketStompIntegrationTest.java
around lines 60 to 65, the teardown currently only disconnects the stompSession
but does not stop the underlying stompClient, which can leave scheduler/threads
running and interfere with other tests; update tearDown to also call
stompClient.stop() (with a null-check on stompClient and optionally wrapped in
try/catch to avoid masking test failures) after disconnecting the session so the
client's threads are cleaned up between tests.


@Nested
class WebSocket_핸드셰이크_및_STOMP_세션_수립_테스트 {

private final BlockingQueue<Throwable> transportErrorQueue = new ArrayBlockingQueue<>(1);

private final StompSessionHandlerAdapter sessionHandler = new StompSessionHandlerAdapter() {
@Override
public void handleTransportError(StompSession session, Throwable exception) {
transportErrorQueue.add(exception);
}
};

@Test
void 인증된_사용자는_핸드셰이크를_성공한다() throws Exception {
// given
SiteUser user = siteUserFixture.사용자();
AccessToken accessToken = authTokenProvider.generateAccessToken(authTokenProvider.toSubject(user), user.getRole());

WebSocketHttpHeaders handshakeHeaders = new WebSocketHttpHeaders();
handshakeHeaders.add("Authorization", "Bearer " + accessToken.token());

// when
stompSession = stompClient.connectAsync(url, handshakeHeaders, new StompHeaders(), sessionHandler).get(5, SECONDS);

// then
assertAll(
() -> assertThat(stompSession).isNotNull(),
() -> assertThat(transportErrorQueue).isEmpty()
);
}

@Test
void 인증되지_않은_사용자는_핸드셰이크를_실패한다() {
// when
Throwable thrown = catchThrowable(() -> {
stompSession = stompClient.connectAsync(url, new WebSocketHttpHeaders(), new StompHeaders(), sessionHandler).get(5, SECONDS);
});

// then
assertAll(
() -> assertThat(thrown)
.isInstanceOf(ExecutionException.class)
.hasCauseInstanceOf(HttpClientErrorException.Unauthorized.class),
() -> assertThat(transportErrorQueue).hasSize(1)
);
}
Comment on lines +106 to +112
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

인증 실패 시 transportErrorQueue 크기 단언을 완화하세요.

  1. 전송 계층에서 에러가 발생하지 않고 즉시 HTTP 예외로 반환될 수 있어, 큐 크기 1을 정확히 보장하기 어렵습니다.
  2. 비어있지 않음 정도로만 검증하면 환경차(컨테이너/드라이버)에 덜 민감합니다.
-                    () -> assertThat(transportErrorQueue).hasSize(1)
+                    () -> assertThat(transportErrorQueue).isNotEmpty()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
assertAll(
() -> assertThat(thrown)
.isInstanceOf(ExecutionException.class)
.hasCauseInstanceOf(HttpClientErrorException.Unauthorized.class),
() -> assertThat(transportErrorQueue).hasSize(1)
);
}
assertAll(
() -> assertThat(thrown)
.isInstanceOf(ExecutionException.class)
.hasCauseInstanceOf(HttpClientErrorException.Unauthorized.class),
() -> assertThat(transportErrorQueue).isNotEmpty()
);
🤖 Prompt for AI Agents
In
src/test/java/com/example/solidconnection/websocket/WebSocketStompIntegrationTest.java
around lines 106 to 112, the test currently asserts transportErrorQueue
hasSize(1) which is brittle because authentication failures can be returned
immediately as HTTP exceptions without a transport-layer error; change the
assertion to check the queue is not empty (e.g.,
assertThat(transportErrorQueue).isNotEmpty() or hasSizeGreaterThan(0)) so the
test passes when at least one transport error was recorded while remaining
tolerant of environment differences; update the assertAll accordingly to use the
relaxed assertion.

}
}
Loading