refactor: 메시지 조회 테스트 리팩토링 #421

hyewoncc · 2022-08-15T08:39:36Z

요약

메시지 조회 서비스 통합 테스트 리팩토링

작업 내용

MessageServiceTest를 아래와 같은 목적으로 리팩토링 했습니다

@Sql 선언된 sql문 내부를 보지 않아도 되도록 (=픽스쳐를 자바 코드 상에서 볼 수 있도록) 합니다
MessageRequest를 생성할 때 팩토리 클래스의 메서드 이름을 통해 어떤 요청인지 읽기 쉽도록 합니다

ParameterizedTest는 아래와 같은 이유로 해제했어요

왜 테스트가 복잡해보일까? 를 생각했을 때 여러 조건을 한 메서드에서 검증하려고 하기 때문 이라고, 조언을 듣고 생각했습니다
그래서 조건을 광범위 -> 작은범위 로 흘러가도록 쪼개서 테스트하면 어떨까? 했어요
- 예를 들면, 20개를 조회했을 때 20개가 나온다
- 더 조회할 게 없으면 isLast가 true다
- 파라미터가 없으면~
- 특정 파라미터가 있으면~

보기에 따라서 개악으로 느껴질 수도 있을 것 같아서 테스트에 대해 얘기 더 나눠봐도 좋을 것 같아요
또, 앞선 PR 커밋을 같이 올려놔서 의논/approve와 별개로 머지는 제가 할게요!

github-actions · 2022-08-15T08:41:27Z

Unit Test Results

  47 files   47 suites 19s ⏱️
184 tests 184 ✔️ 0 💤 0 ❌
187 runs 187 ✔️ 0 💤 0 ❌

Results for commit 3839c85.

♻️ This comment has been updated with latest results.

hyewoncc · 2022-08-16T12:54:44Z

변수 명 변경, Factory 클래스 삭제 등 피드백 반영했습니다

그리고 같은 테스트를 @Sql 사용하지 않은 것과 사용한 것으로 나눠봤어요
데이터 셋업이 길어지긴 해서요
원래 테스트 리팩토링을 하면서 하고 싶었던 주 안점은 두가지입니다

쿼리 파라미터 생성에 파악 가능한 이름을 준다
조회된 메시지의 특징을 sql문을 보지 않아도 파악되게 한다 (ex. isPastMessages...).

저는 2번의 이유로 여전히 길어지더라도, 그럼 클래스를 분리하는 게 낫다 파입니다 (ex. MessageServiceSearchTest, ...)
하지만 @Sql로 셋업이 줄어드는 것도 동의를 하고, 테스크에 시간을 오래 끄는 것 같아서 팀원분들의 결정에 맡길게요 😅

HJ-Rich

고생 많으셨습니다.
회의 때도 많이 나눴지만 개인적인 의견은 코멘트로 다시 정리해봤습니다.

더 읽기 쉬운 테스트 코드는 어떤 형태인가
더 유지보수 하기 쉬운 테스트 코드는 어떤 형태인가
둘이 상충된다면 어디에 더 가치를 둘 것인가 등
여러 가지 고민을 할 수 있는 좋은 기회였던 것 같습니다.

고생 많으셨습니다.
어푸하겠습니다.

HJ-Rich · 2022-08-16T16:15:39Z

backend/src/main/java/com/pickpick/message/domain/MessageRepository.java

