Fix/#42 test fix #52

mjj111 · 2024-02-12T11:20:48Z

🤨 Motivation

🔑 Key Changes

docs 주석처리
MockBean를 실제 빈을 사용하도록 수정 ( docTest 이외 테스트 코드 소생)
DummyGenerator 추가
SuccessResponse 추가
Entity 객체 연관관계 추가
Enum 으로 request 받도록 변경

🙏 To Reviewers

doc을 제외한 테스트 코드는 모두 살렸습니다.
그러나 doc을 위해 작성된 Controller 테스트들은 해결하지 못하여 주석 처리하였습니다.

doc 에서 사용되는 TokenProvider 를 Mockbean으로 뒀던 문제인지,
혹은 Controller에서 연관된 의존성 간의 충돌인지 정확한 문제를 확인하지 못하고 있는 상황입니다.
(어떠한 테스트가 성공하면 어떠한 테스트는 실패하고 있습니다.)
기능 구현을 다 하고 추후에 해결할 수 있도록 조치하겠습니다.
테스트 코드 소생을 위해 실제 빈을 주입받아 동작하도록 수정했습니다. 이에 따라 이전 Pr 에서 피드백에 있던 내용 몇가지를 수정하게 되었습니다.

이후에 작업은 계속해서 해당 브랜치 받고 작업하시면 됩니다!
의도치 않게 피드백에 대한 코드를 수정하게되고, 많은 시간을 끈점 죄송합니다 🥲🙇‍♂️

…때문에 삭제

…및 의존 객체 나열 스타일 변경

…Generator로 변경

#47

…수정 + 실제 데이터베이스 접근하도록 수정할 예정

…ringboot 애너테이션 사용함에따른 삭제 및 의존 객체 나열 스타일 변경

#50

suhyun0918

테스트 코드에 문제 있다는걸 인지 하시고 2~3일간 리펙토링만 하셨는데 너무 고생하셨습니다 !
모든 테스트가 잘 통과하네요 ㅎㅎ

suhyun0918 · 2024-02-12T13:14:43Z

src/main/java/com/aliens/backend/auth/controller/AuthController.java

말씀하신 대로 수정 해주셨군요 ! 컨벤션으로 활용하도록 하겠습니다

suhyun0918 · 2024-02-12T13:16:27Z

src/main/java/com/aliens/backend/block/domain/Block.java

chatBlock -> Block으로 변경된 부분인가요?
변경해주셔서 감사합니다 ㅎㅎ

suhyun0918 · 2024-02-12T13:23:17Z

src/main/java/com/aliens/backend/chat/controller/ChatController.java

