[Feature] - BE 테스트 개선 1단계 Test Fixture 운용 방식 통일 (Enum Fixture) #614

Libienz · 2024-11-30T15:06:08Z

✅ 작업 내용

모든 Test FIxture를 Enum Fixture 기반으로 통일
테스트 객체 구성 시 연관된 도메인을 주입하도록 로직 수정
- 연관 되는 객체가 어떤 객체인지 파라미터 레벨에서 알 수 있도록 함
- 강결합된 의존성 약화
Request 객체 Builder 구현 (builder와 미리 작성된 Fixture를 통해서 쉽게 Request 객체 작성하실 수 있습니다.)
setUp 메서드 활용, 도메인 구성 로직 중복 제거 개선

🙈 참고 사항

loc가 많네요.. 죄송합니다 ^^;; 근데 테스트 코드가 많이 엉켜있어서 PR 분리하기도 여간 까다로워보여서 우선 진행한 것이에요...
너그러이 이해해주시면 감사하겠습니다!

위에서 언급드린 작업 내용이 핵심이자 전부입니다.
위를 위주로 코드 작업 이해해주시고 리뷰 주시면 감사하겠습니다. 🙇🏻‍♂️

(헬퍼가 테스트 데이터를 은닉하는 문제는 다음 단계의 개선에서 해결할 예정입니다)

Libienz · 2024-11-30T15:06:40Z

작업이 조금 늦어져서 내용을 공유드리며 진행하기 위해 PR 우선 올립니다 draft 풀리면 다시 알려드릴게유 👍

github-actions · 2024-11-30T15:07:23Z

Test Results

30 files 30 suites 9s ⏱️
297 tests 296 ✅ 1 💤 0 ❌
309 runs 308 ✅ 1 💤 0 ❌

Results for commit d7ec90f.

♻️ This comment has been updated with latest results.

Libienz · 2024-12-04T18:21:19Z

@hangillee @eunjungL @slimsha2dy @nak-honest

이어지는 테스트 개선(테스트 비용 개선, 컨벤션 통일)을 위한 밑작업들이 완료되었습니다.
Fixture의 운용방식을 통일하고 테스트 객체를 유연한 api로 생성할 수 있도록 작업해보았어요.
얘기 나누어 보고 싶은 부분에 코멘트 남겨볼텐데요 많은 의견 주시면 감사하겠습니다.

Libienz · 2024-12-04T18:25:02Z

backend/src/test/java/kr/touroot/member/fixture/MemberFixture.java

+    public MemberRequest getCreateRequest() {
+        return new MemberRequest(email, password, nickname, profileImageUrl);
+    }
+


enumFixture는 속성 값들을 거느리고 해당 fixture를 통해서 도메인 뿐만 아니라 위의 코드처럼 생성 요청 DTO를 만들 수도 있음을 안내드립니다.

멋지네요! 확실히 Enum Fixture가 기존 방식보다 여러모로 유연하다는 생각이 듭니다.

Libienz · 2024-12-04T18:34:59Z

backend/src/test/java/kr/touroot/travelogue/controller/TravelogueControllerTest.java

+        TravelogueRequest requestWithNoTags = TravelogueRequestBuilder.forTravelogue(JEJU_TRAVELOGUE)
+                .addTags(Collections.EMPTY_LIST)
+                .addDay(FIRST_DAY)
+                .addPlaceWithPhotosIntoDay(HAMDEOK_BEACH, List.of(TRAVELOGUE_PHOTO_1, TRAVELOGUE_PHOTO_2))
+                .addPlaceWithPhotosIntoDay(SEONGSAN_ILCHULBONG, List.of(TRAVELOGUE_PHOTO_3))
+                .buildDay()
+                .addDay(SECOND_DAY)
+                .addPlaceWithPhotosIntoDay(JEJU_FOLK_VILLAGE, List.of(TRAVELOGUE_PHOTO_4))
+                .buildDay()
+                .build();


여러분들에게 투표를 받았었는데요~ 단계별로 객체를 선언 후 Integration하는 방법보다는 Builder를 이용하는 쪽이 개발자가 사용하기 편하다는 의견에 저도 동의해서 이번에 빌더를 제가 새롭게 구상해보았습니다.

다만 TravelogueRequestBuilder는 addDay를 하게 되면 TravelogueRequestBuilder 이외에도 DayBuilder를 추가적으로 다루어야 하는데요, 이 부분에서 고민인 점이 있었습니다.

DayBuilder는 장소와 장소에 해당하는 사진을 addPlaceWithPhotosIntoDay로 추가할 수 있음

DayBuilder는 장소가 몇개 추가되는지 알 수 없음

따라서 buildDay를 통해서 해당 day를 생성하는 일이 끝났음을 알려야 함

buildDay() 함수가 여러분에게 위화감이 있지는 않은지 궁금하군요..
사실 buildDay() 함수 없이 다음처럼 작성되는 것이 여러분들이 더 사용하기 편할 수도 있습니다.

