[BE-FEAT] 정합성 확인을 통한 테스트 정상화 및 데이터 검증 테스트 작성 #370

jinchiim · 2024-10-16T07:24:52Z

🍄 PR 확인 사항

PR이 다음 요구 사항을 충족하는지 확인하세요. :

API 명세서가 업데이트 혹은 작성이 되어 있나요?

현재 작업은 어떤 이슈를 해결한 것인지 설명해주세요.

Issue Number: #364

검증 테스트 작성
테스트 전부 정상화

기존 코드에서 변경된 점이 있다면 설명해주세요. (추가 X)

딱히 변경된 점은 없고 별 별 테스트를 다 짰습니다.
놓친게 있으면 전달해주세요~!

+) 일부러 테스트가 틀리면 어디가 틀린지 알게 하기 위해 isContains 로 검증이 아닌 불일치하는 id 를 가져와 empty 를 확인하도록 작성했습니다.

틀리면 이런식으로 알려줘요 그걸로 찾으시면 됩니다. 🫡

지금은 포케로그와의 정합성 문제인 2가지 빼고는 전부 정상적으로 돌아갑니다!

jongmee

폴라 고생하셨어요! 저는 빠뜨린 부분을 찾지 못했습니다 👍

궁금한 점 하나 있어서 그것만 리뷰 달았습니다 :)

backend/pokerogue/src/test/java/com/pokerogue/data/pattern/DataPattern.java

backend/pokerogue/src/test/java/com/pokerogue/data/BiomeDataTest.java

jongmee · 2024-10-17T06:14:32Z

backend/pokerogue/src/main/java/com/pokerogue/helper/data/PokemonValidator.java

