Fix size manipulation InnerSpec #418
Conversation
github-actions
bot
requested review from
acktsap,
benelog,
jwChung,
mhyeon-lee and
SooKim1110
| @@ -75,7 +75,7 @@ public ArbitraryIntrospectorResult introspect(ArbitraryGeneratorContext context) | |||
|
|
|||
| return new ArbitraryIntrospectorResult( | |||
| builderCombinator.build() | |||
| // .filter(it -> it.size() == childrenArbitraries.size()) | |||
| .filter(it -> it.size() == childrenArbitraries.size()) | |||
There was a problem hiding this comment.
unique를 구현하여 filter를 활성화합니다.
| @Override | ||
| public boolean equals(Object obj) { | ||
| if (this == obj) { | ||
| return true; | ||
| } | ||
| if (obj == null || getClass() != obj.getClass()) { | ||
| return false; | ||
| } | ||
| PropertyDescriptorProperty that = (PropertyDescriptorProperty)obj; | ||
| return annotatedType.getType().equals(that.annotatedType.getType()) | ||
| && annotations.equals(that.annotations); | ||
| } |
There was a problem hiding this comment.
getter를 소유하고 있는 객체 타입은 고려하지 않고 필드의 타입, 어노테이션으로만 비교합니다.
| private boolean isUnique(ArbitraryNode node) { | ||
| return node.getProperty() instanceof MapKeyElementProperty; | ||
| } | ||
|
|
There was a problem hiding this comment.
다음 PR에서 옵션으로 분리할 예정입니다.
| @@ -967,7 +967,10 @@ void registerRootAndChildElementGeneratingRoot() { | |||
| .build(); | |||
|
|
|||
| // when | |||
| List<String> actual = sut.giveMeOne(ComplexObject.class).getList().stream() | |||
| List<String> actual = sut.giveMeBuilder(ComplexObject.class) | |||
| .size("map", 1) | |||
There was a problem hiding this comment.
String이 register되어 항상 key가 동일한 값이 생성되므로 개수를 강제로 지정합니다.
| LabMonkey sut = LabMonkey.labMonkeyBuilder() | ||
| .defaultArbitraryContainerInfo(new ArbitraryContainerInfo(1, 3, false)) | ||
| .build(); | ||
|
|
||
| NestedKeyMapObject actual = sut.giveMeBuilder(NestedKeyMapObject.class) |
There was a problem hiding this comment.
map의 크기가 0인 경우가 너무 많이 생성되어 validation을 통과 못해 객체를 정상적으로 생성 못하는 문제가 있습니다.
따라서 map의 크기를 1이상으로 강제 설정합니다.
There was a problem hiding this comment.
이 부분 잘 이해를 못한 것 같은데 ArbitraryContainerInfo는 min = 1, max = 4 로 설정되지 않나요?
맵 크기가 0인 경우에 어떤 validation이 문제가 되는지 궁금합니다.
There was a problem hiding this comment.
@SooKim1110
Map<Map<String, String>, String> 를 경우로 예를 들면,
기본적인 컨테이너 생성 범위는 크기가 0~3개인데 키가 빈 맵 (크기가 0인 경우)을 반복적으로 생성합니다.
아마 최소 값에 대한 엣지 케이스때문에 경계값인 0이 주로 생성되는 것 같은데, 이 경우에 키가 중복되어 validation을 통과 못합니다.
이게 저희가 jqwik 엔진을 사용하지 않고 sampling 해서 발생하는 문제인지 combination된 Arbitrary의 shrinking 문제인지는 모르겠네요.
이슈를 만들어 고민해볼만한 주제인 것 같습니다.
| public boolean equals(Object obj) { | ||
| if (this == obj) { | ||
| return true; | ||
| } | ||
| if (obj == null || getClass() != obj.getClass()) { | ||
| return false; | ||
| } | ||
| FieldProperty that = (FieldProperty)obj; | ||
|
|
||
| return annotatedType.getType().equals(that.annotatedType.getType()) | ||
| && annotations.equals(that.annotations); | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(annotatedType.getType(), annotations); | ||
| } | ||
|
|
||
| @Nullable |
There was a problem hiding this comment.
필드를 소유하고 있는 객체 타입은 고려하지 않고 필드의 타입, 어노테이션으로만 비교합니다.
fixture-monkey/src/test/java/com/navercorp/fixturemonkey/test/InnerSpecTest.java
Outdated
Show resolved
Hide resolved
| @Property | ||
| void mapPutFourth() { | ||
| MapObject actual = SUT.giveMeBuilder(MapObject.class) | ||
| .setInner( | ||
| "strMap", | ||
| m -> m.minSize(4) | ||
| .entry("key1", "value1") | ||
| .entry("key2", "value2") | ||
| .entry("key3", "value3") | ||
| .entry("key4", "value4") | ||
| ) | ||
| .sample(); | ||
|
|
||
| then(actual.getStrMap().get("key1")).isEqualTo("value1"); | ||
| then(actual.getStrMap().get("key2")).isEqualTo("value2"); | ||
| then(actual.getStrMap().get("key3")).isEqualTo("value3"); | ||
| then(actual.getStrMap().get("key4")).isEqualTo("value4"); | ||
| } | ||
|
|
There was a problem hiding this comment.
중요한건 아닌데 mapPutTwice 테스트가 있는데 mapPutFourth 테스트가 있는 이유가
DEFAULT_ELEMENT_MAX_SIZE 개수를 초과하는 entry 생성이 있었을 때를 확인하는 것 같아서
테스트 이름을 mapPutEntryOverDefaultElementMaxSize로 하는 것은 어떨까요??
There was a problem hiding this comment.
minSize로 사이즈를 설정하고 있어서 말씀하신 의도와는 달라져버렸네욤
이 테스트는 entry가 entry를 추가하는 연산이었을 때는 필요했는데 지금은 필요없겠네요.
제거하겠습니당.
| @@ -75,7 +75,7 @@ public ArbitraryIntrospectorResult introspect(ArbitraryGeneratorContext context) | |||
|
|
|||
| return new ArbitraryIntrospectorResult( | |||
| builderCombinator.build() | |||
| // .filter(it -> it.size() == childrenArbitraries.size()) | |||
| .filter(it -> it.size() == childrenArbitraries.size()) | |||
There was a problem hiding this comment.
이거는 이전에 size 를 채워지지 않는 경우 반복 호출되는 부분 때문에 우선 주석 처리했던거 같은데요
여기서 filter 조건이 안되면 속성을 다시 처음부터 만들게 되는 문제가 있었던거 같습니다.
이미 생성된 값은 두고 새로 생성된 값이 중복일 때 이것만 재생성 하도록 해보면 좋을거 같습니다.
Set 도 같을거 같고요
There was a problem hiding this comment.
요 이슈는 별도 이슈로 만들어 처리하겠습니다.
| FieldProperty that = (FieldProperty)obj; | ||
|
|
||
| return annotatedType.getType().equals(that.annotatedType.getType()) | ||
| && annotations.equals(that.annotations); |
There was a problem hiding this comment.
annotatedType.getType() 이 같은 타입이더라도 Field 는 서로 다른것일 수 있을거 같습니다.
There was a problem hiding this comment.
@mhyeon-lee
넵, 타입이 같고 어노테이션만 같다면 동일한 Arbitrary로 생성이 되기 때문에 필드가 다르더라도 캐싱이 되는 걸 의도했습니다.
There was a problem hiding this comment.
@seongahjo generics 가 다를 수 있어서요
There was a problem hiding this comment.
@mhyeon-lee
generics을 가지고 있는 경우에는 getType()의 결과로 ParameterizedType 이 반환이 되는데요.
ParameterizedType의 equals는 generic 까지 일치해야 합니다.
| return false; | ||
| } | ||
| PropertyDescriptorProperty that = (PropertyDescriptorProperty)obj; | ||
| return annotatedType.getType().equals(that.annotatedType.getType()) |
There was a problem hiding this comment.
마찬가지로 annotatedType.getType() 이 같은 타입이더라도 PropertyDescriptorProperty 는 서로 다른것일 수 있을거 같습니다.
InnerSpec에서 size 연산이 맵 연산과 호환안되는 문제를 해결합니다.
호환 안되는 문제를 해결하니 맵의 키가 중복되어 맵에 설정한
값(value 연산)이 생성안되는 문제가 발생했습니다.이 문제도 해결합니다.
각각 작업이 의존성을 가져 PR을 분리하지 않고 커밋으로 분리했습니다.