[Feat] #609 솝탬프 테스트 코드 작성 #622

jher235 · 2025-10-06T12:51:52Z

Related issue 🛠

closes [FEAT] 솝탬프 테스트 코드 작성 #609

Work Description ✏️

Stamp, SoptampUser, Rank, Mission 등 솝탬프 관련 테스트 코드를 작성
기존 테스트 코드의 흐름을 유지하려 함
- 기존과 동일하게 런던형으로 작성
- Repository 테스트 단은 현재 실행되지 않도록 설정되어 있으므로 작성하지 않음
- 기존 테스트 코드에서 BDD Mockito를 사용한 부분은 이를 유지
- SUCCESS_ 등의 prefix 유지
프로덕션 코드는 가능하면 건드리지 않으려 했는데 기존에 MissionService에 존재하는 getMissionById 메서드의 경우 리턴값이 Mission이 아니라 MissionInfo의 Level 값 객체여서 메서드 명을 getMissionLevelById 으로 수정함.
- 해당 코드의 커밋 로그 및 pr 을 확인해보니 특별한 이유가 있는게 아니고 그냥 초창기에 급하게 작성했던 부분인 것 같아서 일단 수정해두었습니다~

Trouble Shooting ⚽️

Related ScreenShot 📷

Uncompleted Tasks 😅

To Reviewers 📢

Deprecated 메서드들은 우선 테스트만 삭제해두었는데 no usages 인 것 확인하고 프로덕션 코드에서도 지울까요?

- StampInfo.Stamp 에 정적 팩토리 메서드를 사용하여 컨버트 책임을 분리 - private 메서드 하단으로 분리 - Stamp의 validate() 는 서비스 테스트에서 제외. 따로 엔티티 테스트 작성 예정

- 참조하는 oldStamp 의 필드 수정값을 기반으로 테스트를 진행하도록 수정

- EditStampRequestWithMissionId fixture 메서드 명 수정

- 기존 getUserInfo 메서드 명을 getSoptampUserInfo 로 수정

- MissionFixture를 따로 두고 사용하도록 분리

@nested

- 테스트 해야할 범위가 꽤 넓다고 판단되어 @nested 로 묶어서 진행

@nested

- 테스트 케이스를 @nested 로 묶어서 진행

@nested

- 테스트 케이스를 @nested 로 묶어서 진행

height · 2025-10-06T12:51:55Z

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

- 기존 getMissionById 에서 getMissionLevelById 로 메서드명을 수정. 반환값과 메서드명이 너무 달라서 헷갈린다고 판단하여 변경함... - 예외처리도 NotFoundException 으로 감싸서 반환하도록 변경. - 테스트 코드 작성 중 가능하면 프로덕션 코드를 건드리지 않으려 했지만,,, 커밋 로그를 트래킹해보니 초창기에 급하게 작성한 코드인 것 같아서 간단하게 수정함.

- 기존 contains 은 순서 보장 테스트를 정상적으로 수행하지 못해서 containsExactly 로 수정

- getter가 전부 존재하는 케이스이므로 메서드 참조로 변경

huncozyboy

고생하셨습니다 !

즐추 🌕

huncozyboy · 2025-10-08T09:43:54Z

src/main/java/org/sopt/app/application/stamp/StampInfo.java

        private LocalDateTime createdAt;
        private LocalDateTime updatedAt;