-        ChatSummaryResponse chatSummaryResponse = chatService.getChatSummaries(memberId);
-        Map<String, ChatSummaryResponse> response = Collections.singletonMap("response", chatSummaryResponse);
-        return ResponseEntity.ok(response);
+    public SuccessResponse<ChatSummaryResponse> getChatSummaries(@Login Long memberId) {


(@Login Long memberId) 이 부분
@Login LoginMember loginMember 로 변경되어야 하지 않나요?

테스트 코드가 성공하도록만 피드백 헀던 부분을 최대한 수정하지 않고자 했습니다. (알고 있는데 일단 뒀어요 🥲)
제가 수정한 부분들은 정말 테스트 변경에 필요해서 수정한 부분들입니다🙋‍♂️

suhyun0918 · 2024-02-12T13:27:27Z

src/main/java/com/aliens/backend/chat/domain/ChatReport.java

+    public static ChatReport of(Member reportingMember,
+                                Member reportedMember,
+                                ChatReportCategory category,
+                                String content) {
        ChatReport chatReport = new ChatReport();
-        chatReport.reportedMemberId = chatReportRequest.partnerId();
-        chatReport.reportingMemberId = reportingMemberId;
-        chatReport.category = ChatReportCategory.fromString(chatReportRequest.category())
-                .orElseThrow(() -> new RestApiException(ChatError.INVALID_REPORT_CATEGORY));
-        chatReport.content = chatReportRequest.content();
+        chatReport.reportingMember = reportingMember;
+        chatReport.reportedMember = reportedMember;
+        chatReport.category = category;
+        chatReport.content = content;
        return chatReport;


하나씩 집어 넣어서 완성(?)하는 방법 보단, new 키워드로 생성하는게 좋을 것 같습니다 !

이넘 예외처리를 이넘내에서 관리하고 있는것 같은데 수정 파일에는 안보이네요! 머지 후 변경해놓겠습니다!

suhyun0918 · 2024-02-12T13:30:44Z

src/main/java/com/aliens/backend/chat/domain/ChatRoom.java

+    public static ChatRoom of(final Member me, final Member partner) {
+        ChatRoom chatRoom = new ChatRoom();
+        chatRoom.member = me;
+        chatRoom.partner = partner;
+        return chatRoom;
    }


이 부분도 return new 키워드로 생성하는 것이 좋아보입니다! 코드도 더 간결해질 것 같습니다

머지 후 제가 수정해놓겠습니다!

suhyun0918 · 2024-02-12T13:33:34Z

src/main/java/com/aliens/backend/global/exception/RestApiException.java

에러 시, 로그로 에러 코드를 남기도록 변경해달라는 피드백을 적극 반영해주셨군요 !
감사합니다 ! 😊

suhyun0918 · 2024-02-12T13:34:01Z

src/main/java/com/aliens/backend/global/property/MatchingTimeProperties.java

사용하지 않는 import문 삭제! 저도 주의 하겠습니다

suhyun0918 · 2024-02-12T13:36:15Z

src/main/java/com/aliens/backend/global/response/success/AuthSuccess.java

~SuccessCode -> Success로 네이밍 변경하셨군요 !
확인했습니다

MemberError 가 MemberErrorCode가 아닌 것 처럼 네이밍이 길어지지 않도록,
SuccessCode -> Success로 수정했습니다!

coke98

굳굳~~!! 채팅 부분 리팩토링 이슈도 해결해주셨네요! 덕분에 인증쪽 이슈만 집중해서 신경쓸 수 있겠네요 :) 나머지는 머지 후 추가 수정해놓겠습니다!
테스트쪽은 BaseServiceTest, 제너레이터 쓰기 편하도록 잘 구성해주신게 너무 마음에 들어요.
고생하셨습니다!! 밤샘 작업 김명준... 그는 대체

coke98 · 2024-02-12T13:36:10Z

src/main/java/com/aliens/backend/block/domain/Block.java

+
+    @ManyToOne
+    @JoinColumn(name = "blocking_member_id")
+    private Member blockingMember;


Join 넣어주셨군요!

ㅋㅋㅠ 피드백 드린 내용을 제가 수정해서 죄송합니다..
테스트 코드를 수정해서 돌아가도록 해도, 엔티티가 수정되면 다시 테스트코드를 완전 수정하게되는 상황이 벌어져서..
제가 일단 엔티티에 join. 작업을 수행했습니다 🥲

coke98 · 2024-02-12T13:37:06Z

src/main/java/com/aliens/backend/block/controller/BlockController.java

+    }
+
+    @PostMapping("/chat/block")
+    public SuccessResponse<String> blockPartner(@Login LoginMember loginMember,


멤버도 변환해주셨네요! 감사합니다

수현님께서 작성하신 리뷰에서 처럼, 아직 @Login 에서 Long memberId를 받고 있는 컨트롤러도 있습니다!
최소한으로 수정하고자해서 피드백 받으신 부분들 다시 확인 부탁드리겠습니다 🙇‍♂️

coke98 · 2024-02-12T13:45:54Z

src/test/java/com/aliens/backend/block/service/BlockServiceTest.java

+    @DisplayName("채팅 상대 차단")
+    void blockPartner() {
+        //Given
+        List<Member> members = dummyGenerator.generateMultiMember(2);


coke98 · 2024-02-12T13:48:30Z

src/test/java/com/aliens/backend/chatting/service/ChatServiceTest.java

+class ChatServiceTest extends BaseServiceTest {
+
+    @Autowired ChatService chatService;
+    @SpyBean MessageRepository messageRepository;


Mockbean -> Spybean 👍

coke98 · 2024-02-12T13:52:41Z

src/test/java/com/aliens/backend/global/DummyGenerator.java

+import java.util.Random;
+
+@Component
+public class DummyGenerator {


이거 덕분에 테스트 깔끔해지는게 너무 좋아요. 각 도메인 테스트마다 쉽게 관리할 수 있게 잘 해주셨네요 LGTM!!

Member 같은 경우는 모든 도메인에 연결되어 있어서 차라리 만들어 놓는게 좋겠다 판단했어요!
알아봐주셔서 감사합니다 😉

mjj111 · 2024-02-12T13:59:08Z

테스트 코드에 문제 있다는걸 인지 하시고 2~3일간 리펙토링만 하셨는데 너무 고생하셨습니다 ! 모든 테스트가 잘 통과하네요 ㅎㅎ

일단 doc 제외하고 테스트가 돌아가면서.. 피드백 드린 부분들을 제가 수정하지 않게 하기 위해 최대한 노력했지만
테스트를 다시 수정하지는 않도록 하기 위한 작업으로 몇몇 피드백을 제가 수정하게되 었습니다.. 🥲

MockBean을 이제 사용하지 않기로 컨벤션으로 정하고,
게시판 구현하면서 차차 doc도 해결하도록 하겠습니다!

mjj111 added 30 commits February 10, 2024 13:49

refactor(MockTime): 불필요한 생성자에 private 한정자 삭제

86155c6

feat(SuccessResponse): 반환객체 생성

40c8e24

remove: 반환객체 생성으로 인한 삭제

f8dab73

feat(DummyGenerator) : 더미 데이터를 생성하는 객체 구현

63e7ad4

#50

style(BaseServiceTest) : 더미 데이터 객체 추가로 인한 상수 추가, 및 의존객체 나열 순서 변경

a9791b9

#50

refactor(docTests) : Doc 관련 테스트 코드 주석처리

9a8f4c6

feat(EmailSuccess) : 이메일 관련 성공코드 추가

7c30e0c

refactor(WebSocketTest) : 더미 데이터를 통해 실제로 접근하도록 헤더를 삽입 및 Mock 객체 대체

26e1211

#50

refactor(response) : 결과 객체를 global 디렉토리에서 이동함에 따른 변경

af99187

#44

refactor(time) : 가독성을 위해 time에 대한 영역으로 판단할 수 있도록 time 디렉토리로 네이밍 변경

07c02c8

refactor(MessageSendRequest) : String으로 받던 값을 enum 으로 받도록 수정

068d559

#46

refactor(Message) : enum을 받는 controller 에서 입력 예외를 처리하도록 변경

e6b3244

#46

refactor(MemberDocTest) : Doc 테스트코드 주석처리

74f30dd

refactor(SuccessResponse) : 결과 객체 변경으로 인한 코드 수정

8556edb

#44

refactor(AuthServiceTest) : 더미데이터 생성 객체로 인한 테스트 코드 수정

7217245

#50

style(DatabaseCleanup) : 필요없는 import 문 삭제

86546e5

refactor(ChatReportRequest) : request에서 enum 값을 받도록 수정

30787fb

#46

style(AwsS3UploaderServiceTest) : BaseTest에서 @springboot 애너테이션을 갖고 있기…

ef78c37

…때문에 삭제

refatctor(block) : Chatting 도메인에서 분리하기 위한 클래스 작성 및 삭제

e6466c5

style(MemberInfoServiceTest) : BaseTest 상속함에 따른 @springboot 애너테이션 삭제 …

3c5b56d

…및 의존 객체 나열 스타일 변경

style(MatchingApplicationService) : private 메서드를 사용하는 public 메서드 아래로 이동

1bfb467

refactor(MatchingApplicationServiceTest) : 디렉토리 변경으로인한 수정

9c72755

refactor(MatchingBusinessTest) : MatchingApplicationGenerator 를 Dummy…

779faf3

…Generator로 변경

feat(Member) : Chat 도메인에서 사용되는 get 메서드 추가

21e69ba

style(socket) : 소켓과 관련된 코드를 명시적으로 구분하기 위해 디렉토리 변경

b3521b7

refactor(ChatReport,ChatRoom) : 멤버 객체와 연관관계 추가

4f1058a

#47

refactor(ChatReportService) : 멤버 객체와 연관관계 추가로 인한 코드 수정

cacd90b

#47

refactor(ChatReportServiceTest) : 실제 멤버를 사용하도록 수정 및 BaseTest 상속 추가

0a1bc41

refactor(ChatServiceTest) : messageRepository를 MockBean으로 사용하지 않기 위해 …

28f1d09

…수정 + 실제 데이터베이스 접근하도록 수정할 예정

refactor(EmailSenderTest, EventPublisherTest) : BaseServiceTest에서 @sp…

905fd25

…ringboot 애너테이션 사용함에따른 삭제 및 의존 객체 나열 스타일 변경

mjj111 added 6 commits February 12, 2024 19:56

remove(MatchingApplicationGenerator) : DummyGenerator 객체 생성에 따른 삭제

b5db37b

#50

remove(MatchingApplicationApiTest) : 단일로 실행했을 때 동작할 수 있도록 새로 코드 작성

9417cf3

style(BlockServiceTest) : 필요없는 네이밍 변경

288f7b4

style(AuthServiceTest, EmailServiceTest) : 필요없는 네이밍 변경

c703663

style(AwsS3UploaderTest, EmailSenderTest) : 필요없는 네이밍 변경

8ff4c13

refactor(interceptor) : config 디렉토리로 이동

aebc863

mjj111 added feat 기능 개발 이슈 style 코드 스타일 관련 태그 test 테스트 관련 태그 labels Feb 12, 2024

mjj111 requested review from coke98 and suhyun0918 February 12, 2024 11:20

mjj111 self-assigned this Feb 12, 2024

mjj111 and others added 2 commits February 12, 2024 21:04

chore : fcm 관련 설정 추가

c7ce85f

chore : Git Actions에 JSON 사용불가 이슈 해결

b20c336

suhyun0918 approved these changes Feb 12, 2024

View reviewed changes

coke98 approved these changes Feb 12, 2024

View reviewed changes

mjj111 merged commit aca32a1 into develop Feb 12, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix/#42 test fix #52

Fix/#42 test fix #52

mjj111 commented Feb 12, 2024

suhyun0918 left a comment

suhyun0918 Feb 12, 2024

suhyun0918 Feb 12, 2024

suhyun0918 Feb 12, 2024

mjj111 Feb 12, 2024

suhyun0918 Feb 12, 2024

coke98 Feb 12, 2024

suhyun0918 Feb 12, 2024

coke98 Feb 12, 2024

suhyun0918 Feb 12, 2024

suhyun0918 Feb 12, 2024

suhyun0918 Feb 12, 2024

mjj111 Feb 12, 2024

coke98 left a comment

coke98 Feb 12, 2024

mjj111 Feb 12, 2024

coke98 Feb 12, 2024

mjj111 Feb 12, 2024

coke98 Feb 12, 2024

coke98 Feb 12, 2024

coke98 Feb 12, 2024

mjj111 Feb 12, 2024

mjj111 commented Feb 12, 2024

Fix/#42 test fix #52

Fix/#42 test fix #52

Conversation

mjj111 commented Feb 12, 2024

🤨 Motivation

🔑 Key Changes

🙏 To Reviewers

suhyun0918 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coke98 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjj111 commented Feb 12, 2024