Suggested change

TravelogueRequest requestWithNoTags = TravelogueRequestBuilder.forTravelogue(JEJU_TRAVELOGUE)

.addTags(Collections.EMPTY_LIST)

.addDay(FIRST_DAY)

.addPlaceWithPhotosIntoDay(HAMDEOK_BEACH, List.of(TRAVELOGUE_PHOTO_1, TRAVELOGUE_PHOTO_2))

.addPlaceWithPhotosIntoDay(SEONGSAN_ILCHULBONG, List.of(TRAVELOGUE_PHOTO_3))

.buildDay()

.addDay(SECOND_DAY)

.addPlaceWithPhotosIntoDay(JEJU_FOLK_VILLAGE, List.of(TRAVELOGUE_PHOTO_4))

.buildDay()

.build();

TravelogueRequest requestWithNoTags = TravelogueRequestBuilder.forTravelogue(JEJU_TRAVELOGUE)

.addTags(Collections.EMPTY_LIST)

.addDay(FIRST_DAY)

.addPlaceWithPhotosIntoDay(HAMDEOK_BEACH, List.of(TRAVELOGUE_PHOTO_1, TRAVELOGUE_PHOTO_2))

.addPlaceWithPhotosIntoDay(SEONGSAN_ILCHULBONG, List.of(TRAVELOGUE_PHOTO_3))

.addDay(SECOND_DAY)

.addPlaceWithPhotosIntoDay(JEJU_FOLK_VILLAGE, List.of(TRAVELOGUE_PHOTO_4))

.build();

다만 이렇게 구현하기 위해서는 builder 클래스 내부에 현재 빌더가 존재하는지 null처리 로직이 불가피하고 day가 없는 상태에서도 addPlace함수를 사용할 수 있게 되어 사용 명세가 런타임에 예외로 전파되게 됩니다.

런타임에 예외가 발생하는 day없이 place를 추가 시도하는 코드

TravelogueRequest requestWithNoTags = TravelogueRequestBuilder.forTravelogue(JEJU_TRAVELOGUE) .addTags(Collections.EMPTY_LIST) .addPlaceWithPhotosIntoDay(HAMDEOK_BEACH, List.of(TRAVELOGUE_PHOTO_1, TRAVELOGUE_PHOTO_2)) .addPlaceWithPhotosIntoDay(SEONGSAN_ILCHULBONG, List.of(TRAVELOGUE_PHOTO_3))

buildDay()를 명시적으로 운용하면 addDay없이 addPlace 함수를 사용할 수 없기에 컴파일 타임에 개발자 인지비용이 줄 수 있을 것 같아 현행을 택했는데요~ 의견 부탁드립니다.

전 buildDay()가 있는 편이 더 낫다고 생각합니다.
말씀해주신 것처럼 런타임에 예외가 발생하는 것을 막는게 훨씬 중요하다고 생각합니다.
현행 코드의 흐름을 봤을 때, Day의 추가가 끝나게 되는 것을 명시하는 게 그렇게 위화감이 들진 않아요!

쓰기 편한 방법은 확실히 buildDay()가 없는 쪽이지만 편하다고 해서 좋은 코드 같진 않네요..

작성될 수 없는 코드는 애초에 컴파일 타임에 막히는게 좋다고 생각합니다!
사용할 수 있는 메서드라고 떠서 사용했는데 런타임 예외가 발생하면 빠른 파악도 힘들고 오히려 혼란이 가중될 것 같다는 의견입니다.
buildDay() 명시적 운용에 찬성합니다 ~ 👍

Libienz · 2024-12-04T18:35:42Z

backend/src/test/java/kr/touroot/travelogue/controller/TravelogueControllerTest.java

+        TravelogueRequest requestWithTag = TravelogueRequestBuilder.forTravelogue(JEJU_TRAVELOGUE)
+                .addTags(List.of(tag.getId()))
+                .addDay(FIRST_DAY)
+                .addPlaceWithPhotosIntoDay(HAMDEOK_BEACH, List.of(TRAVELOGUE_PHOTO_1, TRAVELOGUE_PHOTO_2))
+                .addPlaceWithPhotosIntoDay(SEONGSAN_ILCHULBONG, List.of(TRAVELOGUE_PHOTO_3))
+                .buildDay()
+                .addDay(SECOND_DAY)
+                .addPlaceWithPhotosIntoDay(JEJU_FOLK_VILLAGE, List.of(TRAVELOGUE_PHOTO_4))
+                .buildDay()
+                .build();


추가로 빌더를 통한 Request 객체 구성은 위처럼 하실 수 있습니다~

사용성에 대해서 많은 피드백 주세요 여러분들의 개발 생산성을 올려보고 싶습니다~

Libienz · 2024-12-04T18:41:56Z

