Hi again everyone, Just for the sake of closure, I think everyone is generally in agreement with this approach. If concerns arise later on, please let me know!
Thanks, -John On Fri, Mar 23, 2018 at 12:41 AM, zhenya Sun <toke...@126.com> wrote: > +1 > > 在 2018年3月23日，下午12:20，Ted Yu <yuzhih...@gmail.com> 写道： > > > > +1 > > -------- Original message --------From: "Matthias J. Sax" < > matth...@confluent.io> Date: 3/22/18 9:07 PM (GMT-08:00) To: > dev@kafka.apache.org Subject: Re: Gradle strategy for exposing and using > public test-utils modules > > +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 > >>> > >> > > > >