+
+        public static Stamp from(org.sopt.app.domain.entity.soptamp.Stamp stamp) {


org.sopt.app.domain.entity.soptamp.Stamp처럼 패키지 명을 전부 포함시켜서 Stamp 클래스를 선언해주신 특별한 이유가 있으셨을까요 ??

의도된 설계가 아니라면, FQCN을 직접 사용하는 방식은 통일성이랑 가독성 측면에서 수정해주시는게 좋아보였어요

이게 아마 Stamp 클래스명이 중복으로 사용돼서 컴파일 시 오류가 발생할거에요... 저희 클래스명이 살짝 중구난방이어서... 지금 보시면 리턴 타입도 Stamp고 파라미터 타입도 Stamp인데 둘의 타입이 달라서요....

huncozyboy · 2025-10-08T09:56:05Z

src/test/java/org/sopt/app/application/MissionServiceTest.java

+        // then
+        assertThat(result).containsAnyElementsOf(expectedSortedMissions);
+    }
+
+    @Test
+    @DisplayName("SUCCESS_display true인 미완료 미션만 level 오름차순, title 오름차순으로 조회함")
+    void SUCCESS_getIncompleteMission() {
+        // given
+        final Long userId = 1L;
+        final Mission mission1 = MissionFixture.getMissionWithTitleAndLevel("test1", 2);
+        final Mission mission2 = MissionFixture.getMissionWithTitleAndLevel("test2", 2);
+        final Mission mission3 = MissionFixture.getMissionWithTitleAndLevel("test3", 2);
+        final Mission mission4 = MissionFixture.getMissionWithTitleAndLevel("test4", 1);
+
+        List<Mission> displayedMissions = List.of(mission1, mission2, mission3, mission4);
+        List<Stamp> completedStamps = List.of(
+            SoptampFixture.getStampWithUserIdAndMissionId(userId, mission2.getId()));
+        List<Mission> sortedInCompletedMissions = List.of(mission4, mission1, mission3);
+
+        when(missionRepository.findAllByDisplay(true)).thenReturn(displayedMissions);
+        when(stampRepository.findAllByUserId(userId)).thenReturn(completedStamps);
+        when(missionRepository.findMissionInOrderByLevelAndTitleAndDisplayTrue(
+            argThat((List<Long> missionIds) -> {
+                assertThat(missionIds).containsAnyElementsOf(sortedInCompletedMissions.stream().map(Mission::getId).toList());
+                return true;
+            })))
+            .thenReturn(sortedInCompletedMissions);
+
+        // when
+        List<Mission> result = missionService.getIncompleteMission(userId);
+
+        // then
+        assertThat(result).containsAnyElementsOf(displayedMissions);
+    }


해당 SUCCESS_getCompleteMission은 DisplayName에도 작성해주셨듯이, 미완료 미션만 level 오름차순, title 오름차순으로 조회라는 정렬 보장 이 핵심이라고 생각했습니다

근데 현재 containsAnyElementsOf 방식으로 검증 해주신걸로 이해했는데, 해당 방식은 위에서 작성한대로, 정렬 보장을 의도한대로 정확하게 테스트하지 못할 수도 있겠다고 생각했어요

제가 아는바에 의하면 containsAnyElementsOf는 원소 집합 동일성만 확인하고 순서를 보장해주지 않는걸로 알고있었어서
아래 내용을 테스트하는게 중요하지않을까라는 개인적인 생각이 들었습니다

1차 키: level ASC
2차 키: title ASC

저는 아래처럼 튜플을 활용해서 검증해주는 방식을 생각했는데, 해당 방식에 대해서도 어떻게 생각하시는지도 궁금하네요 !

assertThat(result).extracting(Mission::getLevel, Mission::getTitle).containsExactly(tuple(...), tuple(...))

앗 이거는 제가 복붙하다가 실수한 것 같네요 검증 시 사용하려고 sortedInCompletedMissions 를 만들어둔건데 이걸 사용하는 쪽으로 변경하겠습니다 👍

huncozyboy · 2025-10-08T09:59:19Z

src/test/java/org/sopt/app/application/MissionServiceTest.java