backend/src/test/java/kr/touroot/travelogue/fixture/TraveloguePhotoFixture.java


-    TRAVELOGUE_PHOTO(1, "https://dev.touroot.kr/temporary/image1.png", TRAVELOGUE_PLACE.get());
+    TRAVELOGUE_PHOTO_1(1, "https://dev.touroot.kr/temporary/image1.png"),


fixture 내부에서 의존 객체가 정해지던 방식은 픽스처를 통한 객체 생성 시 파라미터로 주입받도록 바뀌었습니다.

fixture의 내부 구현을 외부로 공개해 유연성을 높이고 테스트 대상을 명확히 하는게 확실히 나아보이네요.
그동안 테스트 코드 확인할 때마다 어떤 데이터가 들어가고 있는지 하나하나 확인해야 했던 걸 생각하면..

캡슐화를 깬 코드라고 말할 수도 있겠지만 전 리비의 생각이 더 좋아보입니다.

저도 새로 추가된 방식이 훨씬 좋아보입니다!

테스트 코드에서 연관된 데이터가 한눈에 보이는 것이 좋은 테스트 코드라고 생각합니다!

또한 외부에서 연관 관계를 가지는 객체를 주입해 줌으로써 결합도가 낮아지는 부분도 더 좋다고 생각합니다!
해당 연관관계가 만약 바뀌게 된다면 이를 사용하는 모든 픽스처가 전부 영향을 받을텐데, 유지보수성이 너무 떨어지게 되니까요!

테스트간의 격리가 좋은 테스트 코드의 기본적인 원칙이기 때문에 리비의 생각에 동의합니다!

Libienz · 2024-12-04T18:42:18Z

backend/src/test/java/kr/touroot/travelogue/fixture/TraveloguePhotoFixture.java

+    public TraveloguePhoto getTraveloguePhotoIncludedIn(TraveloguePlace place) {
        return new TraveloguePhoto(order, url, place);
    }


의존 객체를 파라미터로 주입해야 합니다.

hangillee

몇가지 확인해주셨으면 하는 부분 있습니다!
나머지는 모두 잘 개선해주신 것 같아서 Apporve 드리겠습니다!

고생 많으셨습니다 리비!

hangillee · 2024-12-05T03:08:29Z

backend/src/test/java/kr/touroot/member/fixture/MemberFixture.java

+    public MemberRequest getCreateRequest() {
+        return new MemberRequest(email, password, nickname, profileImageUrl);
+    }
+


멋지네요! 확실히 Enum Fixture가 기존 방식보다 여러모로 유연하다는 생각이 듭니다.

hangillee · 2024-12-05T03:10:41Z

backend/src/test/java/kr/touroot/member/service/MemberServiceTest.java

-        MemberRequest request = VALID_MEMBER.getRequest();
+        MemberRequest request = MemberFixture.DEFAULT_MEMBER.getCreateRequest();


Static Import를 해제한 이유는 DEFAULT_MEMBER의 출처(?)를 명확하게 하기 위함으로 이해하면 될까요?

static import에 대해서 다른 분들의 생각두 궁금하군요

fixture에서 생성하고 있다라는 명확한 표시가 괜찮을가요 아니면 import하는 편이 가독성이 좋을까요??
다른 분들 의견도 종합되면 반영해보도록 하겠습니다.

이전에 미션하다가 static import는 출처를 명시적으로 확인하기 어려워 지양하는 것이 좋다는 리뷰를 받은 적 있습니다. (참고자료)
이 의견에 공감을 많이 했어서 개인적으로 명확한 표시에 한 표 던지고 싶습니다.

저는 테스트 코드에서 자주 사용되는 정적 멤버이기 때문에 static import를 사용해도 좋다고 생각합니다.
가독성이 올라가는 이점이 더 큰 것 같아서요!

리뷰 받을 때 오히려 반대 리뷰를 받았던 기억도 있구요!
참고 링크

클로버가 첨부해 주신 링크 내용에서도 네임 스페이스가 오염되는 것이 가장 큰 단점이라고 나와있는데,
테스트 코드에서의 픽스쳐 이름이 겹칠 가능성은 희박하다고 생각되구요!!

다른 분들은 어떻게 생각하실까요?

우선은 static import는 적용하지 않겠습니다~

hangillee · 2024-12-05T03:11:47Z

backend/src/test/java/kr/touroot/member/service/MemberServiceTest.java

-    @DisplayName("중복된 이메일을 가진 회원을 생성하려하면 예외가 발생한다.")
+    @DisplayName("중복된 닉네임을 가진 회원을 생성하려하면 예외가 발생한다.")


저희 닉네임 중복도 제한되고 있었나요?
카카오톡 로그인이라 닉네임(이름)은 중복 허용해야하지 않나요?

오 그러네요..

#616

이슈 생성했습니다~
간단한 작업인 듯 해서 해당 PR 머지되고 제가 작업 진행할게요~

