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 >