@@ -17,7 +17,7 @@

 class PokemonValidator {


폴라가 생성한 클래스는 아니지만! 궁금해서 달아봅니다 ☺️

폴라가 생각하는 PokemonValidator의 역할은 무엇인가요? 데이터 정합성 파악을 위해 꼭 필요한 클래스지만 테스트 패키지가 아닌 메인 패키지에 있어야 할까요?? PokemonValidator를 테스트 코드를 제외하고 쓰지 않는 것 같아서 달아봅니다 ! 나중에 필요한 기능이 있으려나요 🤔

다른 분들의 의견도 궁금합니다!!

저도 비슷한 생각입니다!☺️
다른 도메인들과는 달리 Pokemon에 대한 테스트가 분산되어 있네요. 각 테스트들을 하나의 클래스에 모아두고 다른 도메인처럼 테스트 패키지에 위치하는 게 좋을 것 같아요!☺️☺️

저는 메인 패키지에 있어야 한다고 생각합니다!
제가 PokemonValidator을 구현했을 때는 포켓몬에 대한 정보를 관리하고, 검증하는 클래스로 생각하고 구현했습니다.
그래서 테스트코드에서 직접 검증 과정을 거치기보다는, 검증할 메서드를 클래스 안에 정의하고 테스트코드에서 재사용했습니다.

예를 들면, PokemonDataTest에서는 PokemonValidator를 사용해서 실제 데이터가 정확한지 검증을합니다.
포켓몬 수가 늘어난다면, PokemonDataTest를 고칠 필요 없이 PokemonValidator안에 있는 포켓몬 메타정보만 수정하면 테스트가 통과합니다.
이를 통해 깨지는 테스트를 줄일 수 있고, 데이터 변화의 관리나 추적이 쉬워질 것이라고 생각했어요.
PokemonValidatorTest에서는 데이터와 상관 없는 검증 메서드에 대한 테스트를 하기도 합니다.
(지금은 두개밖에 없지만..)

저희의 코드 구조를 조금만 바꾸어 검증 책임을 각 엔티티 혹은 도메인이 갖게 하면 자연스럽게 PokemonValidator는 메인 패키지에 있게 되지 않을까요??

지금은 PokemonValidator가 생각보다 중요한 역할을 하고 있기 때문에 메인 패키지에 있는 것이 좋아보여요!
(제 의견일 뿐입니다~!)

생각해보니까 테스트에서는 데이터 제한이 엄격한데 실제 프로덕션 코드는 제한이 없는것이 조금 이상하긴 하네요 ㅋㅋㅋ
이제 엔티티 말고 각자의 데이터를 검증하는 도메인을 만들어야 할까요?

내일 얘기해보아요

도메인 만드는거 개인적으로 굿..

unifolio0

역시 항상 폴라의 코드는 읽기 쉽다는 생각이 들게 만드네요!
덕분에 제법 변경사항이 있어도 편하게 읽을 수 있었어요! ☺️☺️

개인적인 생각 몇가지 남기고 갑니다. 구현 수고하셨어요! 👍

unifolio0 · 2024-10-17T12:06:42Z

backend/pokerogue/src/main/java/com/pokerogue/helper/data/PokemonValidator.java

@@ -17,7 +17,7 @@

 class PokemonValidator {


저도 비슷한 생각입니다!☺️
다른 도메인들과는 달리 Pokemon에 대한 테스트가 분산되어 있네요. 각 테스트들을 하나의 클래스에 모아두고 다른 도메인처럼 테스트 패키지에 위치하는 게 좋을 것 같아요!☺️☺️

unifolio0 · 2024-10-17T12:19:27Z

backend/pokerogue/src/test/java/com/pokerogue/data/MoveDataTest.java

+    }
+
+    @Test
+    @DisplayName("Move의 아이디는 영어 소문자와 단일 _로만 이루어져 있다.")


단일 _라는게 연속되지 않은 _를 의미하는 걸까요? 리소스가 클 것 같아 필수는 아니지만 통일하면 좋을 것 같아요. Biome의 native Pokemon 속 pokemonId들이 전부 존재한다와 Move의 PokemonId 들은 Pokemon Collection 에 존재한다.이런 표현 같은 것들을요.

추가로 Move의 아이디에서 _는 연속되는 게 있었던 것 같아요! 정규식은 맞는데 DisplayName을 수정하면 좋을 것 같아요!☺️☺️

아이디의 구분자가 _이기 때문에 연속되면 안됩니다!

Move의 아이디를 살펴봐야겠네요.

unifolio0 · 2024-10-17T12:24:49Z

backend/pokerogue/src/test/java/com/pokerogue/data/pattern/DataPattern.java

+
+import java.util.regex.Pattern;
+
+public enum DataPattern {


이건 단순히 의견입니다!☺️☺️

폴라가 데이터가 안 맞는게 있다고 찾아올 때는 잘 몰랐는데 이번 테스트들을 보면서 과연 우리가 포케로그 개발자가 지정한 형식을 유추해서 테스트를 하는 게 맞을까라는 생각이 들어요.

제가 생각했던 테스트는 어플을 실행함에 있어 추출한 데이터가 문제를 발생시키지 않는 지 테스트한다 정도였거든요. 그래서 Move가 들고 있는 pokemonId가 실제로 존재한다, 추출한 id의 형식이 우리가 정한 컨벤션에 일치한다. 이 정도만 테스트 하는 걸 생각했어요.

지금 테스트는 포케로그 개발자가 맘먹고 이제 포켓몬에서도 _를 연속으로 쓰게 해야지 하면 바로 깨질 것 같아요. 포케로그 개발자의 생각에 따라 얼마든지 깨질 수 있는 테스트가 처음에 데이터 관련 테스트를 짜자고 했을 때 생각했던 테스트였나요?

일리가 잇어요 내일 다같이 얘기해보죠!

단축키가 안먹혀서 이모지는 사진으로 대체합니다

의견)
예시겠지만

포케로그 개발자가 맘먹고 이제 포켓몬에서도 _를 연속으로 쓰게 해야지

이런 경우를 대비해서 테스트를 짜는거 아닐까요?
저희가 정한 아이디 정책이 소문자에 언더스코어를 구분자를 쓰는 것인데, 개발자가 아이디에 구분자를 연속으로 쓰게되면 저희도 아이디 정책을 바꾸는 걸 고려해야하지 않을까요..

dwax1324

폴라~!
엄청 열심히 해주셨네요 칭찬합니다👏👏👏
덕분에 오늘의 포켓몬 마을도 안전해졌어요🍀

dwax1324 · 2024-10-20T08:40:49Z

backend/pokerogue/src/test/java/com/pokerogue/data/MoveDataTest.java

+    }
+
+    @Test
+    @DisplayName("Move의 아이디는 영어 소문자와 단일 _로만 이루어져 있다.")


아이디의 구분자가 _이기 때문에 연속되면 안됩니다!

Move의 아이디를 살펴봐야겠네요.

dwax1324 · 2024-10-20T08:42:00Z

backend/pokerogue/src/test/java/com/pokerogue/data/MoveDataTest.java

