+1 from my side. -Matthias
On 3/22/18 5:12 PM, John Roesler wrote: > Yep, I'm super happy with this approach vs. a third module just for the > tests. > > For clairty, here's a PR demonstrating the model we're proposing: > https://github.com/apache/kafka/pull/4760 > > Thanks, > -John > > On Thu, Mar 22, 2018 at 6:21 PM, Guozhang Wang <wangg...@gmail.com> wrote: > >> I'm +1 to the approach as well across modules that are going to have test >> utils artifacts in the future. To me this seems to be a much smaller change >> we can make to break the circular dependencies than creating a new package >> for our own testing code. >> >> Guozhang >> >> On Thu, Mar 22, 2018 at 1:26 PM, Bill Bejeck <bbej...@gmail.com> wrote: >> >>> John, >>> >>> Thanks for the clear, detailed explanation. >>> >>> I'm +1 on what you have proposed. >>> While I agree with you manually pulling in transitive test dependencies >> is >>> not ideal, in this case, I think it's worth it to get over the circular >>> dependency hurdle and use streams:test-utils ourselves. >>> >>> -Bill >>> >>> On Thu, Mar 22, 2018 at 4:09 PM, John Roesler <j...@confluent.io> wrote: >>> >>>> Hey everyone, >>>> >>>> In 1.1, kafka-streams adds an artifact called >> 'kafka-streams-test-utils' >>>> (see >>>> https://kafka.apache.org/11/documentation/streams/ >>>> developer-guide/testing.html >>>> ). >>>> >>>> The basic idea is to provide first-class support for testing Kafka >>> Streams >>>> applications. Without that, users were forced to either depend on our >>>> internal test artifacts or develop their own test utilities, neither of >>>> which is ideal. >>>> >>>> I think it would be great if all our APIs offered a similar module, and >>> it >>>> would all be good if we followed a similar pattern, so I'll describe >> the >>>> streams approach along with one challenge we had to overcome: >>>> >>>> ===================== >>>> = Project Structure = >>>> ===================== >>>> >>>> The directory structure goes: >>>> >>>> kafka/streams/ <- main module code here >>>> /test-utils/ <- test utilities module here >>>> /examples/ <- example usages here >>>> >>>> Likewise, the artifacts are: >>>> >>>> kafka-streams >>>> kafka-streams-test-utils >>>> kafka-streams-examples >>>> >>>> And finally, the Gradle build structure is: >>>> >>>> :streams >>>> :streams:test-utils >>>> :streams:examples >>>> >>>> >>>> ============================= >>>> = Problem 1: circular build = >>>> ============================= >>>> >>>> In eat-your-own-dogfood tradition, we wanted to depend on our own >>>> test-utils in our streams tests, but :streams:test-utils (obviously) >>>> depends on :streams already. >>>> >>>> (:streams) <-- (:streams:test-utils) >>>> \---> >>>> >>>> Luckily, Filipe Agapito found a way out of the conundrum ( >>>> https://issues.apache.org/jira/browse/KAFKA-6474? >>>> focusedCommentId=16402326&page=com.atlassian.jira. >>>> plugin.system.issuetabpanels:comment-tabpanel#comment-16402326). >>>> Many thanks to him for this contribution. >>>> >>>> * Add this to the ':streams' definition: >>>> testCompile project(':streams:test-utils').sourceSets.main.output >>>> >>>> * And this to the ':streams:test-utils' definition: >>>> compile project(':streams').sourceSets.main.output >>>> >>>> * And finally (because we also have tests for the examples), add this >> to >>>> the ':streams:examples' definition: >>>> testCompile project(':streams:test-utils') >>>> >>>> >>>> >>>> By scoping the dependencies to 'sourceSets.main', we break the cyclic >>>> dependency: >>>> >>>> (:streams main) <-- (:streams:test-utils main) >>>> ^ ^ ^ >>>> | / | >>>> | / | >>>> (:streams test) (:streams:test-utils test) >>>> >>>> >>>> ============================================== >>>> = Problem 2: missing transitive dependencies = >>>> ============================================== >>>> >>>> Scoping the dependency to source-only skips copying transitive library >>>> dependencies into the build & test environment, so we ran into the >>>> following error in our tests for ':streams:test-utils' : >>>> >>>> java.lang.ClassNotFoundException: org.rocksdb.RocksDBException >>>> >>>> This kind of thing is easy to resolve, once you understand why it >>> happens. >>>> We just added this to the :test-utils build definition: >>>> testCompile libs.rocksDBJni >>>> >>>> It's a little unfortunate to have to manually pull in transitive >>>> dependencies for testing, but it's really the only downside of this >>>> approach (so far). >>>> >>>> >>>> >>>> That's about it! This is partly to propose a similar model across other >>>> parts of Kafka's API and partly to collect feedback on this approach. >>>> >>>> Thoughts? >>>> -John >>>> >>> >> >> >> >> -- >> -- Guozhang >> >
signature.asc
Description: OpenPGP digital signature