hangillee · 2024-12-05T03:13:39Z

backend/src/test/java/kr/touroot/travelogue/fixture/TravelogueCountryFixture.java

@@ -7,12 +7,12 @@

 @AllArgsConstructor
 public enum TravelogueCountryFixture {
-    TRAVELOGUE_COUNTRY(CountryCode.KR, 1L);
+    KOREA(CountryCode.KR, 1L);


명확한 네이밍👍

hangillee · 2024-12-05T03:15:38Z

backend/src/test/java/kr/touroot/travelogue/fixture/TraveloguePhotoFixture.java


-    TRAVELOGUE_PHOTO(1, "https://dev.touroot.kr/temporary/image1.png", TRAVELOGUE_PLACE.get());
+    TRAVELOGUE_PHOTO_1(1, "https://dev.touroot.kr/temporary/image1.png"),


fixture의 내부 구현을 외부로 공개해 유연성을 높이고 테스트 대상을 명확히 하는게 확실히 나아보이네요.
그동안 테스트 코드 확인할 때마다 어떤 데이터가 들어가고 있는지 하나하나 확인해야 했던 걸 생각하면..

캡슐화를 깬 코드라고 말할 수도 있겠지만 전 리비의 생각이 더 좋아보입니다.

eunjungL

수고하셨습니다 리비~ 👍
PR양이 어마무시한만큼 고생을 많이 하셨을 것 같군요...

의견을 나누고 싶은 리뷰가 대부분이라 바로 approve 드립니다~
줄바꿈 요상한 부분 몇 개 발견했는데 고 부분만 놓치지 않도록 확인 부탁드려요 😄

eunjungL · 2024-12-05T05:49:02Z

backend/src/test/java/kr/touroot/member/fixture/MemberFixture.java

+    public MemberRequest getCreateRequestWithEmail(String email) {
+        return new MemberRequest(email, password, nickname, profileImageUrl);
+    }
+
+    public MemberRequest getCreateRequestWithPassword(String password) {
+        return new MemberRequest(email, password, nickname, profileImageUrl);
+    }
+
+    public MemberRequest getCreateRequestWithNickname(String nickname) {
+        return new MemberRequest(email, password, nickname, profileImageUrl);
+    }
+
+    public MemberRequest getCreateRequestWithProfileImageUrl(String profileImageUrl) {
+        return new MemberRequest(email, password, nickname, profileImageUrl);
+    }
+
+    public MemberRequest getCreateRequestWithEmailAndNickname(String email, String nickname) {
+        return new MemberRequest(email, password, nickname, profileImageUrl);
+    }


테스트에 필요한 조합이 다르다보니 점층적인 생성자 오버로딩이 많아졌네요..
새로운 조합이 생길 때마다 또 다시 오버로딩 메서드가 생긴다는 유지보수 약점이 될 수도 있어 보입니다. 😢
Travelouge 쪽은 RequestBuilder를 도입을 하신걸로 보이는데 이런 부분도 Builder 패턴 도입 고민해보는 것도 좋을 것 같아 보입니다

builder를 사용해도 개선점이 크진 않을 것 같아요~ 미리 작성된 MemberFixture에 email하나, 닉네임 하나의 값만 바꾸고 싶은 경우에 사용하는 메서드들이어서요~

빌더를 통해 진행되어도 다음처럼 어색한 구문이 탄생할 수 있겠어요

MemberRequestBuilder.forMemberFixture(MemberFixture.LIBI) .setEmail("[email protected]") .build();

다만 Member에 새로운 속성이 추가되게 된다면 나머지 메서드들이 영향을 받는 다는 점에서 클로버가 지적해주신 유지보수 약점이 타당해 보이네요..

@eunjungL

빌더 역시 사용하게 되면 도메인 명세를 무시한 (특정 속성을 지니지 않은 데이터 생성 요청) 객체를 생성할 수 있는 취약점이 될 수 있을 것 같아서 우선은 트레이드 오프 지점이라고 결론 내렸습니다.

해당 코멘트는 반영하지 않을 예정인데 다른 의견 있으시면 알려주시면 감사하겠숨다

유지보수 약점만 인지하고 넘어가도 좋을 것 같습니다~
리비의 말처럼 명세를 무시한 생성 가능성을 열어두는것도 좋지 않은 방법으로 보이네요 👍
개선할 수 있는 방안이 있는지 함께 고민해보시죠~

eunjungL · 2024-12-05T05:52:05Z

backend/src/test/java/kr/touroot/member/service/MemberServiceTest.java

-        MemberRequest request = VALID_MEMBER.getRequest();
+        MemberRequest request = MemberFixture.DEFAULT_MEMBER.getCreateRequest();


이전에 미션하다가 static import는 출처를 명시적으로 확인하기 어려워 지양하는 것이 좋다는 리뷰를 받은 적 있습니다. (참고자료)
이 의견에 공감을 많이 했어서 개인적으로 명확한 표시에 한 표 던지고 싶습니다.

eunjungL · 2024-12-05T05:54:10Z

backend/src/test/java/kr/touroot/travelogue/domain/TravelogueCountryTest.java

    @DisplayName("검증 규칙에 어긋나지 않는 여행기 생성 시 예외가 발생하지 않는다")
    @Test
    void createTravelogueCountryWithValidData() {
-        assertThatCode(() -> new TravelogueCountry(TravelogueFixture.TRAVELOGUE.get(), CountryCode.KR, 1))
+        assertThatCode(


요상한 줄바꿈 발견! 밑에도 전체적으로 동일한 줄바꿈이 생겼네요.
리비의 특별한 의도가 있었을까요?

아이고.. 이게 뭐시여.. 수정했습니다 👍 꼼꼼히 봐주셔서 감사해유

eunjungL · 2024-12-05T06:03:41Z

backend/src/test/java/kr/touroot/travelogue/fixture/TraveloguePlaceFixture.java

+            "126.53326",
+            "KR"
+    ),
+    TRAVELOGUE_PLACE_WITH_NONE_COUNTRY_CODE(1,


Suggested change

TRAVELOGUE_PLACE_WITH_NONE_COUNTRY_CODE(1,

TRAVELOGUE_PLACE_WITH_NONE_COUNTRY_CODE(

1,

반영 완!

eunjungL · 2024-12-05T06:04:15Z

backend/src/test/java/kr/touroot/travelogue/fixture/TraveloguePlaceFixture.java

        return new TraveloguePlace(order, description, name, latitude, longitude, day, countryCode);
    }

-    public TraveloguePlace create(TravelogueDay day) {
-        return new TraveloguePlace(order, description, name, latitude, longitude, day, countryCode);
+    public TraveloguePlaceRequest getCreateRequestWith(TraveloguePhotoRequest... traveloguePhotoRequests) {


현재 사용 중이지 않는 메서드로 확인되는데요~ 이후의 확장성 혹은 다른 Fixture와의 통일성을 위해 남겨두신걸까요?

제거되지 못했던 메서드... 제거했습니다
코드 깔끔하게 유지할 수 있도록 신경써주셔서 감사합니다.

다음에 더욱 주의할게요~

eunjungL · 2024-12-05T06:07:47Z

backend/src/test/java/kr/touroot/travelogue/fixture/TraveloguePlaceFixture.java

+        );
+    }
+
+    public TraveloguePlaceRequest getCreateRequestWith(List<TraveloguePhotoFixture> photoFixtures) {


다른 Fixture들은 request를 받는데 Fixture를 받는 형식은 PlaceFixture가 유일하군요..
요기만 이런 특별한 메서드가 생긴 이유가 궁금합니다~

요부분은 Request를 Build할 때의 편의성을 위한 것입니다~
addPlaceWithPhotos로 한번에 생성할 때 생성되는 장소와 photo를 Fixture 준위로 통일하기 위함이기도 해요

TravelogueRequest requestContainsTooMuchPhotos = TravelogueRequestBuilder.forTravelogue(JEJU_TRAVELOGUE) .addTags(List.of(tag.getId())) .addDay(FIRST_DAY) .addPlaceWithPhotosIntoDay(HAMDEOK_BEACH, elevenPhotos) .addPlaceWithPhotosIntoDay(SEONGSAN_ILCHULBONG, List.of(TRAVELOGUE_PHOTO_3)) .buildDay() .addDay(SECOND_DAY) .addPlaceWithPhotosIntoDay(JEJU_FOLK_VILLAGE, List.of(TRAVELOGUE_PHOTO_4)) .buildDay() .build();

eunjungL · 2024-12-05T06:15:06Z

backend/src/test/java/kr/touroot/travelogue/helper/TravelogueRequestBuilder.java

+    }
+
+    @RequiredArgsConstructor
+    public static class TravelogueDayRequestBuilder {


inner class로 구현하신 이유가 궁금하군요~ 응집을 위해서였을까요..?!

응집도 있지만 parentBuilder를 통한 구현이 불가피한 만큼 사용되는 맥락속에서 코드를 이해하기 위한 것도 있는데..

지금 생각해봤을 떄 어색한 결론인지 궁금하네요~

조금 더 혼자 생각해봤는데 private static class로 이너클래스화 시킬 수 없다면 분리하는 쪽이 좋다고 생각하게 되었습니다!

분리했는데 어색한 결론이었다면 한 번 알려주세요~

개인적으로 inner class를 자주 사용하지는 않았다보니 분리된게 더 나아보이네요!
연관관계가 있긴 하지만 별개의 도메인을 생성해주는 builder이니 개별 class여도 어색함이 없어보입니다 👍

eunjungL · 2024-12-05T06:16:24Z

backend/src/test/java/kr/touroot/travelplan/controller/TravelPlanControllerTest.java

@@ -248,24 +247,20 @@ void readTravelPlanByInvalidShareKey() {
                .body("message", is("존재하지 않는 여행 계획입니다."));
    }

-    @DisplayName("여행기를 수정한다.")


zzzz 제가 발견하고 고쳤습니다~

eunjungL · 2024-12-05T06:17:08Z

backend/src/test/java/kr/touroot/travelplan/controller/TravelPlanControllerTest.java

-                .days(List.of(planDayRequest))
+        TravelPlan travelPlan = testHelper.initTravelPlanTestData(planWriter);
+
+        PlanRequest request = TravelPlanRequestBuilder.forTravelPlan(PAST_DATE_TRAVEL_PLAN)


최상위 Request build하는 과정이 한 번에 이루어져서 확실히 given 구성이 더 편리해보이네용 👍

eunjungL · 2024-12-05T06:21:57Z

backend/src/test/java/kr/touroot/travelogue/controller/TravelogueControllerTest.java

+        TravelogueRequest requestWithNoTags = TravelogueRequestBuilder.forTravelogue(JEJU_TRAVELOGUE)
+                .addTags(Collections.EMPTY_LIST)
+                .addDay(FIRST_DAY)
+                .addPlaceWithPhotosIntoDay(HAMDEOK_BEACH, List.of(TRAVELOGUE_PHOTO_1, TRAVELOGUE_PHOTO_2))
+                .addPlaceWithPhotosIntoDay(SEONGSAN_ILCHULBONG, List.of(TRAVELOGUE_PHOTO_3))
+                .buildDay()
+                .addDay(SECOND_DAY)
+                .addPlaceWithPhotosIntoDay(JEJU_FOLK_VILLAGE, List.of(TRAVELOGUE_PHOTO_4))
+                .buildDay()
+                .build();


작성될 수 없는 코드는 애초에 컴파일 타임에 막히는게 좋다고 생각합니다!
사용할 수 있는 메서드라고 떠서 사용했는데 런타임 예외가 발생하면 빠른 파악도 힘들고 오히려 혼란이 가중될 것 같다는 의견입니다.
buildDay() 명시적 운용에 찬성합니다 ~ 👍

nak-honest

와우 리비!!! 너무 고생 많으셨슴다!!

전체적으로 테스트 코드가 많이 개선 된것 같네요!
특히 결합도가 낮아진 점이 좋은 것 같습니다!

빌더 관련된 부분은 저도 좀 더 고민해보고 말씀드리겠습니다!

수고하셨습니다 리비!!!

nak-honest · 2024-12-05T05:58:47Z

backend/src/test/java/kr/touroot/travelogue/fixture/TravelogueDayFixture.java

+    public TravelogueDay getTravelogueDayIncludedIn(Travelogue travelogue) {
        return new TravelogueDay(order, travelogue);
    }


전체적으로 연관관계에 있는 애들을 주입 받으니, 결합도가 낮아지면서 오는 이점들이 많아지는 것 같네요!

nak-honest · 2024-12-05T06:09:58Z

backend/src/test/java/kr/touroot/travelogue/fixture/TraveloguePhotoFixture.java


-    TRAVELOGUE_PHOTO(1, "https://dev.touroot.kr/temporary/image1.png", TRAVELOGUE_PLACE.get());
+    TRAVELOGUE_PHOTO_1(1, "https://dev.touroot.kr/temporary/image1.png"),


저도 새로 추가된 방식이 훨씬 좋아보입니다!

테스트 코드에서 연관된 데이터가 한눈에 보이는 것이 좋은 테스트 코드라고 생각합니다!

또한 외부에서 연관 관계를 가지는 객체를 주입해 줌으로써 결합도가 낮아지는 부분도 더 좋다고 생각합니다!
해당 연관관계가 만약 바뀌게 된다면 이를 사용하는 모든 픽스처가 전부 영향을 받을텐데, 유지보수성이 너무 떨어지게 되니까요!

테스트간의 격리가 좋은 테스트 코드의 기본적인 원칙이기 때문에 리비의 생각에 동의합니다!

nak-honest · 2024-12-05T06:46:04Z

backend/src/test/java/kr/touroot/member/service/MemberServiceTest.java

-        MemberRequest request = VALID_MEMBER.getRequest();
+        MemberRequest request = MemberFixture.DEFAULT_MEMBER.getCreateRequest();


저는 테스트 코드에서 자주 사용되는 정적 멤버이기 때문에 static import를 사용해도 좋다고 생각합니다.
가독성이 올라가는 이점이 더 큰 것 같아서요!

리뷰 받을 때 오히려 반대 리뷰를 받았던 기억도 있구요!
참고 링크

클로버가 첨부해 주신 링크 내용에서도 네임 스페이스가 오염되는 것이 가장 큰 단점이라고 나와있는데,
테스트 코드에서의 픽스쳐 이름이 겹칠 가능성은 희박하다고 생각되구요!!

다른 분들은 어떻게 생각하실까요?

nak-honest · 2024-12-05T06:56:40Z

backend/src/test/java/kr/touroot/member/fixture/MemberFixture.java

+    DEFAULT_MEMBER(
+            null,
+            "[email protected]",
+            "password",
+            "https://dev.touroot.kr/temporary/profile.png",
+            "뚜리",
+            LoginType.DEFAULT
+    ),


사소하지만 DEFAULT 라는 네이밍이 조금 모호한 것 같아요!
카카오가 아닌 투룻에서 인증을 하니 TOUROOT_MEMBER 나,
자체 로그인을 의미하는 NATIVE_MEMBER 와 같은 이름을 주는건 어떨까요??

TOUROOT_MEMBER는 카카오 멤버는 투룻 멤버가 아닌건가?? 하는 명세 곡해가 있을 것 같고요
NATIVE_MEMBER 역시 그 명세를 드러내기에는 부족한 이름인 듯 하네요..!

근데 저도 낙낙의 의견에 동의해서 NON_SOCIAL_LOGINED_MEMBER정도로 고쳐볼까 하는데요~
개선에 대해서 어떻게 생각하는지 한번 말씀 주시면 감사하겠습니다~

TOUROOT_LOCAL_USER 채택!!

nak-honest · 2024-12-05T07:25:31Z

backend/src/test/java/kr/touroot/travelogue/helper/TravelogueTestHelper.java

+    public void initAllTravelogueTestData(List<Tag> tags) {
+        Member author = persistMember();
+        Travelogue travelogue = initTravelogueTestData(author);
+        initTravelogueTestDataWithTag(author, tags);
+        persistTravelogueLike(travelogue, author);
+    }
+


테스트 헬퍼는 계속 운용이 되는 것일까요?
만약 운용이 된다면 테스트 헬퍼도 파라미터로 픽스쳐를 받는 방식이 된다면 좋을 것 같아요!

그것이 결합도가 낮아지고, 테스트 코드 간의 격리가 되는 것이라 생각합니다!!
테스트 코드에서도 한눈에 보일 것 같구요!

PR 본문에서 말씀을 드렸어야 했는데요~ TestHelper는 테스트 데이터를 감추지 않도록 다음 단계에서 개선될 예정입니다.
헬퍼 사용성을 개선할지~ 운용하지 않게될지는 모르겠군요..!

Libienz · 2024-12-05T11:01:48Z

횽님들 안녕하세요 추가적으로 진행된 작업들 다음과 같은데 작업내용 확인해주세요~
별 말씀 없으시다면 오늘 오후 9시에 머지하겠습니다

사용하지 않는 상수 제거 개선
줄바꿈 등의 컨벤션 미적용 메서드 체크
inner class로 선언된 builder 명시적으로 분리
DEFAULT_MEMBER -> TOUROOT_LOCAL_MEMBER 로 fixture 이름 변경

머지 시간 이전까지 확인해주시면 감사드리겠습니다 🙇🏻‍♂️

Libienz added 15 commits November 19, 2024 18:15

test: OauthUserFixture - Enum 기반의 Fixture로 전환

47b71e2

test: MemberFixture - EnumFixture 컨벤션 통일 개선

e446ed7

test: MemberFixture 속성을 가지는 Member를 만드는 Request 객체 생성하는 기능 추가

26c535d

test: MemberFixture 속성을 기반으로 Request 객체를 만들도록 테스트 코드 수정

739b5a6

test: OauthUserFixture의 팩토리 메서드 이름 수정 (createXXX -> getXXX)

adb1607

test: MemberFixture의 팩토리 메서드 이름 수정 (createXXX -> getXXX)

ac6904f

test: 사용되지 않는 Fixture 클래스 제거 개선

5a02135

test: Travelogue관련 Enum Fixture 컨벤션 통일 개

521c494

test: TravelogueCountry, TravelogueDay 도메인 테스트 중복 데이터 셋팅 제거 개선

1bbcaed

test: Request 객체 생성 도메인 Fixture 내부로 응집

f0b74bf

test: 도메인 구성 시 연관관계를 직접 주입하도록 테스트 코드 수정

123a7ba

test: CountryCode 패키지 위치 수정

99963e0

test: TraveloguePhotoTest 연관관계 도메인 주입을 셋업 코드로 이동 개선

bf0b043

test: Fixture를 통한 도메인 생성 시 연관관계를 주입하도록 로직 수정

db795f0

chore: develop 브랜치의 작업들과 병합

807247d

Libienz added BE TEST 🧪 labels Nov 30, 2024

Libienz self-assigned this Nov 30, 2024

Libienz marked this pull request as draft November 30, 2024 15:06

Libienz added 9 commits December 2, 2024 13:37

test: 사용하지 않는 상수 제거 개선

0b29c38

test: 반복되는 메서드 모킹 setUp으로 이동

a136269

test: 반복되는 메서드 모킹 setUp으로 이동

f63349b

test: 연관 관계를 가지는 Fixture를 주입받아 Request객체를 생성하는 기능 추가

b8aae21

test: TravelogueRequestBuilder 구현

f54079b

test: TravelogueRequestBuilder에서 DayBuilder 체크로직 제거

6b83c22

test: 실패하는 테스트 수정

5b80e2d

test: RequestBuilder 객체 패키지 이동 fixture -> helper

de7e81a

test: TravelPlan관련 fixture에 연관 도메인 관련 내용 제거

d206e14

Libienz added 6 commits December 4, 2024 22:29

test: TravelPlaceTodoFixture 구현

0b51fc3

test: Fixture를 통해서 Request 객체 만드는 기능 구현

72d9350

test: TravelPLanRequestBuilder 구현

f854dec

test: 과거 날짜 여행기 픽스쳐 추가

e23ce11

test: Builder 클래스 개행 수정

99cf179

test: TravelPlanControllerTest에서 Request를 생성 시 RequestBuilder를 이용하도록 개선

2c0198d

Libienz requested review from hangillee, eunjungL, nak-honest and slimsha2dy December 4, 2024 18:18

Merge branch 'develop/be' into feature/be/#602

8e590b0

Libienz marked this pull request as ready for review December 4, 2024 18:19

Libienz commented Dec 4, 2024

View reviewed changes

Libienz linked an issue Dec 4, 2024 that may be closed by this pull request

[Feature] - BE 테스트 개선 1단계 Test Fixture 운용 방식 통일 (Enum Fixture) #602

Closed

4 tasks

Libienz added this to the sprint 8 milestone Dec 4, 2024

hangillee approved these changes Dec 5, 2024

View reviewed changes

eunjungL approved these changes Dec 5, 2024

View reviewed changes

nak-honest approved these changes Dec 5, 2024

View reviewed changes

Libienz mentioned this pull request Dec 5, 2024

[Feature] - 멤버 닉네임 중복을 허용 #616

Closed

1 task

Libienz added 6 commits December 5, 2024 19:34

test: 사용하지 않는 테스트 상수 제거 개선

5b49640

test: TravelogueCountryTest 줄바꿈 컨벤션 적용

f24222f

test: TraveloguePlaceFixture 줄바꿈 컨벤션 적용 및 사용하지 않는 메서드 제거 개선

a9e5902

test: TravelogueRequestBuilder와 TravelogueDayRequestBuilder 별도 클래스로 분리

18a6e65

test: TravelPlanRequestBuilder와 PlanDayRequestBuilder 별도 클래스로 분리

4b9730f

test: MemberFixture 이름 변경 DEFAULT_MEMBER -> TOUROOT_LOCAL_USER

d7ec90f

Libienz merged commit f922a95 into develop/be Dec 5, 2024
3 checks passed

Libienz deleted the feature/be/#602 branch December 5, 2024 13:13


		TRAVELOGUE_PHOTO(1, "https://dev.touroot.kr/temporary/image1.png", TRAVELOGUE_PLACE.get());
		TRAVELOGUE_PHOTO_1(1, "https://dev.touroot.kr/temporary/image1.png"),

		MemberRequest request = VALID_MEMBER.getRequest();
		MemberRequest request = MemberFixture.DEFAULT_MEMBER.getCreateRequest();

		@DisplayName("중복된 이메일을 가진 회원을 생성하려하면 예외가 발생한다.")
		@DisplayName("중복된 닉네임을 가진 회원을 생성하려하면 예외가 발생한다.")

	TRAVELOGUE_PLACE_WITH_NONE_COUNTRY_CODE(1,
	TRAVELOGUE_PLACE_WITH_NONE_COUNTRY_CODE(
	1,

[Feature] - BE 테스트 개선 1단계 Test Fixture 운용 방식 통일 (Enum Fixture) #614

[Feature] - BE 테스트 개선 1단계 Test Fixture 운용 방식 통일 (Enum Fixture) #614

Conversation

Libienz commented Nov 30, 2024 • edited Loading

✅ 작업 내용

🙈 참고 사항

Libienz commented Nov 30, 2024

github-actions bot commented Nov 30, 2024 • edited Loading

Test Results

Libienz commented Dec 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

런타임에 예외가 발생하는 day없이 place를 추가 시도하는 코드

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

hangillee left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Libienz Dec 5, 2024 • edited Loading

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

eunjungL 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

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

nak-honest 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

Libienz commented Dec 5, 2024 • edited Loading

Libienz commented Nov 30, 2024 •

edited

Loading

github-actions bot commented Nov 30, 2024 •

edited

Loading

Libienz Dec 5, 2024 •

edited

Loading

Libienz commented Dec 5, 2024 •

edited

Loading