+    }
+
+    @Test
+    @DisplayName("Move의 MoveFlag는 모두 Move Flag Enumd이다")


새로운 클래스 생성

dwax1324 · 2024-10-20T08:55:14Z

backend/pokerogue/src/test/java/com/pokerogue/data/BiomeDataTest.java

+    void name_compositionWhit_English() {
+        List<String> notMatchNames = biomes.stream()
+                .map(Biome::getName)
+                .filter(DataPattern.NAME_PATTERN::isNotMatch)


DataPattern.NAME_PATTERN은 영어와 특수문자 및 공백 및 숫자...

BIOME_NAME 도 따로 만들까요? 🤔

추가하고 Enum도 개행 및 주석 추가해서 가독성 좋게 해봤어요!

dwax1324 · 2024-10-20T08:56:17Z

backend/pokerogue/src/test/java/com/pokerogue/data/BiomeDataTest.java

+    @Test
+    @DisplayName("Biome의 타입들은 전부 Enum Type 안에 들어가있다.")
+    void type_isInEnumType() {
+        assertThat(biomes.stream()
+                .flatMap(biome -> biome.getTypes().stream()))
+                .allMatch(type -> type.getDeclaringClass()
+                        .equals(Type.class));
+    }
+
+    @Test
+    @DisplayName("Biome의 Tier 들은 전부 Enum Tier 안에 들어가있다.")
+    void tier_isInEnumTier() {
+        assertThat(biomes.stream()
+                .flatMap(biome -> biome.getNativePokemons().stream()
+                        .map(NativePokemon::getTier)
+                )
+        ).allMatch(tier -> tier.getDeclaringClass().equals(Tier.class));
+    }
+
+    @Test
+    @DisplayName("Biome의 트레이너들의 타입들은 전부 Enum Type 안에 들어가있다.")
+    void trainerType_isInEnumType() {
+        List<Type> types = biomes.stream()
+                .flatMap(biome -> biome.getTrainers().stream()
+                        .map(Trainer::getTypes))
+                .flatMap(Collection::stream)
+                .toList();
+
+        assertThat(types)
+                .allMatch(type -> type.getDeclaringClass().equals(Type.class));
+    }


컴파일 되면서 Enum Type인지 체크해주지 않을까요??
null인지는 확인해볼 필요가 있겠지만...
타입 체킹은 필요한 테스트인지 잘 모르겠어요.ㅠㅠ

컴파일 상황에서는 해주지 않습니다! (왜냐면 컨버터를 만든 것이 저이기 때문에..)
런타임때 발견되는데, 저도 이미 따로 컨버터 테스트를 만들어서 필요한 지 모르겠으나
일단 목표는 Data 패키지만 돌릴 수 있는 것이기 때문이 일단 추가했습니다 😋

dwax1324 · 2024-10-20T09:06:15Z

backend/pokerogue/src/test/java/com/pokerogue/data/pattern/DataPattern.java

+
+import java.util.regex.Pattern;
+
+public enum DataPattern {


의견)
예시겠지만

포케로그 개발자가 맘먹고 이제 포켓몬에서도 _를 연속으로 쓰게 해야지

이런 경우를 대비해서 테스트를 짜는거 아닐까요?
저희가 정한 아이디 정책이 소문자에 언더스코어를 구분자를 쓰는 것인데, 개발자가 아이디에 구분자를 연속으로 쓰게되면 저희도 아이디 정책을 바꾸는 걸 고려해야하지 않을까요..

dwax1324 · 2024-10-20T09:43:17Z

backend/pokerogue/src/main/java/com/pokerogue/helper/data/PokemonValidator.java

@@ -17,7 +17,7 @@

 class PokemonValidator {


저는 메인 패키지에 있어야 한다고 생각합니다!
제가 PokemonValidator을 구현했을 때는 포켓몬에 대한 정보를 관리하고, 검증하는 클래스로 생각하고 구현했습니다.
그래서 테스트코드에서 직접 검증 과정을 거치기보다는, 검증할 메서드를 클래스 안에 정의하고 테스트코드에서 재사용했습니다.

예를 들면, PokemonDataTest에서는 PokemonValidator를 사용해서 실제 데이터가 정확한지 검증을합니다.
포켓몬 수가 늘어난다면, PokemonDataTest를 고칠 필요 없이 PokemonValidator안에 있는 포켓몬 메타정보만 수정하면 테스트가 통과합니다.
이를 통해 깨지는 테스트를 줄일 수 있고, 데이터 변화의 관리나 추적이 쉬워질 것이라고 생각했어요.
PokemonValidatorTest에서는 데이터와 상관 없는 검증 메서드에 대한 테스트를 하기도 합니다.
(지금은 두개밖에 없지만..)

저희의 코드 구조를 조금만 바꾸어 검증 책임을 각 엔티티 혹은 도메인이 갖게 하면 자연스럽게 PokemonValidator는 메인 패키지에 있게 되지 않을까요??

지금은 PokemonValidator가 생각보다 중요한 역할을 하고 있기 때문에 메인 패키지에 있는 것이 좋아보여요!
(제 의견일 뿐입니다~!)

생각해보니까 테스트에서는 데이터 제한이 엄격한데 실제 프로덕션 코드는 제한이 없는것이 조금 이상하긴 하네요 ㅋㅋㅋ
이제 엔티티 말고 각자의 데이터를 검증하는 도메인을 만들어야 할까요?

- 가독성 좋게 Enum 개행 추가

…4-pokerogue-helper into feat/#364-data-test

jinchiim added 6 commits October 16, 2024 15:59

test(PokeomonControllerTest): 사용하지 않는 테스트 삭제

7804482

test(,,,): 포켓몬 개수 변경 및 사용하지 않는 테스트 삭제

ca75bea

test(PokemonDataTest): @disabled 삭제

757f945

feat(MoveFlag): 누락된 MoveFlag 추가

a0b2c3f

feat(DataPattern): 데이터 패턴 상수 선언

459e114

test(...): 정상 구동 데이터 테스트 작성

20f3fbe

jinchiim added the BE_TEST 🧪 백엔드 테스트 코드 label Oct 16, 2024

jinchiim requested review from dwax1324, jongmee and unifolio0 October 16, 2024 07:24

jinchiim self-assigned this Oct 16, 2024

jongmee approved these changes Oct 17, 2024

View reviewed changes

unifolio0 requested changes Oct 17, 2024

View reviewed changes

dwax1324 approved these changes Oct 20, 2024

View reviewed changes

jinchiim added 4 commits October 20, 2024 21:42

test(...): 오타 수정

1b209ca

test(BiomeDataTest): 바이옴 이름 형식 추가

0ace12d

- 가독성 좋게 Enum 개행 추가

Merge branch 'be/develop' of https://github.com/woowacourse-teams/202…

278deee

…4-pokerogue-helper into feat/#364-data-test

modify(MoveFlag) : 중복된 MoveFlag 삭제

616dbeb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BE-FEAT] 정합성 확인을 통한 테스트 정상화 및 데이터 검증 테스트 작성 #370

[BE-FEAT] 정합성 확인을 통한 테스트 정상화 및 데이터 검증 테스트 작성 #370

jinchiim commented Oct 16, 2024

jongmee left a comment

jongmee Oct 17, 2024

jongmee Oct 17, 2024

unifolio0 Oct 17, 2024

dwax1324 Oct 20, 2024

jinchiim Oct 20, 2024

unifolio0 left a comment

unifolio0 Oct 17, 2024

unifolio0 Oct 17, 2024

dwax1324 Oct 20, 2024

unifolio0 Oct 17, 2024

jinchiim Oct 17, 2024

dwax1324 Oct 20, 2024

dwax1324 left a comment

dwax1324 Oct 20, 2024

dwax1324 Oct 20, 2024

jinchiim Oct 20, 2024 •

edited

Loading

dwax1324 Oct 20, 2024

jinchiim Oct 20, 2024

jinchiim Oct 20, 2024

dwax1324 Oct 20, 2024

jinchiim Oct 20, 2024

dwax1324 Oct 20, 2024

dwax1324 Oct 20, 2024

[BE-FEAT] 정합성 확인을 통한 테스트 정상화 및 데이터 검증 테스트 작성 #370

Are you sure you want to change the base?

[BE-FEAT] 정합성 확인을 통한 테스트 정상화 및 데이터 검증 테스트 작성 #370

Conversation

jinchiim commented Oct 16, 2024

🍄 PR 확인 사항

현재 작업은 어떤 이슈를 해결한 것인지 설명해주세요.

기존 코드에서 변경된 점이 있다면 설명해주세요. (추가 X)

jongmee 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

unifolio0 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

dwax1324 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jinchiim Oct 20, 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

jinchiim Oct 20, 2024 •

edited

Loading