[TEST] DTO Null, Blank E2E Test by unifolio0 · Pull Request #174 · debate-timer/debate-timer-be

unifolio0 · 2025-05-16T06:44:21Z

🚩 연관 이슈

closed #162

🗣️ 리뷰 요구사항 (선택)

github-actions · 2025-05-16T06:46:18Z

Test Results

67 files 67 suites 11s ⏱️
191 tests 191 ✅ 0 💤 0 ❌
203 runs 203 ✅ 0 💤 0 ❌

Results for commit 8fcc4b0.

♻️ This comment has been updated with latest results.

github-actions · 2025-05-16T06:46:20Z

📝 Test Coverage Report

Overall Project	83.94% `-0.08%`	🍏
Files changed	0%	❌

File	Coverage
MemberController.java	100%	🍏
NotifierConfig.java	53.85%	🍏
ClientLoggingAspect.java	5.98%	🍏
DiscordNotifier.java	0% `-3.08%`	❌

coli-geonwoo

/noti

복잡하고 더러워질 수 있었던 로직을 최대한 깔끔하게 정리한 느낌이 느껴져요.

다만 커찬 의견도 보고 의견 더 남기고자 일단 RC로 남겨둡니다!

coli-geonwoo · 2025-05-16T07:02:28Z

src/main/java/com/debatetimer/client/notifier/DiscordNotifier.java

        try {
            return JDABuilder.createDefault(token).build().awaitReady();
        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();


[질문 ✋ ]

이거 interrupt 왜 넣었는지 설명좀 부탁합니다..

InterruptedException 자체가 다른 스레드에서 자신의 스레드에게 "인터럽트 신호를 보낸 것"인데, 왜 자신이 자기에게 인터럽트를 또 다시 거는 것이죠?

회의 내용에 따라 그대로 유지하겠습니다

coli-geonwoo · 2025-05-16T07:03:34Z

src/main/java/com/debatetimer/config/NotifierConfig.java


 public class NotifierConfig {

+    private NotifierConfig() {


[제안]

롬복을 최대한 활용해서 NoArgsConstructor(access = PRIVATE)로 표기하는 건 어떤가요?

반영했습니다

coli-geonwoo · 2025-05-16T07:04:34Z

src/test/java/com/debatetimer/fixture/BlankArgumentsProvider.java

+import org.junit.jupiter.params.provider.ArgumentsProvider;
+
+public class BlankArgumentsProvider implements ArgumentsProvider {
+    @Override


컨벤션 개행 부탁합니당

Suggested change

@Override

@Override

반영했습니다

coli-geonwoo · 2025-05-16T07:10:10Z

src/test/java/com/debatetimer/fixture/NullAndEmptyAndBlankSource.java

+@NullSource
+@EmptySource
+@ArgumentsSource(BlankArgumentsProvider.class)
+public @interface NullAndEmptyAndBlankSource {


[제안 2가지]

ElementType.ANNOTATION_TYPE은 어노테이션에 붙을 수 있는 걸 의미하는 걸로 아는데, 크게 쓰이는 곳이 없어보여서 METHOD로만 선정해도 될 것 같은데 의도가 있는 부분인지 궁금해요!

@NullAndEmptySource로 어노테이션 하나로 합치는 건 어떤가유

개행 컨벤션 부탁드립니당

반영했습니다

[제안] @NullAndEmptyAndBlankSource를 붙여 쓰는 것보다, @NullAndEmptySource, @BlankSources 형식으로 따로 쓰는게 가독성이 더 좋지 않을까요?

저는 어노테이션이 많아질수록 가독성을 해친다고 생각해서 통합했는데 만약 정말 분리하길 원한다면 분리하겠습니다.

coli-geonwoo · 2025-05-16T07:12:34Z

src/test/java/com/debatetimer/repository/customize/CustomizeTimeBoxRepositoryTest.java

            CustomizeTimeBox chanBox2 = customizeTimeBoxGenerator.generate(chanTable, CustomizeBoxType.NORMAL, 2);
-            CustomizeTimeBox bitoBox1 = customizeTimeBoxGenerator.generate(bitoTable, CustomizeBoxType.NORMAL, 2);
-            CustomizeTimeBox bitoBox2 = customizeTimeBoxGenerator.generate(bitoTable, CustomizeBoxType.NORMAL, 2);
+            customizeTimeBoxGenerator.generate(bitoTable, CustomizeBoxType.NORMAL, 2);


사용되지 않는 변수의 경우 선언하지 않는 군요! 저는 이름을 붙여주면 테스트 맥락 파악이 더. 쉬워질 때도 있는 것 같긴 한데, 취향차리라 생각하긴 합니다.

coli-geonwoo · 2025-05-16T07:30:59Z

src/test/java/com/debatetimer/controller/customize/CustomizeControllerTest.java

+        }
+
+        private CustomizeTableCreateRequest createCustomizeTableCreateRequest(String field, Object value) {
+            String name = field.equals("name") ? (String) value : "자유 테이블";


문자열 기반 필드 매핑 <- 이거 좋은 토론거리인데요 ㅋㅋ

커찬 vs 비토-콜리?

아니... 이럴꺼면 콜리가 이야기 한 fixtureMonkey 사용하는 게 더 나을 것 같은데요?

반영 했습니다

leegwichan

/noti @unifolio0
일단 리뷰 남겼습니다~ 오늘 회의 때 더 이야기해보죠

leegwichan · 2025-05-16T13:36:06Z

src/main/java/com/debatetimer/client/notifier/DiscordNotifier.java

        try {
            return JDABuilder.createDefault(token).build().awaitReady();
        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();


InterruptedException 자체가 다른 스레드에서 자신의 스레드에게 "인터럽트 신호를 보낸 것"인데, 왜 자신이 자기에게 인터럽트를 또 다시 거는 것이죠?

leegwichan · 2025-05-16T13:37:21Z

src/main/java/com/debatetimer/config/NotifierConfig.java

 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Profile;

+@NoArgsConstructor(access = lombok.AccessLevel.PRIVATE)


leegwichan · 2025-05-16T13:40:47Z

src/test/java/com/debatetimer/fixture/NullAndEmptyAndBlankSource.java

+@NullSource
+@EmptySource
+@ArgumentsSource(BlankArgumentsProvider.class)
+public @interface NullAndEmptyAndBlankSource {


[제안] @NullAndEmptyAndBlankSource를 붙여 쓰는 것보다, @NullAndEmptySource, @BlankSources 형식으로 따로 쓰는게 가독성이 더 좋지 않을까요?

leegwichan · 2025-05-16T13:43:27Z

src/test/java/com/debatetimer/controller/customize/CustomizeControllerTest.java

+        }
+
+        private CustomizeTableCreateRequest createCustomizeTableCreateRequest(String field, Object value) {
+            String name = field.equals("name") ? (String) value : "자유 테이블";


아니... 이럴꺼면 콜리가 이야기 한 fixtureMonkey 사용하는 게 더 나을 것 같은데요?

leegwichan · 2025-05-16T13:44:03Z

src/test/java/com/debatetimer/controller/customize/CustomizeControllerTest.java

+        @ParameterizedTest
+        @NullAndEmptyAndBlankSource
+        void 사용자_지정_테이블을_생성할때_테이블_이름은_개행문자_외_다른_글자가_포함되야한다(String tableName) {
+            CustomizeTableCreateRequest request = createCustomizeTableCreateRequest("name", tableName);
+
+            ErrorResponse errorResponse = sendCustomizeTableCreateRequest(request, HttpStatus.BAD_REQUEST)
+                    .extract().as(ErrorResponse.class);
+
+            assertThat(errorResponse.message()).isEqualTo(ClientErrorCode.FIELD_ERROR.getMessage());
+        }


테스트가 전반적으로 중복되는 부분이 많다고 느껴지는데... 이 중복을 줄일 만한 대안이 없을까?

그 부분을 고민 안해본 건 아니지만 지금 구조가 각 줄이 given when then을 구성하고 있기에 더 줄이는 건 오히려 가독성을 해친다고 판단했습니다.

unifolio0 · 2025-05-17T06:43:54Z

/noti
전부 반영 완료

coli-geonwoo

/noti

비토.. 역시 빠르게 작업해주셨네유.. 작업 내용에서 간단히 2가지 질문 남겼습니다!

coli-geonwoo · 2025-05-17T11:03:56Z

src/test/java/com/debatetimer/controller/customize/CustomizeControllerTest.java

+        @ParameterizedTest
+        void 사용자_지정_테이블을_생성할때_테이블_주제는_null이_올_수_없다(String agenda) {
+            CustomizeTableCreateRequest request = getCustomizeTableCreateRequestBuilder()
+                    .set("info.agenda", agenda)


오... 이게 .으로 타고 들어갈 수도 있구나.. 알아갑니다 👍

coli-geonwoo · 2025-05-17T11:10:33Z

src/test/java/com/debatetimer/controller/BaseControllerTest.java

    }
+
+    protected ArbitraryBuilder<CustomizeTableCreateRequest> getCustomizeTableCreateRequestBuilder() {
+        FixtureMonkey fixtureMonkey = FixtureMonkey.builder()


[제안]

fixtureMonkey 생성하는 부분 중복되는 만큼 메서드로 빼도 괜찮을 것 같아요.

수정했습니다.

coli-geonwoo · 2025-05-17T11:13:30Z

src/test/java/com/debatetimer/controller/customize/CustomizeControllerTest.java

-            );
-            Headers headers = headerGenerator.generateAccessTokenHeader(bito);
+            CustomizeTableCreateRequest request = getCustomizeTableCreateRequestBuilder()
+                    .set("table[1].speaker", null)


[질문]

table 배열에 인덱스 1번에 해당하는 객체가 없는데 이거 혹시 어떤 의도인가요? 제가 놓친 부분이 있을 것 같아서요. 실제로 없애도 잘 테스트가 통과해서 궁금합니다.

뭐야 저거 왜저래... 아니 테스트를 실패시키는게 아니라 걍 넘어가네요. 아... 다시 fixtureMonkey 전으로 돌리고 싶은 마음이 들지만 일단 넘어가겠습니다.

coli-geonwoo · 2025-05-17T11:14:50Z

src/test/java/com/debatetimer/controller/customize/CustomizeControllerTest.java

-                    )
-            );
            Headers headers = headerGenerator.generateAccessTokenHeader(bito);
+            CustomizeTableCreateRequest renewTableRequest = getCustomizeTableCreateRequestBuilder()


여기도 같은 질문입니다.

수정했습니다.

unifolio0 · 2025-05-17T11:25:53Z

/noti
@coli-geonwoo 수정했습니다

coli-geonwoo

/noti

반영감사합니다. approve 하겠슴다

leegwichan

/noti @unifolio0
비토 고생 많았구여~ Approve 드리겠습니다.
대신에 의견 몇 개 남겼으니, 확인 부탁해요!

leegwichan · 2025-05-18T01:31:30Z

src/test/java/com/debatetimer/controller/BaseControllerTest.java

+    protected ArbitraryBuilder<CustomizeTableCreateRequest> getCustomizeTableCreateRequestBuilder() {
+        return fixtureMonkey.giveMeBuilder(CustomizeTableCreateRequest.class)
+                .set("info", getCustomizeTableInfoCreateRequestBuilder().sample())
+                .set("table", getCustomizeTimeBoxCreateRequestBuilder().sampleList(2));
+    }


여기 포지션에 있는게 마음에 안들 수 있지만, 해당 메서드가 많아지면 Generator 등으로 분리 해보죠

음... 그때 가서 토의해봐야겠지만 전 fixtreMonkey에 대해선 Generator를 반대할 것 같아요

leegwichan · 2025-05-18T01:33:42Z

src/test/java/com/debatetimer/controller/BaseControllerTest.java

Member 관련 API는 왜 안했나요? ex. MemberCreateRequest

저것도 있었군요, 추가했습니다

unifolio0 · 2025-05-18T01:49:47Z

/noti
@debate-timer/backend
null, blank e2e 테스트가 필요한 이유
8fcc4b0

unifolio0 added 2 commits May 16, 2025 11:03

refactor: 사용하지 않는 변수 제거

ff281fc

test: DTO Null, Blank E2E Test

7794960

unifolio0 added the test 테스트 label May 16, 2025

unifolio0 requested a review from coli-geonwoo May 16, 2025 06:44

unifolio0 self-assigned this May 16, 2025

unifolio0 requested a review from leegwichan May 16, 2025 06:44

coli-geonwoo requested changes May 16, 2025

View reviewed changes

unifolio0 added 2 commits May 16, 2025 16:41

refactor: 불필요한 타겟 제거

94bf468

refactor: 롬복 활용

75986d8

leegwichan requested changes May 16, 2025

View reviewed changes

refactor: FixtureMonkey 활용

d50e522

coli-geonwoo requested changes May 17, 2025

View reviewed changes

unifolio0 added 2 commits May 17, 2025 20:21

fix: 사이즈 수정

271979d

refactor: 중복 제거

c2b7513

coli-geonwoo approved these changes May 17, 2025

View reviewed changes

leegwichan approved these changes May 18, 2025

View reviewed changes

unifolio0 added 2 commits May 18, 2025 10:43

refactor: 메소드 명 변경

2bd1974

refactor: 멤버 생성 실패 케이스 테스트 추가

8fcc4b0

unifolio0 merged commit 09d22f6 into develop May 18, 2025
3 checks passed

unifolio0 deleted the test/#162 branch May 18, 2025 01:51

Conversation

unifolio0 commented May 16, 2025

🚩 연관 이슈

🗣️ 리뷰 요구사항 (선택)

Uh oh!

github-actions bot commented May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

github-actions bot commented May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📝 Test Coverage Report

Uh oh!

coli-geonwoo left a comment

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!

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!

leegwichan left a comment

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!

unifolio0 commented May 17, 2025

Uh oh!

coli-geonwoo left a comment

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!

github-actions bot commented May 16, 2025 •

edited

Loading

github-actions bot commented May 16, 2025 •

edited

Loading