+    @Test
+    @DisplayName("FAIL_미션 Id로 미션을 찾을 수 없을 경우 정상적으로 예외를 반환함")
+    void FAIL_getMissionLevelById_whenNotFoundMission() {
+        // given
+        final Long anyMissionId = anyLong();


해당 내용은 필수는 아닐거같긴한데, final Long anyMissionId = anyLong();처럼 matcher를 값 변수로 저장해 사용하는 부분이 보였습니다

제가 아는 바에 의하면 matcher는 stub/verify 호출의 파라미터로만 쓰는 게 안전하다고 알고있었지만, 여기서는 단순한 임의값이 필요한거니까

long missionId = 999L; when(missionRepository.findById(anyLong())).thenReturn(Optional.empty());

처럼 구체값 + anyLong() 조합을 활용해주는건 어떨까요

저도 처음 작성할 땐 그렇게 생각했어서 현재 pr의 다른 테스트들에선 아마 Long 타입을 직접 줄텐데 이게 큰 의미가 있나??라는 생각도 들더라구요. 우선 통일을 위해서라도 직접 주입하는 쪽으로 수정하겠습니다~ 그런데 이렇게 값을 명시해서 전달하더라도 파라미터가 anyLong이라면 어떤 이점이 있다고 생각하세요?

huncozyboy · 2025-10-08T10:01:11Z

src/test/java/org/sopt/app/facade/SoptampFacadeTest.java

+import static org.mockito.BDDMockito.given;
+import static org.mockito.BDDMockito.then;


PR 설명대로 BDD 사용을 유지하려는 의도가 좋아 보인다고 생각하긴했는데, 일부 테스트가 when/verify, 일부는 given/then이 혼용 되어있는 형식이여서,

개인적인 생각으로는 한 클래스 내에서는 BDDMockito의 given/when/then으로 통일하는게 좋지않을까하는 생각이 들었는데 어떻게 생각하실까요 ?

제가 그냥 기존 코드를 최대한 재사용하려고 하다보니 이런 부분에 통일성이 좀 부족하네요,..
혹시 BDDMockito 를 사용하는게 더 좋다고 생각하세요?? 사실 저는 원래 Mockito를 먼저 접했어서 그런지 when() 메서드를 사용해도 이해하기 어렵다는 생각은 별로 못했었거든요.

제가 기존에 BDDMockito 를 사용했던 부분을 그대로 유지한건 이런 부분들을 완전히 통일하면서 가는 것보다 그냥 기존 코드를 재사용하면서 빠르게 테스트코드를 작성해놓는게 더 좋겠다는 생각이었어요. 개인적으로 어짜피 룰을 정한다면 그냥 저희끼리 Mockito 혹은 BDDMockito 로 아예 통일하는게 더 좋다고 생각하는데... Mockito로 통일하는건 어떨까요? 이유는 Mockito를 사용하는 부분도 크게 헷갈리지 않다고 생각하고 기존 코드에서 Mockito 비중이 더 많았기 때문입니다.
물론 BDDMockito가 더 좋다고 생각하시는 분이 계신다면 편하게 말씀해주세요~~

huncozyboy · 2025-10-08T10:02:51Z

src/test/java/org/sopt/app/application/MissionServiceTest.java

+        when(stampRepository.findAllByUserId(userId)).thenReturn(completedStamps);
+        when(missionRepository.findAllByDisplay(true)).thenReturn(displayedMissions);


해당 부분이 맨아래 given/when/then로 통일해주는게 좋지않을까라고 생각했던 부분입니다

given(stampRepository.findAllByUserId(userId)).willReturn(completedStamps); given(missionRepository.findAllByDisplay(true)).willReturn(displayedMissions);

처럼 바꿔줄 수 있을거같다고 생각했어요. 더 좋은 의견이 있으시다면 알려주세용

+SoptampUserFinderTest에서 Mockito.when으로 선언해주신 부분도 동일합니다 !

huncozyboy · 2025-10-08T10:04:39Z

src/test/java/org/sopt/app/application/MissionServiceTest.java

+    @Test
+    @DisplayName("SUCCESS_완료한 미션만 level 오름차순, title 오름차순으로 조회함")
+    void SUCCESS_getCompleteMission() {
+        // given
+        final Long userId = 1L;
+
+        final Mission mission1 = MissionFixture.getMissionWithTitleAndLevel("test1", 3);
+        final Mission mission2 = MissionFixture.getMissionWithTitleAndLevel("test2", 2);
+        final Mission mission3 = MissionFixture.getMissionWithTitleAndLevel("test3", 2);
+
+        List<Mission> expectedSortedMissions = List.of(mission2, mission3, mission1);
+        List<Stamp> completedStamps = List.of(
+            SoptampFixture.getStampWithUserIdAndMissionId(userId, mission1.getId()),
+            SoptampFixture.getStampWithUserIdAndMissionId(userId, mission2.getId()),
+            SoptampFixture.getStampWithUserIdAndMissionId(userId, mission3.getId())
+        );
+
+        when(stampRepository.findAllByUserId(userId)).thenReturn(completedStamps);
+        when(missionRepository.findMissionInOrderByLevelAndTitle(
+            List.of(mission1.getId(), mission2.getId(), mission3.getId())))
+            .thenReturn(expectedSortedMissions);
+
+        // when
+        List<Mission> result = missionService.getCompleteMission(userId);
+
+        // then
+        assertThat(result).containsAnyElementsOf(expectedSortedMissions);
+    }


이건 메서드 컨벤션 관련한 내용인데, “get vs find vs load”가 섞여있는 방식으로 구현해주신걸로 이해했어요

재헌님이 생각해주신 구분 기준이 있으셨다면 공유해주시면 더 좋을거같아요

“get vs find vs load”가 섞여있는 방식으로 구현해주신걸로 이해했어요
-> 혹시 이 부분에서 load가 어떤 걸까요?? 메서드 컨벤션 자체는 제가 딱히 관여하지 않고 기존 방식을 사용했는데, repository 단에서는 jpa 자동 생성 메서드로 find를 사용하고 service에서는 get 을 많이 사용하셨구나~ 정도로만 생각하고 있었습니다.

huncozyboy · 2025-10-08T10:11:52Z

src/main/java/org/sopt/app/application/mission/MissionService.java

-    public MissionInfo.Level getMissionById(Long missionId) {
+    public MissionInfo.Level getMissionLevelById(Long missionId) {


꼼꼼한 변수명 수정 좋네요 👍🏻

huncozyboy · 2025-10-08T10:19:14Z

src/test/java/org/sopt/app/common/fixtures/MissionFixture.java

+public class MissionFixture {
+
+    private static final AtomicLong idGenerator = new AtomicLong(1L);


MissionFixture에서 AtomicLong idGenerator는 전역 정적 상태라 테스트 간 순서 의존성이 생길 수도 있지않을까 하는 걱정이 들었습니다.
idGenerator가 전역(static) 가변 상태라서, 테스트 실행 순서라던지, 방식에 따라서 값이 달라질 수도 있으니까요

제가 개인적으로 생각한 방식은 Fixture에 전역 카운터 자체는 유지하되, 테스트마다 리셋해서 독립 실행성을 보장하면 더 안전하지 않을까하는 생각이 있었어요.

private static final AtomicLong idGenerator = new AtomicLong(1L); public static void resetIds() { idGenerator.set(1L); }

해당 방식으로 작성해주면 final이라도 AtomicLong#set은 가능하니까 객체 교체 없이 값만 초기화할 수 있다고 생각했는데, 관련해서도 어떻게 생각하시는지 궁금했습니다 !

idGenerator를 사용하는 것 자체가 외부에서 id값 생성에 관여하지 않아야 한다고 생각했어요. 당연히 테스트도 따로 객체에 id값을 주입하지 않고 생성된 객체의 id값을 사용하도록 테스트를 작성해야할테니 이렇게 초기화하는 부분의 필요성이 잘 와닿지 않는 것 같아요.
개인적인 생각으로는 애초에 idGenerator를 사용하는 부분에서 테스트 독립성을 위해 매번 id 초기화를 해주는 부분 없이 어떤 id 값이 오더라도 해당 값으로 통과하는 테스트여야 한다고 생각해요. 특히 지금처럼 내부 메서드를 대부분 모킹하는 테스트에서는요...
혹시 그럼에도 매번 초기화하는게 좋다고 생각하세요??

huncozyboy · 2025-10-08T10:22:23Z

src/test/java/org/sopt/app/application/SoptampUserFinderTest.java

+        Mockito.when(soptampUserRepository.findUserByNickname(soptampUser.getNickname()))
+            .thenReturn(Optional.of(soptampUser));


여기도 BDDMockito로 해줄 수 있을거같네요

given(soptampUserRepository.findUserByNickname(soptampUser.getNickname())) .willReturn(Optional.of(soptampUser));

jher235 added 30 commits September 29, 2025 14:25

[#609] feat: SoptampUserFixture 복원 및 개선

d246ea1

[#609] feat: SoptampFixture 주석 해제

dd7d027

[#609] feat: SoptampFixture 주석 해제 및 Stamp 생성 메서드 추가

ea777c6

[#609] feat: Repository 단 통합 테스트 환경 분리

3886b9c

[#609] feat: findAllByUserId 테스트 작성

1389e16

[#609] feat: findByUserIdAndMissionId 테스트 작성

164a9ca

[#609] feat: UserFixture 중앙인증화로 수정된 유저에 맞게 수정

1e263b9

[#609] feat: deleteAllByUserId 테스트 작성

9ea0a73

[#609] feat: findByIdAndUserId 테스트 작성

ac21f26

[#609] feat: findByNickname 테스트 작성

a9ca767

[#609] feat: findAllByPartAndCurrentGeneration 테스트 작성

bb3a25e

[#609] feat: findById 테스트 작성

62c2da8

[#609] feat: findStamp 테스트 작성

27e6884

- StampInfo.Stamp 에 정적 팩토리 메서드를 사용하여 컨버트 책임을 분리 - private 메서드 하단으로 분리 - Stamp의 validate() 는 서비스 테스트에서 제외. 따로 엔티티 테스트 작성 예정

[#609] feat: uploadStamp 메서드 테스트 작성

c1c4717

[#609] feat: editStampContents 메서드 테스트 작성

70a45cb

- 참조하는 oldStamp 의 필드 수정값을 기반으로 테스트를 진행하도록 수정

[#609] feat: checkDuplicateStamp 메서드 테스트 작성

e590a8c

[#609] feat: deleteStampById 메서드 테스트 작성

0363f18

[#609] feat: deleteAllStamps 메서드 테스트 작성

9841d43

[#609] feat: getMissionIdByStampId 메서드 테스트 작성

f3bcab4

[#609] feat: deprecated 된 메서드 테스트 삭제

fec8b3b

[#609] feat: getSoptampUserInfo 메서드 테스트 작성

63bab17

[#609] feat: editProfileMessage 메서드 테스트 작성

f468456

[#609] feat: addPointByLevel 메서드 테스트 작성

9f5afdb

[#609] feat: subtractPointByLevel 메서드 테스트 작성

9c48d63

[#609] feat: initPoint 메서드 테스트 작성

7ddf250

[#609] feat: upsertSoptampUser 메서드 테스트 작성

a04a57b

[#609] feat: SoptPart Enum 네이밍 변경 반영

1256f10

[#609] feat: uploadStamp 메서드 테스트 작성

17c9a63

[#609] feat: editStamp 메서드 테스트 작성

c2a5674

- EditStampRequestWithMissionId fixture 메서드 명 수정

[#609] feat: deleteStamp 메서드 테스트 작성

9b9dce8

jher235 added 9 commits October 4, 2025 00:42

[#609] feat: editSoptampUserProfileMessage 메서드 테스트 작성

f05bedb

[#609] feat: getStampInfo 메서드 테스트 작성

e7858a7

- 기존 getUserInfo 메서드 명을 getSoptampUserInfo 로 수정

[#609] feat: findSoptampUserAndCompletedMissionByNickname 메서드 테스트 작성

7ee548d

- MissionFixture를 따로 두고 사용하도록 분리

[#609] chore: 불필요한 주석 및 import 문 삭제

7d133e6

[#609] feat: findCurrentRanks 메서드 테스트 작성

7c3c371

[#609] feat: findCurrentRanksByPart 메서드 테스트 작성

3d43c31

[#609] feat: findAllPartRanks 메서드 테스트 작성

5b85118

- 테스트 해야할 범위가 꽤 넓다고 판단되어 @nested 로 묶어서 진행

[#609] feat: findPartRank 메서드 테스트 작성

c9d5cbd

- 테스트 케이스를 @nested 로 묶어서 진행

[#609] feat: findUserRank 메서드 테스트 작성

484e6ef

- 테스트 케이스를 @nested 로 묶어서 진행

jher235 requested review from huncozyboy and hyerinhwang-sailin October 6, 2025 12:51

jher235 self-assigned this Oct 6, 2025

jher235 added the ✨ Feat 새로운 피쳐 생성 label Oct 6, 2025

pull-request-size bot added the size/XXL label Oct 6, 2025

jher235 added 13 commits October 7, 2025 00:04

[#609] feat: 캐시 관련 Fixture 추가

52e5b28

[#609] feat: FindCurrentRanks 테스트 의 캐시 히트, 미스 여부에 따른 테스트 분리

7e858dd

[#609] feat: FindUserRank 테스트 의 캐시 히트 여부에 따른 테스트 분리

27d43ad

[#609] feat: 메서드 명에서 파라미터를 구체적으로 명시하도록 수정

bcd096c

[#609] feat: findAllMission 메서드 테스트 작성

ccf5ac1

[#609] feat: uploadMission 메서드 테스트 작성

94e6281

[#609] feat: getCompleteMission 메서드 테스트 작성

9c105f0

[#609] feat: getIncompleteMission 메서드 테스트 작성

2339409

[#609] feat: getMissionLevelById 메서드 테스트 작성

7af650c

[#609] feat: validate 메서드 테스트 작성

bb37bfe

[#609] fix: 솝탬프 포인트 랭킹 파트 순서 테스트 수정

eb5b46d

- 기존 contains 은 순서 보장 테스트를 정상적으로 수행하지 못해서 containsExactly 로 수정

[#609] refactor: extracting 의 파라미터 부분을 메서드 참조로 변경

bc598a7

- getter가 전부 존재하는 케이스이므로 메서드 참조로 변경

huncozyboy reviewed Oct 8, 2025

View reviewed changes

[#609] feat: getIncompleteMission 검증 로직 수정

d77d9f7

		import static org.mockito.BDDMockito.given;
		import static org.mockito.BDDMockito.then;

		when(stampRepository.findAllByUserId(userId)).thenReturn(completedStamps);
		when(missionRepository.findAllByDisplay(true)).thenReturn(displayedMissions);

		public MissionInfo.Level getMissionById(Long missionId) {
		public MissionInfo.Level getMissionLevelById(Long missionId) {

		public class MissionFixture {

		private static final AtomicLong idGenerator = new AtomicLong(1L);

		Mockito.when(soptampUserRepository.findUserByNickname(soptampUser.getNickname()))
		.thenReturn(Optional.of(soptampUser));

[Feat] #609 솝탬프 테스트 코드 작성 #622

Are you sure you want to change the base?

[Feat] #609 솝탬프 테스트 코드 작성 #622

Conversation

jher235 commented Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related issue 🛠

Work Description ✏️

Trouble Shooting ⚽️

Related ScreenShot 📷

Uncompleted Tasks 😅

To Reviewers 📢

Uh oh!

height bot commented Oct 6, 2025

Uh oh!

huncozyboy left a comment

Choose a reason for hiding this comment

Uh oh!

huncozyboy Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jher235 Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jher235 commented Oct 6, 2025 •

edited

Loading

huncozyboy Oct 8, 2025 •

edited

Loading

jher235 Oct 8, 2025 •

edited

Loading