kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From John Roesler <j...@confluent.io>
Subject Re: Gradle strategy for exposing and using public test-utils modules
Date Tue, 03 Apr 2018 21:16:34 GMT
Hello again all,

It turns out that the implementation I provided was not correct after all.

The issue was gradle tracked the compiled source files included via

> compile project(':streams').sourceSets.main.output

and included them in the release tarball.

Ewen found the issue and corrected the project to use "compileOnly"
instead, which uses the dependency to compile the project but does not
export the dependency:
https://github.com/apache/kafka/pull/4816

Although Ewen's patch does produce the desired output, I think we'd rather
have test-utils export a regular dependency on the main streams module. But
we can apply the same strategy to allow the streams tests to pull in
test-utils as a compile-only dependency:

> testCompileOnly project(':streams:test-utils')


Here's a PR with the new proposed strategy:
https://github.com/apache/kafka/pull/4821. Feel free to comment.

For those wishing for a summary, here's the new proposed strategy:

project(':streams') {
>     ...
>     dependencies {
>         ...
>
        // this breaks the dependency cycle:

        testCompileOnly project(':streams:test-utils')
>         ...
>     }
>     ...
> }




> project(':streams:test-utils') {
>     ...
>     dependencies {
>         compile project(':streams')
>         ...
>     }
>     ...
> }




> project(':streams:examples') {
>     ...
>     dependencies {
>         compile project(':streams')
>         ...
>         testCompile project(':streams:test-utils')
>         ...
>     }
>     ...
> }


Thanks,
-John

On Tue, Mar 27, 2018 at 7:06 PM, John Roesler <john@confluent.io> wrote:

> 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 <token01@126.com> wrote:
>
>> +1
>> > 在 2018年3月23日,下午12:20,Ted Yu <yuzhihong@gmail.com> 写道:
>> >
>> > +1
>> > -------- Original message --------From: "Matthias J. Sax" <
>> matthias@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 <wangguoz@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 <bbejeck@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 <john@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
>> >>>
>> >>
>> >
>>
>>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message