Hi Tommy and Matthias, Thanks for the context! I can see that there are good reasons to want a supported, embedded Kafka cluster. If you want to propose to add one, I can help you navigate the process.
Thanks, John On Tue, Dec 10, 2019, at 07:13, Matthias Merdes wrote: > Hi John and Thomas, > > Thanks a lot for your detailed and well-informed replies. > I am adding some comments below. > > > On 6. Dec 2019, at 17:42, John Roesler > <vvcep...@apache.org<mailto:vvcep...@apache.org>> wrote: > > Hi Matthias! > > Thanks for the note, and the kind sentiment. > > We're always open to improvements like this, so your contribution would > certainly be welcome. > > Just FYI, "public interface" changes need to go through the KIP process > (see > https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals > ). You could of course, open an exploratory PR and ask here for comments > before deciding if you want to make a KIP, if you prefer. Personally, I often > find it clarifying to put together a PR concurrently with the KIP, because it > helps to flush out any "devils in the details", and also can help the KIP > discussion. > > > So far I was under the impression that KIPs were only for major changes. > From GitHub projects I am used to discussions directly in the issue. > > Just wanted to make you aware of the process. I'm happy to help you > navigate this process, by the way. > > Regarding the specific proposal, the number one question in my mind is > compatibility... I.e., do we add new dependencies, or change > dependencies, that may conflict with what's already on users' > classpaths? Would the change result in any source-code or binary > incompatibility with previous versions? Etc... you get the idea. > > > In this case the additional dependency would be on the JUnit Jupiter > API which was intended to work alongside JUnit 4 in IDEs and build > tools. > So there should not be compatibility problems. The existing Rule could > be slightly refactored such that JUnit 4 users would not need the JUnit > 5 dependency (and vice versa) at test time. > > We can make such changes, but it's a _lot_ easier to support doing it > in a major release. Right now, that would be 3.0, which is not > currently planned. We're currently working toward 2.5, and then 2.6, > and so on, essentially waiting for a good reason to bump up to 3.0. > > All that said, EmbeddedKafkaCluster is an interesting case, because > it's not actually a public API! > When Bill wrote the book, he included it (I assume) because there was > no other practical approach to testing available. > However, since the publication, we have added an official integration > testing framework called TopologyTestDriver. When people ask me for > testing advice, I tell them: > 1. Use TopologyTestDriver (for Streams testing) > 2. If you need a "real" broker for your test, then set up a pre/post > integration test hook to run Kafka independently (e.g., with Maven). > 3. If that's not practical, then _copy_ EmbeddedKafkaCluster into your > own project, don't depend on an internal test artifact. > > > TopologyTestDriver ist very useful for doing unit tests (in the narrow > sense of the word) and should of course be used whereever possible > > The EmbeddedKafkaCluster would be aimed at integration testing - with > or without streams. > Personally, I have had mixed experiencies with running ‘middleware’ > externally to tests (Docker container, or other kind of external > process) > Having control and configuration right within your tests can be very > helpful. > When I first suggested the JUnit 5 variant I was not aware of it not > being ‘public’ because I checked the book and Kafka’s own sources only > and missed this fact. > From your answers I understood that you are not convinced that > publishing EmbeddedKafkaCluster is a good idea at all. > Maybe this could be reevaluated when (if at all) the Kafka test code > base is migrated to JUnit 5. > > I guess in the meantime the similar support from spring-kafka-test > (EmbeddedKafkaBroker) can be used - > at least when working with the spring stack. > > > > > To me, this means that we essentially have free reign to make changes > to EmbeddedKafkaCluster, since it _should_ only > be used for internal testing. In that case, I would just evaluate the > merits up bumping all the tests in Apache Kafka up to JUnit 5. Although > that's not a public API, it might be a big enough change to the process > to justify a design document and project-wide discussion. > > > Upgrading the Kafka test code base to JUnit 5 should not be too hard, > with @(Class)Rule usage needing some work. > The rules used here (Timeout, TestName, TempFolder, Moskito, and > EasyMock) all have JUnit 5 replacements. > So at least the Java part should mostly be busy work only (I cannot > judge the Scala part). > If you ever decide to upgrade to JUnit 5 let me know und I would be > happy to help. :) > > Thanks again, > Cheers, > Matthias > > > > > Well, I guess it turns out I had more to say than I initially > thought... Sorry for rambling a bit. > > What are your thoughts? > -John > > On Fri, Dec 6, 2019, at 07:05, Matthias Merdes wrote: > Hi all, > > when reading ‘Kafka Streams in Action’ I especially enjoyed the > thoughtful treatment of > mocking, unit, and integration tests. > Integration testing in the book (and in the Kafka codebase) is done > using the @ClassRule-annotated EmbeddedKafkaCluster. > JUnit 5 Jupiter replaced Rules with a different extension model. > So my proposal would be to support a JUnit 5 Extension in addition to > the JUnit 4 Rule for EmbeddedKafkaCluster > to enable ’native’ integration in JUnit 5-based projects. > Being a committer with the JUnit 5 team I would be happy to contribute > a PR for such an extension. > Please let me know if you are interested. > Cheers, > Matthias > > > > > > > > > > > Matthias Merdes > Senior Software Architect > > > > heidelpay GmbH > Vangerowstraße 18 > 69115 Heidelberg > ------------------------------------------------------------- > +49 6221 6471-692 > matthias.mer...@heidelpay.com<mailto:matthias.mer...@heidelpay.com> > -------------------------------------------------------------- > > Geschäftsführer: Dipl. Vw. Mirko Hüllemann, > Tamara Huber, André Munk, Georg Schardt > Registergericht: AG Mannheim, HRB 337681 > > >
