Refactor build as private #111
Conversation
github-actions
bot
requested review from
acktsap,
benelog,
jwChung and
mhyeon-lee
| @@ -179,7 +180,8 @@ private ArbitraryBuilder( | |||
| return this; | |||
| } | |||
|
|
|||
| public Arbitrary<T> build() { | |||
| @API(since = "0.4.0", status = Status.EXPERIMENTAL) | |||
| Arbitrary<T> build() { | |||
There was a problem hiding this comment.
private으로 전환하면서 giveMeArbitrary와 build는 동일한 의도인 것 같아서 giveMeArbitrary도 제거했습니다.
autoparams 모듈에서도 Arbitrary가 아닌 ArbitraryBuilder를 반환하도록 수정했습니다.
@mhyeon-lee
jqwik 엔진을 사용해 테스트를 하려면 giveMeArbitrary가 필요하네요.
private으로 두지 않고 package-private으로 뒀을 때 문제가 있을까요?
There was a problem hiding this comment.
@seongahjo @Provider 로 제공하는거 때문에 그런거죠?
그러고보니 package-private 이든 private 이든 바꾸면 @Provider 로 아예 사용 못하는거 아닌가요?
There was a problem hiding this comment.
@mhyeon-lee
ArbitraryBuilder와 동일한 패키지에 존재하는 Fixture Monkey 인스턴스에서 package-private 인 build를 사용하여 Arbitrary를 public 메소드인 giveMeArbitrary 으로 노출하고 있기 때문에 사용할 수 있습니다.
There was a problem hiding this comment.
@seongahjo
@Provider 에서 ArbitraryBuilder 로 조작을 한 Arbitrary 는 반환 못하는거 아닌가요?
There was a problem hiding this comment.
@mhyeon-lee
넵 맞습니다.
지금 있는 테스트 중에서는 ArbitraryBuilder에서 Arbitrary를 생성해서 테스트하는 경우는 없습니다.
FixtureMonkey 인스턴스에서 Arbitrary를 생성하는 경우는 연산을 적용하지 않기 때문에 문제가 없습니다.
There was a problem hiding this comment.
지금 있는 테스트 중에서는 ArbitraryBuilder에서 Arbitrary를 생성해서 테스트하는 경우는 없습니다.
오픈 소스로 공개한 이상 내부 사용여부로 판단하기는 어려울거 같습니다. 저 기능을 사용하는 누군가에게는 breaking 이 되기 때문에 그것을 감수할만한 것인지 고려할 필요가 있습니다.
가능하면 @Deprecated 마킹 후에 제거하는 방향으로 순차적으로 가는게 좋긴 합니다.
There was a problem hiding this comment.
@mhyeon-lee
넵, 말씀드린 건 Fixture Monkey 내에 있는 테스트들인데요.
일단 0.4.0 에서 build를 depreacted 마킹하고 이후에 private으로 전환해도 되겠군요.
| @@ -179,7 +180,8 @@ private ArbitraryBuilder( | |||
| return this; | |||
| } | |||
|
|
|||
| public Arbitrary<T> build() { | |||
| @API(since = "0.4.0", status = Status.EXPERIMENTAL) | |||
There was a problem hiding this comment.
공개 메소드가 아니므로 안 붙여도 될거 같습니다.
신규 메소드는 아니고 breaking 이 발생하는 부분이므로 간단하게 주석으로 이 PR 주소와 함께 breaking 한 이유를 간단하게 적어주시면 좋을거 같습니다.
seongahjo
force-pushed
the
sa/refactor-convert-build-to-private
branch
from
1668c00
to
7ce481e
Compare
|
짬이 좀 나서 Arbitrary 의 list, set, stream, iterator, array 정도만 구현해주면 될거 같은데요 이정도는 할 수 있지 않나 싶은데 더 제한되는 부분이 있나요? |
|
@mhyeon-lee Fixture Monkey에서 연산을 적용하다 보면 객체를 구성하는 Arbitrary들이 실제로는 Arbitrary가 아닌 Arbitrary에 wrapping한 고정 값일 확률이 높기 때문입니다. |
|
@seongahjo 이렇게 테스트했을 때도 같은 값이 나오는군요 이게 해결되면 list, set, stream, array 들도 자연스럽게 문제가 해결될거 같군요 내일 일정 잡고 같이 한번 보죠
|
|
@mhyeon-lee 내일 같이 보고 jqwik 객체 생성 구조를 좀 더 알게되면 |
|
#115 로 Arbitrary 인터페이스가 지원하는 기능을 제공할 수 있게 되어 닫습니다. |
ArbitraryBuilder 인터페이스에서 jqwik의
Arbitrary의존성을 제거하기 위해build를 private으로 변경하여 클라이언트에게 노출하지 않습니다.이런 결정을 한 이유는 다음과 같습니다.
apply,acceptIf등의 복잡한 연산을 사용하는 경우 jqwik에서 정의한 연산을 의도대로 지원하기 어렵다.