Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 10 commits into
base: be/develop
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

class PokemonValidator {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

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

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

내일 얘기해보아요

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


private static final int POKEMON_SIZE = 1446;
private static final int POKEMON_SIZE = 1453;
private static final int MIN_GENERATION = 1;
private static final int MAX_GENERATION = 9;
private static final int MIN_TYPE_COUNT = 1;
Expand All @@ -31,8 +31,8 @@ class PokemonValidator {
private static final String DELIMITER = "_";
private static final String EMPTY_ABILITY = "none";
private static final IntPredicate isExpectedIdLetter = character -> isLowerCase(character)
|| isDigit(character)
|| isDelimiter(character);
|| isDigit(character)
|| isDelimiter(character);

private PokemonValidator() {
}
Expand All @@ -46,8 +46,8 @@ static void validatePokemonSize(int pokemonCount) {

static void validatePokemonIdFormat(List<String> pokemonIds) {
Predicate<String> isExpectedLetter = id -> id.codePoints().allMatch(isExpectedIdLetter);
Predicate<String> isDelimiterSeparated = id -> id.contains(DELIMITER + DELIMITER);
Predicate<String> isDelimiterInPlace = id -> id.startsWith(DELIMITER) || id.endsWith(DELIMITER);
Predicate<String> isDelimiterSeparated = id -> !id.contains(DELIMITER + DELIMITER);
Predicate<String> isDelimiterInPlace = id -> !id.startsWith(DELIMITER) || id.endsWith(DELIMITER);

String message = "아이디: %s는 아이디 규칙에 맞지 않습니다.";

Expand Down Expand Up @@ -147,22 +147,6 @@ static void validateTotalAbilityCount(List<Pokemon> pokemons) {
}
}

static void validateTotalAbilityDuplication(List<Pokemon> pokemons) {
Predicate<Pokemon> isAbilityDisjoint = pokemon -> {
List<String> totalAbilityIds = pokemon.getNormalAbilityIds();
totalAbilityIds.add(pokemon.getHiddenAbilityId());
totalAbilityIds.add(pokemon.getPassiveAbilityId());

Set<String> uniqueIds = new HashSet<>(totalAbilityIds);

return totalAbilityIds.size() == uniqueIds.size();
};

for (Pokemon pokemon : pokemons) {
validate(isAbilityDisjoint, pokemon, ErrorMessage.POKEMON_ABILITY_DUPLICATION);
}
}

static void validateStatValueRange(List<Pokemon> pokemons) {
Predicate<Pokemon> isStatsInRange = pokemon -> {
List<Number> stats = List.of(
Expand Down Expand Up @@ -271,5 +255,4 @@ private static boolean isInRange(int target, int min, int max) {
private static boolean isDelimiter(int character) {
return DELIMITER.charAt(0) == character;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ public enum MoveFlag {
MAKES_CONTACT("makes_contact"),
IGNORE_PROTECT("ignore_protect"),
IGNORE_VIRTUAL("ignore_virtual"),
IGNORE_SUBSTITUTE("ignore_substitute"),
SOUND_BASED("sound_based"),
HIDE_USER("hide_user"),
HIDE_TARGET("hide_target"),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package com.pokerogue.data;

import static org.assertj.core.api.Assertions.assertThat;

import com.pokerogue.data.pattern.DataPattern;
import com.pokerogue.environment.repository.MongoRepositoryTest;
import com.pokerogue.helper.ability.data.Ability;
import com.pokerogue.helper.ability.repository.AbilityRepository;
import com.pokerogue.helper.pokemon.data.Pokemon;
import com.pokerogue.helper.pokemon.repository.PokemonRepository;
import java.util.HashSet;
import java.util.List;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;

public class AbilityDataTest extends MongoRepositoryTest {

private final PokemonRepository pokemonRepository;
private final List<Ability> abilities;

@Autowired
public AbilityDataTest(AbilityRepository abilityRepository, PokemonRepository pokemonRepository) {
this.pokemonRepository = pokemonRepository;
this.abilities = abilityRepository.findAll();
}

@Test
@DisplayName("Ability의 아이디는 영어 소문자와 단일 _ 로만 이루어져 있다.")
void id_validateAbilityIds() {
List<String> notMatchAbilityIds = abilities.stream()
.map(Ability::getId)
.filter(DataPattern.ID_PATTERN::isNotMatch)
.toList();

assertThat(notMatchAbilityIds).isEmpty();
}

@Test
@DisplayName("Ability 속 포켓몬 id들은 전부 존재한다.")
void pokemonId_validatePokemonIds() {
List<String> pokemonIds = abilities.stream()
.map(Ability::getPokemonIds)
.flatMap(List::stream)
.toList();
List<Pokemon> pokemons = pokemonRepository.findAll();
List<String> ids = pokemons.stream()
.map(Pokemon::getId)
.toList();

List<String> notMatchIds = pokemonIds.stream()
.filter(pokemonId -> !ids.contains(pokemonId))
.toList();

assertThat(notMatchIds).isEmpty();
}

@Test
@DisplayName("모든 능력 설명이 한글이 포함되어 구성하고 있다.")
void description_compositionWithKorean() {
List<String> notMatchDescription = abilities.stream()
.map(Ability::getDescription)
.filter(DataPattern.DESCRIPTION_PATTERN::isNotMatch)
.toList();

assertThat(notMatchDescription).isEmpty();
}

@Test
@DisplayName("모든 KoName이 적어도 한 자의 한글과 영어로 이루어져 있다.")
void koName_compositionWith_AtLeastOneKorean() {
List<String> notMatchNames = abilities.stream()
.map(Ability::getKoName)
.filter(DataPattern.KO_NAME_PATTERN::isNotMatch)
.toList();

assertThat(notMatchNames).isEmpty();
}

@Test
@DisplayName("모든 Name이 영어로 이루어져 있다.")
void name_compositionWith_English() {
List<String> notMatchNames = abilities.stream()
.map(Ability::getName)
.filter(DataPattern.NAME_PATTERN::isNotMatch)
.toList();

assertThat(notMatchNames).isEmpty();
}

@Test
@DisplayName("Ability의 pokemon Id들은 중복되지 않는다.")
void pokemonIds_NotDuplicated() {
List<String> duplicatedPokemonAbilityIds = abilities.stream()
.filter(ability -> isDuplicated(ability.getPokemonIds()))
.map(Ability::getId)
.toList();

assertThat(duplicatedPokemonAbilityIds).isEmpty();
}

private boolean isDuplicated(List<String> pokemonIds) {
return pokemonIds.size() != new HashSet<>(pokemonIds).size();
}
}
205 changes: 205 additions & 0 deletions backend/pokerogue/src/test/java/com/pokerogue/data/BiomeDataTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,205 @@
package com.pokerogue.data;

import static org.assertj.core.api.Assertions.assertThat;

import com.pokerogue.data.pattern.DataPattern;
import com.pokerogue.environment.repository.MongoRepositoryTest;
import com.pokerogue.helper.biome.data.Biome;
import com.pokerogue.helper.biome.data.NativePokemon;
import com.pokerogue.helper.biome.data.Tier;
import com.pokerogue.helper.biome.data.Trainer;
import com.pokerogue.helper.biome.repository.BiomeRepository;
import com.pokerogue.helper.pokemon.data.Pokemon;
import com.pokerogue.helper.pokemon.repository.PokemonRepository;
import com.pokerogue.helper.type.data.Type;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;

public class BiomeDataTest extends MongoRepositoryTest {

private final List<Biome> biomes;
private final List<String> pokemonIds;

@Autowired
public BiomeDataTest(BiomeRepository biomeRepository, PokemonRepository pokemonRepository) {
this.biomes = biomeRepository.findAll();

List<Pokemon> pokemons = pokemonRepository.findAll();
this.pokemonIds = pokemons.stream()
.map(Pokemon::getId)
.toList();
}

@Test
@DisplayName("Biome의 아이디는 영어 소문자와 단일 _ 로만 이루어져 있다. ")
void id_validateBiomeIds() {
List<String> notMatchBiomeIds = biomes.stream()
.map(Biome::getId)
.filter(DataPattern.ID_PATTERN::isNotMatch)
.toList();

assertThat(notMatchBiomeIds).isEmpty();
}

@Test
@DisplayName("Biome의 name은 영어 문자로 이루어져 있다.")
void name_compositionWhit_English() {
List<String> notMatchNames = biomes.stream()
.map(Biome::getName)
.filter(DataPattern.NAME_PATTERN::isNotMatch)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BIOME_NAME 도 따로 만들까요? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

.toList();

assertThat(notMatchNames).isEmpty();
}

@Test
@DisplayName("Biome의 koName은 한글이 포함되어 구성하고 있다.")
void koName_compositionWith_AtLeastOneKorean() {
List<String> notMatchNames = biomes.stream()
.map(Biome::getKoName)
.filter(this::isNotMatchKoNamePattern)
.toList();

assertThat(notMatchNames).isEmpty();
}

@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));
}
Comment on lines +72 to +102
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


@Test
@DisplayName("Biome의 native Pokemon 속 pokemonId들이 전부 존재한다.")
void nativePokemonId_isInPokemonCollection() {
List<NativePokemon> nativePokemons = biomes.stream()
.map(Biome::getNativePokemons)
.flatMap(List::stream)
.toList();

List<String> nativePokemonIds = nativePokemons.stream()
.map(NativePokemon::getPokemonIds)
.flatMap(List::stream)
.toList();

List<String> notMatchIds = nativePokemonIds.stream()
.filter(nativeId -> !pokemonIds.contains(nativeId))
.toList();
jinchiim marked this conversation as resolved.
Show resolved Hide resolved

assertThat(notMatchIds).isEmpty();
}

@Test
@DisplayName("Biome Trainer 의 name 필드가 영어 소문자 혹은 연속되지 않는 _ 로 이루어져 있다.")
void name_isCompositionWithEnglish() {
List<Trainer> trainers = biomes.stream()
.map(Biome::getTrainers)
.flatMap(List::stream)
.toList();

List<String> notMatchTrainerNames = trainers.stream()
.map(Trainer::getName)
.filter(DataPattern.NAME_PATTERN::isNotMatch)
.toList();

assertThat(notMatchTrainerNames).isEmpty();
}

@Test
@DisplayName("Biome Trainer 의 KoName 필드가 한국어로 이루어져있다.")
void koName_isCompositionWithKorean() {
List<Trainer> trainers = biomes.stream()
.map(Biome::getTrainers)
.flatMap(List::stream)
.toList();

List<String> notMatchTrainerNames = trainers.stream()
.map(Trainer::getKoName)
.filter(this::isNotMatchKoNamePattern)
.toList();

assertThat(notMatchTrainerNames).isEmpty();
}

@Test
@DisplayName("Biome 의 nativePokemon Id들은 중복되지 않는다.")
void nativePokemonIds_NotDuplicate() {
List<List<NativePokemon>> nativePokemons = biomes.stream()
.map(Biome::getNativePokemons)
.toList();

List<NativePokemon> duplicatedNativePokemon = new ArrayList<>();

nativePokemons.forEach(natives ->
natives.forEach(nativePokemon -> {
if (isDuplicated(nativePokemon.getPokemonIds())) {
duplicatedNativePokemon.add(nativePokemon);
}
})
);

assertThat(duplicatedNativePokemon).isEmpty();
}

@Test
@DisplayName("Biome의 trainer PokemonId 들은 중복되지 않는다.")
void trainerPokemonIds_NotDuplicated() {
List<List<Trainer>> trainers = biomes.stream()
.map(Biome::getTrainers)
.toList();

List<Trainer> duplicatedPokemonTrainer = new ArrayList<>();

trainers.forEach(trainerList ->
trainerList.forEach(trainer -> {
if (isDuplicated(trainer.getPokemonIds())) {
duplicatedPokemonTrainer.add(trainer);
}
})
);

assertThat(duplicatedPokemonTrainer).isEmpty();
}

private boolean isNotMatchKoNamePattern(String koName) {
if ("???".equals(koName)) {
return false;
}
return DataPattern.KO_NAME_PATTERN.isNotMatch(koName);
}

private boolean isDuplicated(List<String> pokemonIds) {
return pokemonIds.size() != new HashSet<>(pokemonIds).size();
}
}
Loading
Loading