@@ -8,6 +8,8 @@ public interface MessageRepository extends Repository<Message, Long> {

    Message save(Message message);

+    List<Message> saveAll(Iterable<Message> messages);


테스트에서만 사용되는 메서드인 것 같아요

삭제했습니다~

HJ-Rich · 2022-08-16T16:17:15Z

backend/src/test/java/com/pickpick/fixture/MessageFixtures.java

+    }
+
+    public Message create(final Channel channel, final Member member) {
+        return new Message(UUID.randomUUID().toString(), this.text, member, channel, this.dateTime, this.dateTime);


UUID.randomUUID().toString() 이 방법은 정말 좋은 것 같아요

리뷰는 아니고 그냥 찾아봤는데 슬랙에서 메시지 아이디를 만들 때도 uuid로 만드는 것 같네요 규칙이 같아요!

@JangBomi 봄 최고..😀

HJ-Rich · 2022-08-16T16:21:38Z

backend/src/test/java/com/pickpick/message/application/MessageServiceQueryTest.java

+    @Autowired
+    private MessageService messageService;
+
+    @DisplayName("쿼리 파라미터와 count가 없다면, 회원의 첫번째 순서 구독 채널의 최신 메시지를 20개 내림차순 조회한다")


Suggested change

@DisplayName("쿼리 파라미터와 count가 없다면, 회원의 첫번째 순서 구독 채널의 최신 메시지를 20개 내림차순 조회한다")

@DisplayName("count만 전달됐다면, 회원의 첫번째 순서 구독 채널의 최신 메시지 20개를 작성시간 내림차순 조회한다")

내림차순의 기준이 명시되어 있지 않아 제안해봅니다!

count는 쿼리 파라미터와는 다르다고 생각하신걸까요 ?-?
저는 count도 쿼리 파라미터도 포함된다고 생각했는데 별도로 명시하셨기에 궁금해서 여쭤봅니다!

😅 동일한 쿼리 파람인데 나머지는 필터링/검색의 조건, count는 별도로 자를 개수... 라는 생각이 깔려있어서 이렇게 적은 것 같네요
수정했어요~

HJ-Rich · 2022-08-16T16:26:51Z

backend/src/test/java/com/pickpick/message/application/MessageServiceQueryTest.java

+        // given
+        Member summer = members.save(summer());
+        Channel notice = channels.save(notice());
+        messages.saveAll(createMessages(notice, summer));
+
+        Channel freeChat = channels.save(freeChat());
+        List<Message> messagesInFreeChat = messages.saveAll(createMessages(freeChat, summer));
+
+        subscriptions.save(new ChannelSubscription(freeChat, summer, VIEW_ORDER_FIRST));
+        subscriptions.save(new ChannelSubscription(notice, summer, VIEW_ORDER_SECOND));
+
+        MessageRequest request = emptyQueryParams();


물론 지속적으로 노력해볼 순 있겠지만,
한편으론 인정을 해야할 것 같기도 해요.
실제로 복잡한 로직이기 때문에, 복잡하지 않을 순 없는 것 같아요.
아무래도 이번 스프린트에서는 회의 때 이야기한 트레이드 오프에서 결정을 해야한다는 생각입니다.
그래서 저도 어느 방향으로 진행이 되도 괜찮은데 개인적인 의견은 남겨봅니다.

개인적으론 개별 케이스마다 given이 정말 많아졌다는 점에서
PR로 제시해주신 방법의 단점이 더 크지 않나 생각됩니다.

ParameterizedTest를 사용할 경우,
하나의 케이스에 대해 이해하고 나면
그 다음 케이스들에 대해서는 쉽게 파악이 가능하다고 생각해요.
모든 경우의 수를 한 곳에서 테스트하기 때문에
추후 테스트 케이스가 늘어나거나 수정해야하는 경우에도
오히려 한 곳에만 집중하면 되니 더 수월한 점이 있다고 생각해요.

한 곳에서 몰아서 테스트할 경우,
봄이 제시해준 의견처럼 리팩터링 하기 어렵다고 느껴지는 점도 공감이 가는 지점이 있지만,
이처럼 로직들이 분산되게 되면, 이들이 결국 관리가 어려워질 거라는 생각이에요.
규모가 커질수록 어디에서 어디까지 테스트를 하고 있는지 파악하기 어려워질 것 같다는 생각이에요.
어찌보면 하나의 로직을 테스트하는 것인데, 응집도가 떨어지게 되는 것 같아요.

회의 때 했던 이야기의 반복이긴 하지만,
저는 서비스 레이어의 테스트가 가장 엄격해야 한다고 생각해요.

그런 관점에서 같은 데이터를 쌓아두고
여러가지 요청을 보냈을 때 어떻게 결과가 나오는지 보는 게 중요하다고 생각해요.
하나의 로직을 테스트하는 건데
지금은 개별 케이스마다 데이터를 다르게 세팅하는 모습이라서
개인적인 취향상 덜 엄격해서 아쉽다는 생각이에요.
보다 실제와 유사한 환경에서 테스트를 하고 검증하고 싶다는 생각입니다.

저는 해당 테스트 케이스에서 ParamterizedTest를 사용하는건 반대하는 입장인데요
하나의 케이스에 대해 이해하고 나면 그 다음 케이스들에 대해 쉽게 파악이 가능하지만
단 하나의 테스트 케이스를 이해하는 것 자체가 너무너무 어렵고 복잡하게 느껴져요
저는 제가 짰던 코드임에도 간만에 다시 보니 알아보는게 엄청 힘들더라고요😅

하나의 로직에서 다양한 값들을 포함하고 있으니까 이를 하나하나 테스트할 필요가 있다고 느껴져요🤔
저는 하나하나 테스트해본 케이스 n개와 모든 케이스를 테스트한 케이스 1개 이런 식으로 테스트하는게 오히려 더 엄격하다고 테스트하는 것이라고 생각이 드네요

하나씩 하는 것도 좋은 방법이라고 생각해요
그런데 몇 가지 전제가 필요하다고 개인적으로 생각합니다

하나의 로직에 대해 여러 테스트 케이스가 검증되어야할 경우 이 케이스들을 체계적으로 정리해야 합니다

여러 클래스에 분산해서 관리한다는 의견도 나왔었기에 명시해봅니다!

분리하는 것도 방법일 수 있지만, 향후 추가되거나 수정될때, 개발자들이 하나의 로직들에 대한 검증을 어디에서 어떤 케이스들을 하고 있는 지에 대해 파악할 수 있는 수단이 필요합니다.

각 케이스들은 최대한 실제 환경과 동일한 데이터를 기준으로 테스트해야 합니다

저희의 경우 isLast 등의 이슈가 실제 존재했기에, 실제 사용환경과 최대한 비슷하게 꼼꼼히 검증해야 한다고 생각해요

가급적 여러 케이스들이 하나의 동일한 데이터 셋을 기준으로 검증되어야 한다고 생각합니다

변인 통제 차원에서 필요성이 있다고생각합니다.

같은 로직이고, 같은 데이터가 주어져있을 때, 요청에 따라 응답이 달라지는 부분에 대해 검증해야 하기 떄문입니다.

HJ-Rich · 2022-08-16T16:36:19Z

backend/src/test/java/com/pickpick/message/application/MessageServiceQueryTest.java

+
+        // then
+        List<MessageResponse> foundMessages = response.getMessages();
+        boolean isContainingKeyword = foundMessages.stream()


isContainingKeyword 를 containsKeyword 로 표현하면 어떨까요?
Collections 클래스에도 boolean을 반환하는 containsAll이 있어서 제안해봅니다

HJ-Rich · 2022-08-16T16:46:52Z

backend/src/test/java/com/pickpick/message/application/MessageServiceQueryTestSql.java

+    @DisplayName("쿼리 파라미터와 count가 없다면, 회원의 첫번째 순서 구독 채널의 최신 메시지를 20개 내림차순 조회한다")
+    @Test
+    void findMessagesEmptyParameters() {
+        // given
+        MessageRequest request = emptyQueryParams();
+
+        // when
+        MessageResponses response = messageService.find(MEMBER_ID, request);
+
+        // then
+        List<MessageResponse> foundMessages = response.getMessages();
+        List<Long> expectedIds = createExpectedMessageIds(20L, 1L);
+
+        assertAll(
+                () -> assertThat(foundMessages).extracting("id").isEqualTo(expectedIds),
+                () -> assertThat(foundMessages).hasSize(MESSAGE_COUNT_DEFAULT)
+        );
+    }


이 정도 깔끔하면 좋긴 하겠지만..
이런 형태를 보고나면 완전히 중복되는 코드여서 ParameterizedTest로 빼고 싶어지는...
반복이군요 ㅋㅋㅋ

HJ-Rich · 2022-08-16T16:48:28Z

backend/src/test/java/com/pickpick/message/application/MessageServiceTest.java

-                Arguments.of(
-                        "5번 채널에서 메시지ID가 1인 메시지 이후에 작성된 메시지 7개 조회",
-                        new MessageRequest("", "", List.of(5L), false, 1L, 7),
-                        createExpectedMessageIds(8L, 2L),
-                        false),


테스트하려는 내용, 매개변수, 응답값이 모두 한 눈에 파악된다는 점에서
저는 ParameterizedTest가 좋아보입니다.
물론 지적해주신 이 방식의 단점도 충분이 공감이 갑니다.

그러나 보다 실제와 유사한 테스트 환경, 엄격한 테스트,
하나의 로직에 대한 테스트를 한 곳에서 응집도 있게 관리한다는 점에서 매력을 느끼는 것 같습니다.

yeon-06

안녕하세요 써머 ㅎㅎ
리팩토링 정말정말정말 고생 많으셨습니다!!
몇 가지 남긴 질문에 대해 답변을 확인하고 싶어서 approve 대신 request changes로 해둘게요!❤

yeon-06 · 2022-08-17T04:59:01Z

backend/src/main/java/com/pickpick/message/domain/MessageRepository.java

@@ -8,6 +8,8 @@ public interface MessageRepository extends Repository<Message, Long> {

    Message save(Message message);

+    List<Message> saveAll(Iterable<Message> messages);


저도 테스트에서만 사용하는 메서드는 없으면 좋을 것 같아요~
for문 통해서 save를 하던가 하는 식이 될 것 같은데...
성능 개선을 위해 saveAll을 사용하고 싶다면 제가 말씀드렸던 SupportRepository같은 클래스를 만드는건 어떨지 다른 방법도 있을지 고민되네요 🤔

for문으로 save하도록 변경하고 삭제했습니다~

yeon-06 · 2022-08-17T05:09:05Z

backend/src/test/java/com/pickpick/message/application/MessageServiceQueryTest.java

+        // given
+        Member summer = members.save(summer());
+        Channel notice = channels.save(notice());
+        messages.saveAll(createMessages(notice, summer));
+
+        Channel freeChat = channels.save(freeChat());
+        List<Message> messagesInFreeChat = messages.saveAll(createMessages(freeChat, summer));
+
+        subscriptions.save(new ChannelSubscription(freeChat, summer, VIEW_ORDER_FIRST));
+        subscriptions.save(new ChannelSubscription(notice, summer, VIEW_ORDER_SECOND));
+
+        MessageRequest request = emptyQueryParams();


저는 해당 테스트 케이스에서 ParamterizedTest를 사용하는건 반대하는 입장인데요
하나의 케이스에 대해 이해하고 나면 그 다음 케이스들에 대해 쉽게 파악이 가능하지만
단 하나의 테스트 케이스를 이해하는 것 자체가 너무너무 어렵고 복잡하게 느껴져요
저는 제가 짰던 코드임에도 간만에 다시 보니 알아보는게 엄청 힘들더라고요😅

하나의 로직에서 다양한 값들을 포함하고 있으니까 이를 하나하나 테스트할 필요가 있다고 느껴져요🤔
저는 하나하나 테스트해본 케이스 n개와 모든 케이스를 테스트한 케이스 1개 이런 식으로 테스트하는게 오히려 더 엄격하다고 테스트하는 것이라고 생각이 드네요

yeon-06 · 2022-08-17T05:11:57Z

backend/src/test/java/com/pickpick/message/application/MessageServiceQueryTest.java

+        messages.save(MessageFixtures.KEYWORD_20220714_14_00_00.create(notice, summer));
+        messages.save(MessageFixtures.KEYWORD_20220714_14_00_00.create(freeChat, summer));
+
+        MessageRequest request = searchByKeywordInChannels(List.of(notice, freeChat), MESSAGE_KEYWORD, MESSAGE_COUNT);


MESSAGE_KEYWORD는 해당 메서드에서만 사용되고 있는 것 같아요
상수처리 하지 않아도 될 것 같아서 말씀 드립니다😄

저는 매직 넘버 등의 상수화가 중복 제거 보다는
의미있는 이름을 제공하는 데 더 의의가 있다고 생각해서
한 번만 사용되더라도 상수 처리를 선호합니다!

yeon-06 · 2022-08-17T05:26:07Z

backend/src/test/java/com/pickpick/message/application/MessageServiceQueryTest.java

+@Sql({"/truncate.sql"})
+@Transactional
+@SpringBootTest
+class MessageServiceQueryTest {


네이밍에 대해서 고민을 해보면 좋을 것 같아요
MessageServiceQueryTest 라고 지으신 이유가 있을까요?-?

내부 구현이 QueryDSL을 사용하긴 했지만 이름만으로는 RepositoryTest 같은건가?
쿼리문이 잘 실행되는걸 테스트하나? 라는 혼돈이 좀 올 수 있는 것 같아요!

MessageServiceTest에서 따로 분리하신 이유가 있을까요..?!

추가로 테스트케이스가 추가되면 좋을 것 같아요 😄
아마 지금 develop 브랜치에 MessageServiceTest 파일에 변경사항이 또 있어서 확인해주셔야할 것 같아요🙏

쿼리 파라미터와 count가 없다면, 회원의 첫번째 순서 구독 채널의 최신 메시지를 20개 내림차순 조회한다

쿼리 파라미터가 없고 count가 있다면, 회원의 첫번째 순서 구독 채널의 최신 메시지를 count개 내림차순 조회한다

더 이상 조회할 메시지가 없다면 isLast로 true를 반환한다

조회할 메시지가 더 있다면 isLast로 false를 반환한다

메시지 조회 시, 텍스트가 비어있는 메시지는 필터링된다

여러 채널의 특정 키워드로 조회 시, 해당 키워드가 존재하는 메시지만 조회된다

하나의 채널로 조회 시, 해당 채널에 존재하는 메시지만 조회된다

여러 채널로 조회 시, 해당 채널에 존재하는 메시지만 조회된다

채널과 메시지ID로 앞선 메시지 조회 시, 메시지 기준 해당 채널의 미래 메시지를 시간 내림차순 조회한다

채널과 메시지ID로 지난 메시지 조회 시, 메시지 기준 해당 채널의 과거 메시지를 시간 내림차순 조회한다

리마인더 등록된 메시지 조회 시, isReminder가 true이고 remindDate 값이 조회된다

리마인더 등록된 메시지 조회 시, isReminder가 false이고 remindDate가 null이다

북마크 등록된 메시지 조회 시, isBookmarked가 true이다

북마크 등록되지 않은 메시지 조회 시, isBookmarked가 false이다

저도 MessageServiceQueryTest말고 다른 이름이었으면 좋겠어요!

보면서 비교하기 편하라고 임시로 분리해뒀습니다 ㅎㅎ MessageServiceTest로 올릴게요

yeon-06 · 2022-08-17T05:41:34Z

backend/src/test/java/com/pickpick/message/application/MessageServiceQueryTestSql.java

+@Sql({"/truncate.sql", "/message_fixture.sql"})
+@Transactional
+@SpringBootTest
+class MessageServiceQueryTestSql {


동일한 코드 같은데 왜 둘로 나눴나했는데
MessageServiceQueryTestSql이 @Sql을 사용하고 MessageServiceQueryTest가 Repository로 직접 생성한거군요!
저는 MessageServiceQueryTest에 한표 던집니다 😄

yeon-06 · 2022-08-17T06:02:35Z

backend/src/test/java/com/pickpick/message/application/MessageServiceQueryTest.java

+    @Autowired
+    private MessageService messageService;
+
+    @DisplayName("쿼리 파라미터와 count가 없다면, 회원의 첫번째 순서 구독 채널의 최신 메시지를 20개 내림차순 조회한다")


count는 쿼리 파라미터와는 다르다고 생각하신걸까요 ?-?
저는 count도 쿼리 파라미터도 포함된다고 생각했는데 별도로 명시하셨기에 궁금해서 여쭤봅니다!

JangBomi

MessageServiceQueryTest 와 MessageServiceQueryTestSql, 그리고 이전의 방식인 ParameterizedTest 세 가지를 비교해봤을 때 저는 MessageServiceQueryTest 방식이 가장 좋은 것 같아요.
연로그와 리차드가 제가 생각한 리뷰들 다 남겨줘서, 저는 우선 Approve 할게요!
연로그가 추가로 써준 테스트케이스도 추가되면 좋을 것 같아요!!

JangBomi · 2022-08-17T04:43:21Z

backend/src/test/java/com/pickpick/fixture/MessageFixtures.java

+    }
+
+    public Message create(final Channel channel, final Member member) {
+        return new Message(UUID.randomUUID().toString(), this.text, member, channel, this.dateTime, this.dateTime);


리뷰는 아니고 그냥 찾아봤는데 슬랙에서 메시지 아이디를 만들 때도 uuid로 만드는 것 같네요 규칙이 같아요!

JangBomi · 2022-08-17T09:51:58Z

backend/src/test/java/com/pickpick/message/application/MessageServiceQueryTest.java

+@Sql({"/truncate.sql"})
+@Transactional
+@SpringBootTest
+class MessageServiceQueryTest {


저도 MessageServiceQueryTest말고 다른 이름이었으면 좋겠어요!

hyewoncc · 2022-09-02T04:06:20Z

조회 테스트 개선을 쭉 고민했는데 오늘에서야 추가 커밋 올리네요 😅
@Nested를 이용해 아래 사항을 중점으로 개선해보았습니다

테스트 실행 시 출력 가독성 개선
테스트 픽스쳐 공유를 통한 리소스 절약 및 정확도 개선

작성한 역순으로 출력되는 게 아쉽지만, 내용을 충분히 적을 수 있어서 더 잘읽히는 것 같아요
한편 이렇게 바꿈으로써 생긴 고민도 있습니다

픽스쳐 공유를 위해 @Nested클래스 라이프 사이클을 변경
사용하지 않는 구독 변수 두 개의 할당이 불가피

그래도 저는 현재 버전이 제일 마음에 드는 것 같아요
괜찮다면 최종 머지할 때는 이 형태를 리마인더, 북마크 조회에도 적용해서 한 클래스로 합쳐 올리도록 하겠습니다!

‼️ 클래스 내 메소드 정렬이 조금 이상하게 되어있네요 😂 바꿔서 푸시해두겠습니다
원래 @Nested -> 나머지 테스트 -> private 메서드 가 맞아요

yeon-06

MessageServiceQueryTest의 내용을 MessageServiceTest로 옮겨주시기만 하면 될 것 같아요😄
정말정말정말 고생 많으셨습니다~~!!🥳🥳

yeon-06 · 2022-09-14T02:58:16Z

backend/src/test/java/com/pickpick/message/application/MessageServiceQueryTest.java

+        @Nested
+        @DisplayName("조회 조건과 관련없이")


중요한게 메서드와 가까이 붙어있도록 어노테이션 순서 통일 부탁드려요 😄

yeon-06 · 2022-09-14T03:08:29Z

backend/src/test/java/com/pickpick/message/application/MessageServiceQueryTest.java

+            MessageRequest request = searchByKeywordInChannels(List.of(notice, freeChat), keyword, allMessages.size());
+            MessageResponses response = messageService.find(summer.getId(), request);
+
+            @DisplayName("해당 채널에 있고, 특정 단어가 포함된 메시지가 조회된다")


DisplayName을 합쳐보면 특정 단어로 여러 채널에서 검색한다면 해당 채널에 있고, 특정 단어가 포함된 메시지가 조회된다 라는 말이 완성되는데요

Suggested change

@DisplayName("해당 채널에 있고, 특정 단어가 포함된 메시지가 조회된다")

@DisplayName("해당 채널들에서 특정 단어가 포함된 메시지가 조회된다")

위와 같은 변경은 어떨까 제안드려 봅니다👀

JangBomi

써머 👍 👍 MessageServiceQueryTest.java 이름만 MessageServiceTest.java로 바꿔서 나가면 될 것 같아요!! approve는 하겠습니다 :)

pickpick-sonarqube · 2022-09-15T02:13:58Z

Analysis Details

0 Issues

0 Bugs
0 Vulnerabilities
0 Code Smells

Coverage and Duplications

No coverage information (90.00% Estimated after merge)
0.00% Duplicated Code (0.60% Estimated after merge)

Project ID: woowacourse-teams_2022-pickpick_AYKprLeNXDQxKhlck1fc

View in SonarQube

* refactor: 메시지 조회 테스트 개선 * refactor: 코드리뷰 반영 및 코드 비교용 테스트 클래스 추가 * refactor: 코드리뷰 반영 * refactor: Nested 클래스를 이용한 조회 테스트 개선 * refactor: 코드리뷰 반영(어노테이션 순서 변경, DisplayName 개선) * refactor: 클래스명을 MessageServiceTest로 변경 및 시간에 따른 리마인드 테스트 이동

@disabled

* refactor: 리액트 쿼리 로직 커스텀 훅 분리 (#482) * refactor: 불필요하게 Feed 페이지와 SpecificPage 에서 훅 실행하는 것 제거 * refactor: useEffect 실행구문에 Early Return 추가 * refactor: useMutateChannel 훅 분리 - 채널 구독 관련 Muatation 로직 훅으로 분리 - Settle 이후 동작 (refetch)은 Props 로 받도록 구현 * refactor: 전체 채널 가져오는 useQuery 훅으로 분리 * refactor: useMutateBookmark 훅 작성 및 적용 * refactor: useGetInfiniteBookmarks 훅 작성 및 적용 * refactor: useGetInfiniteMessages 훅 작성 및 적용 - 기존의 메시지 무한 스크롤 요청하던 페이지에서 일괄적으로 사용하도록 적용 - channelId, keyword, date 를 옵셔널 프롭스로 받도록 함 * refactor: useGetInfiniteReminders 훅 작성 및 적용 * refactor: useGetCertification 훅 작성 및 적용 * refactor: useSetTargetMessage -> useSetReminderTargetMessage 네이밍 수정 * refactor: slack login url 상수화 * refactor: hasNavBar 함수 한줄로 리팩터링 * refactor: channelId 쿼리 파람 구하는 유틸함수 리팩터링 * chore: 오타수정 * refactor: 불필요한getPageParam util 함수 제거 - 훅으로 분리하여 재사용 될 일이 없기 때문에, 각 훅에서 직접 함수를 정의하여 사용하도록 수정 * refactor: @disabled 지정한 테스트 제거 (#487) * chore: 운영용 slack app 추가 및 yml 설정 정리 (#493) * chore: test용 submodule 제거 * chore: submodule 업데이트 변경 - yml 파일 통일 - local profile의 DB 정보 변경 - prod용 slack app 정보 추가 * refactor: 슬랙에서 오는 예외를 프로젝트 예외인 SlackApiCallException으로 변환 (#492) * chore: profile 설정 오류 업데이트 (#496) * refactor: 서비스 테스트에 DatabaseCleaner 도입 (#479) * refactor: 사용하지 않는 @AutoConfigureMockMvc 어노테이션 제거 * refactor: ServiceTest에서 DatabaseCleaner 도입 * refactor: 테스트에서 @transactional 어노테이션 제거 * optimization: 프론트엔드 성능 최적화 (#499) * chore: package.json 수정 - scripts에 build-dev 모드 설정 - 불필요한 @babel dependencies 제거 - webpack-bundle-analyzer 설치 Co-authored-by: moonheekim0118 <hellomooneekim@gmail.com> * chore: ts config module: esnext로 수정 Co-authored-by: moonheekim0118 <hellomooneekim@gmail.com> * chore: webpack config 수정 및 babelrc 제거 - bundle analyzer dev mode에 추가 - babel loader 제거 Co-authored-by: moonheekim0118 <hellomooneekim@gmail.com> * chore: Routes.tsx import문 수정 Co-authored-by: moonheekim0118 <hellomooneekim@gmail.com> * chore: file-loader 제거 후 webpack asset 설정 적용 Co-authored-by: moonheekim0118 <hellomooneekim@gmail.com> * chore: assets 파일 정리 - 불필요한 italic font 제거 - woff2, ttf 폰트 추가 - svg 파일 제거 Co-authored-by: moonheekim0118 <hellomooneekim@gmail.com> * chore: 파일 확장자 설정 (웹팩, d.ts) Co-authored-by: moonheekim0118 <hellomooneekim@gmail.com> * chore: prettier ignore 설정 Co-authored-by: moonheekim0118 <hellomooneekim@gmail.com> * chore: tway air 폰트 preload 설정 Co-authored-by: moonheekim0118 <hellomooneekim@gmail.com> * chore: 변경된 폰트 포맷 적용 Co-authored-by: moonheekim0118 <hellomooneekim@gmail.com> * refactor: svgIcon 컴포넌트 생성 및 적용 Co-authored-by: moonheekim0118 <hellomooneekim@gmail.com> Co-authored-by: moonheekim0118 <hellomooneekim@gmail.com> * refactor: 메시지 조회 테스트 리팩토링 (#421) * refactor: 메시지 조회 테스트 개선 * refactor: 코드리뷰 반영 및 코드 비교용 테스트 클래스 추가 * refactor: 코드리뷰 반영 * refactor: Nested 클래스를 이용한 조회 테스트 개선 * refactor: 코드리뷰 반영(어노테이션 순서 변경, DisplayName 개선) * refactor: 클래스명을 MessageServiceTest로 변경 및 시간에 따른 리마인드 테스트 이동 * refactor: 채널 목록 조회 API 개선 (#495) * refactor: Channel -> ChannelResponse 변환 로직 개선 * test: 불필요한 변수 선언 제거 * refactor: 채널 목록 조회 로직 ChannelSubscriptionService에서 ChannelService로 이동 * test: ChannelSubscriptionServiceTest에서 ChannelService 의존 제거 * test: 채널 전체 조회 테스트 추가 * refactor: Map 대신 Set 활용 * style: 개행 문자 제거 및 메서드 순서 변경 * refactor: ChannelService와 ChannelCreateService 분리 * test: DB Cleaner 적용 * test: DB Cleaner 누락된 곳 추가 * refactor: Service Layer에서 Domain이 아닌 DTO 반환 * style: 줄바꿈 개선 * refactor: 헤더 토큰값 추출 클래스를 auth 패키지로 이동 (#504) * refacotor: 접근제어자 수정 (#512) * feat: SlackApiCallException 로깅 메시지 추가 (#498) * feat: 슬랙 api 호출 예외 발생 시 에러 메시지 추가 및 로깅 * refactor: 로깅 메시지 상수화, 응답코드 500 지정 * feat: ChannelCreateService 의 슬랙 호출 예외에 메시지 추가 * feature: submodule 버전을 jenkinsfile을 삭제한 버전으로 업그레이드 (#510) * refactor: 메시지/북마크/리마인더 조회 API 스펙 변경 (#513) * refactor: 메시지 조회 응답 isLast를 hasPast로 변경, isFuture 추가 * refactor: 메시지 api 변경 관련 테스트 수정 * refactor: 북마크의 isLast 필드를 hasPast로 변경 * refactor: 리마인더 조회 시 isLast 필드를 hasFuture로 변경 * refactor: 필요없는 @JsonProperty 선언 제거 * refactor: has~ 필드의 getter 명 개선, 오타 정정 * refactor: 메시지/북마크/리마인더 조회 API 스펙 변경 대응 (#515) * refactor: 불필요한 주석 제거 * refactor: 북마크 get API 스펙 변경 대응 - isLast -> hasPast 로 수정하여 nextPageParam 함수 구현 - bookmark 응답값 타입 interface 수정 * chore: 단위 테스트 임시 주석처리 * refactor: 리마인더 get API 스펙 변경 대응 - isLast -> hasFuture 로 수정하여 nextPageParam 함수 구현 - Reminder 응답값 타입 interface 수정 * refactor: 메시지 get API 스펙 변경 대응 - isLast -> hasFuture 로 변경하여 previousPageParam 함수 구현 - isLast -> hasPast 로 변경하여 nextPageParam 함수 구현 - 메시지 응답값 interface 수정 * fix: Storybook 빌드 실패 해결 - 삭제된 svg 파일 Import 문 제거, 컴포넌트 svg 로 대체 * fix: font 적용되지 않는 문제 해결 - font-face 내 오타 수정 * refactor: HashMap 강제 형변환 제거 (#508) * refactor: HashMap 강제 형변환 대신 ObjectMapper와 DTO 사용 * refactor: challenge 검증 방식 변경 * refactor: code smell 제거 - final 키워드 추가 - 불필요한 import 제거 - util 클래스에서 private 생성자 생성 - deprecated 제거 * test: toJson 위치 변경 * refactor: json 변환 실패 시 커스텀 Exception 적용 * test: JsonUtils 테스트 코드 추가 * refactor: DTO 이름 변경 * refactor: ControllerDto -> ServiceDto 변경 로직 ControllerDto로 이동 * feat: 저장하는 이미지 512에서 48로 변경 * test: json으로 전환하는 로직 메서드에서 제거 * refactor: 미사용 import 제거 * style: 컨벤션 맞춤 - 불필요한 공백 제거 - 불필요한 import 제거 - 어노테이션 순서 변경 - InvalidJsonRequestException 로깅 메시지 변경 * test: toJson() 로직 분리 * fix: 사파리에서 로그인 되지 않는 버그 해결 (#518) * refactor: 불필요한 주석 제거 * fix: 사파리에서 로그인되지 않는 버그 해결 - 사파리에서 지원하지 않는 정규표현식 문법 (look behind) 제거 - 대체 가능한 정규표현식 및 로직으로 대체 * fix: 달력 컴포넌트 미래 날짜 및 빈 공간 클릭시 데이터 요청되는 버그 수정 (#519) * refactor: 매번 생성되는 date 객체 최소한으로 생성될 수 있도록 변경 * refactor: isBlank, isFuture 인 경우 클릭 이벤트 발생하지 않도록 css 수정 * fix: isBlank, isFuture인 day 클릭시 link 이동되는 버그 해결 * refactor: reminderModal component 리팩터링 (#521) * refactor: useSetReminder hook 분리 - useDatePicker, useTimePicker로 분리 Co-authored-by: moonheekim0118 <hellomooneekim@gmail.com> * refactor: ReminderModal component 분리 - DatePicker, TimePicker 로 분리 Co-authored-by: moonheekim0118 <hellomooneekim@gmail.com> * refactor: DatePicker, TimePicker 스크롤 위치 버그 수정 및 string literal, magic number 상수화 Co-authored-by: moonheekim0118 <hellomooneekim@gmail.com> Co-authored-by: moonheekim0118 <hellomooneekim@gmail.com> * refactor: 최상단으로 스크롤을 움직이는 로직 리팩터링 (#527) * feat: 초기 렌더링 및 의존성 배열 내 상태가 바뀌었을 때 스크롤 위치를 최상단으로 변경하는 로직 훅으로 작성 * refactor: 반복되는 로직 useScrollToTop 훅 적용 * fix: 시,분,초가 같은 메시지를 조회하는 경우 정렬 순서가 바뀌는 현상 (#528) * feat: postedDate의 컬럼 타입 변경 * feat: LocalDateTime 변환 시 milliSecond 단위로 변경 * test: 올바른 테스트 데이터 세팅 * refacotor: 쿼리에서 불필요한 left outer join 삭제 (#531) * fix: Drawer 내부 클릭 시, Drawer 가 닫히는 문제 해결 (#529) * refactor: useOuterClick 훅 수정 - props 로 내려준 숫자에 따라서 innerRef 를 n개 생성 할 수 있도록 수정 * fix: Drawer 내부 클릭 시, Drawer 닫히는 문제 해결 - 변경된 useOuterClick 훅 적용하여 여러군데에 innerRef 적용 할 수 있도록 수정 * refactor: useOuterClick 훅 적용 * refactor: useOuterClick 에서 props 로 받은 ref 숫자가 0일 경우, 길이가 1개인 배열을 디폴트로 생성하도록 수정 * fix: useOuterClick의 이벤트 리스너에서 불필요한 조건문 제거 - Array 의 length 가 무조건 1개 이상이기 때문에, 불필요한 가드로 판단되어 제거 * fix: nullish 연산자 or 연산자로 수정 * refactor: useOuterClick 함수에서 innerRef 배열 크기 지정을 강제하도록 수정 - requiredRefCount -> requiredInnerRefCount 로 네이밍 수정 - requiredInnerRefCount 타입 옵셔널 제거 - requiredInnerRefCount 가 0보다 같거나 작을 경우 1로 강제하도록 수정 * refactor: useOuterClick 반환 값 주석 작성 * refactor: useOuterClick 반환값 수정 - 기존에 객체형태로 반환하던 것을 array 로 수정 * refactor: SearchForm component 리렌더링 최소화 로직 추가 (#535) * chore: tomcat 설정 추가 (#533) * feat: Service Layer에서 완성된 DTO 반환 (#534) * refactor: ChannelSubscriptionService DTO 반환하도록 변경 * style: 메서드 순서 CRUD로 변경 * style: 줄바꿈 변경 Co-authored-by: hyewoncc <80666066+hyewoncc@users.noreply.github.com> Co-authored-by: yeonLog <53105735+yeon-06@users.noreply.github.com> Co-authored-by: 봄 <55357130+JangBomi@users.noreply.github.com> Co-authored-by: Jaejeung Ko <jaejeung3210@gmail.com>

hyewoncc added 🎉 BE 백엔드 관련 ⚙️ REFACTOR labels Aug 15, 2022

hyewoncc requested review from yeon-06, JangBomi and HJ-Rich August 15, 2022 08:39

hyewoncc self-assigned this Aug 15, 2022

hyewoncc added this to In progress in 4차 스프린트 via automation Aug 15, 2022

hyewoncc force-pushed the feature/refactor-message-test-fixture branch 2 times, most recently from d5480b3 to 24fa974 Compare August 16, 2022 12:48

HJ-Rich approved these changes Aug 16, 2022

View reviewed changes

4차 스프린트 automation moved this from In progress to Reviewer approved Aug 16, 2022

yeon-06 requested changes Aug 17, 2022

View reviewed changes

4차 스프린트 automation moved this from Reviewer approved to Review in progress Aug 17, 2022

JangBomi approved these changes Aug 17, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

hyewoncc force-pushed the feature/refactor-message-test-fixture branch from 2920aa4 to 783fd46 Compare September 2, 2022 04:14

This comment has been minimized.

Sign in to view

hyewoncc force-pushed the feature/refactor-message-test-fixture branch from 783fd46 to 519c90f Compare September 2, 2022 04:18

This comment has been minimized.

Sign in to view

hyewoncc force-pushed the feature/refactor-message-test-fixture branch from 519c90f to 1145c28 Compare September 10, 2022 02:23

This comment has been minimized.

Sign in to view

hyewoncc added this to To do in 5차 스프린트 Sep 13, 2022

yeon-06 approved these changes Sep 14, 2022

View reviewed changes

yeon-06 requested review from JangBomi and yeon-06 September 14, 2022 03:14

yeon-06 approved these changes Sep 14, 2022

View reviewed changes

JangBomi approved these changes Sep 14, 2022

View reviewed changes

hyewoncc changed the title ~~feat: 메시지 조회 테스트 리팩토링~~ refactor: 메시지 조회 테스트 리팩토링 Sep 14, 2022

hyewoncc added 6 commits September 15, 2022 10:50

refactor: 메시지 조회 테스트 개선

8cb432d

refactor: 코드리뷰 반영 및 코드 비교용 테스트 클래스 추가

2d9a65c

refactor: 코드리뷰 반영

b9cb3a3

refactor: Nested 클래스를 이용한 조회 테스트 개선

3bbc5c6

refactor: 코드리뷰 반영(어노테이션 순서 변경, DisplayName 개선)

8e87ece

refactor: 클래스명을 MessageServiceTest로 변경 및 시간에 따른 리마인드 테스트 이동

3839c85

hyewoncc force-pushed the feature/refactor-message-test-fixture branch from f8a62dc to 3839c85 Compare September 15, 2022 02:11

hyewoncc merged commit 582b6ec into develop Sep 15, 2022

5차 스프린트 automation moved this from To do to Done Sep 15, 2022

hyewoncc deleted the feature/refactor-message-test-fixture branch September 15, 2022 02:15

hyewoncc mentioned this pull request Sep 15, 2022

MessageServiceTest에 DatabaseCleaner 적용 #501

Closed

		@@ -8,6 +8,8 @@ public interface MessageRepository extends Repository<Message, Long> {

		Message save(Message message);

		List<Message> saveAll(Iterable<Message> messages);

	@DisplayName("쿼리 파라미터와 count가 없다면, 회원의 첫번째 순서 구독 채널의 최신 메시지를 20개 내림차순 조회한다")
	@DisplayName("count만 전달됐다면, 회원의 첫번째 순서 구독 채널의 최신 메시지 20개를 작성시간 내림차순 조회한다")

	@DisplayName("해당 채널에 있고, 특정 단어가 포함된 메시지가 조회된다")
	@DisplayName("해당 채널들에서 특정 단어가 포함된 메시지가 조회된다")

refactor: 메시지 조회 테스트 리팩토링 #421

refactor: 메시지 조회 테스트 리팩토링 #421

Conversation

hyewoncc commented Aug 15, 2022

요약

작업 내용

github-actions bot commented Aug 15, 2022 • edited

Unit Test Results

hyewoncc commented Aug 16, 2022

HJ-Rich 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

HJ-Rich Aug 20, 2022 • edited

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

yeon-06 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

yeon-06 Aug 17, 2022 • edited by hyewoncc

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

JangBomi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

hyewoncc commented Sep 2, 2022 • edited

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

yeon-06 left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JangBomi left a comment

Choose a reason for hiding this comment

pickpick-sonarqube bot commented Sep 15, 2022

Analysis Details

0 Issues

Coverage and Duplications

github-actions bot commented Aug 15, 2022 •

edited

HJ-Rich Aug 20, 2022 •

edited

yeon-06 Aug 17, 2022 •

edited by hyewoncc

hyewoncc commented Sep 2, 2022 •

edited

yeon-06 left a comment •

edited