Hi, In many cases you have already serde for the stream definition. So I see natural to use it also for the tests. So you can write:
public void setup() { testDriver = new TopologyTestDriver(TestStream.getTopology(), TestStream.getConfig()); inputTopic = testDriver.createInputTopic(TestStream.INPUT_TOPIC, keySerde, myClassSerde); outputTopic = testDriver.createOutputTopic(TestStream.OUTPUT_TOPIC, k eySerde, outClassSerde); } Instead of public void setup() { testDriver = new TopologyTestDriver(TestStream.getTopology(), TestStream.getConfig()); inputTopic = testDriver.createInputTopic(TestStream.INPUT_TOPIC, keySerde.serializer(), myClassSerde.serializer()); outputTopic = testDriver.createOutputTopic(TestStream.OUTPUT_TOPIC, k eySerde.deserializer(), outClassSerde.deserializer()); } In original KIP-456 I had variation for Contructors with Serde and Serializer/Deserializer, but based on the request to keep the interface as simple as possible only version with Serde was kept. Jukka pe 13. syysk. 2019 klo 19.15 Matthias J. Sax (matth...@confluent.io) kirjoitti: > Maybe one follow up question: > > Why do we pass `Serdes` into `createInputTopic` and createOutputTopic` > -- seems `Serializer` (for input) and `Deserialized` (for output) should > be sufficient? > > > -Matthias > > On 9/12/19 4:59 PM, Matthias J. Sax wrote: > > Thanks for updating the KIP. > > > > I agree with John that we (ie, you :)) can start a vote. > > > > > > -Matthias > > > > On 9/11/19 9:23 AM, John Roesler wrote: > >> Thanks for the update, Jukka! > >> > >> I'd be in favor of the current proposal. Not sure how the others feel. > >> If people generally feel positive, it might be time to start a vote. > >> > >> Thanks, > >> -John > >> > >> On Sat, Sep 7, 2019 at 12:40 AM Jukka Karvanen > >> <jukka.karva...@jukinimi.com> wrote: > >>> > >>> Hi, > >>> > >>> Sorry; I need to rollback right away the one method removal what I was > >>> proposing. > >>> > >>> One constructor with Long restored to TestRecord, which is needed by > >>> TestInputTopic. > >>> > >>> Regards, > >>> Jukka > >>> > >>> la 7. syysk. 2019 klo 8.06 Jukka Karvanen (jukka.karva...@jukinimi.com > ) > >>> kirjoitti: > >>> > >>>> Hi, > >>>> > >>>> Let's get back to this after summer break. > >>>> Thanks Antony to offering help, it might be needed. > >>>> > >>>> I modified the KIP based on the feedback to be a mixture of > variations 4 > >>>> and 5. > >>>> > >>>> In TestInputTopic I removed deprecation from one variation with long > >>>> timestamp and removed totally one version without key. > >>>> The existing test code with it can be easily migrated to use remaining > >>>> method adding null key. > >>>> > >>>> In TestRecord I removed constructors with Long timestamp from the > public > >>>> interface. You can migrate existing code > >>>> with Long timestamp constructors to use constructors with > ProducerRecord > >>>> or ConsumerRecord. > >>>> There is still Long timestamp(); method like in ProducerRecord / > >>>> ConsumerRecord. > >>>> > >>>> Is this acceptable alternative? > >>>> What else is needed to conclude the discussion phase and get to > voting? > >>>> > >>>> Regards, > >>>> Jukka > >>>> > >>>> to 5. syysk. 2019 klo 17.17 Antony Stubbs (ant...@confluent.io) > kirjoitti: > >>>> > >>>>> Hi Jukka! I just came across your work - it looks great! I was > taking a > >>>>> stab at improving the existing API, but yours already looks great > and just > >>>>> about complete! Are you planning on continuing your work and > submitting a > >>>>> PR? If you want some help, I'd be happy to jump in. > >>>>> > >>>>> Regards, > >>>>> Antony. > >>>>> > >>>>> On Thu, Aug 1, 2019 at 3:42 PM Bill Bejeck <bbej...@gmail.com> > wrote: > >>>>> > >>>>>> Hi Jukka, > >>>>>> > >>>>>> I also think 3, 4, and 5 are all good options. > >>>>>> > >>>>>> My personal preference is 4, but I also wouldn't mind going with 5 > if > >>>>> that > >>>>>> is what others want to do. > >>>>>> > >>>>>> Thanks, > >>>>>> Bill > >>>>>> > >>>>>> On Tue, Jul 30, 2019 at 9:31 AM John Roesler <j...@confluent.io> > wrote: > >>>>>> > >>>>>>> Hey Jukka, > >>>>>>> > >>>>>>> Sorry for the delay. > >>>>>>> > >>>>>>> For what it's worth, I think 3, 4, and 5 are all good options. I > >>>>> guess my > >>>>>>> own preference is 5. > >>>>>>> > >>>>>>> It seems like the migration pain is a one-time concern vs. having > more > >>>>>>> maintainable code for years thereafter. > >>>>>>> > >>>>>>> Thanks, > >>>>>>> -John > >>>>>>> > >>>>>>> > >>>>>>> > >>>>>>> On Tue, Jul 2, 2019 at 4:03 AM Jukka Karvanen < > >>>>>> jukka.karva...@jukinimi.com > >>>>>>>> > >>>>>>> wrote: > >>>>>>> > >>>>>>>> Hi Matthias, > >>>>>>>> > >>>>>>>> Generally I think using Instant and Duration make the test more > >>>>>> readable > >>>>>>>> and that's why I modified KIP based on your suggestion. > >>>>>>>> Now a lot of code use time with long or Long and that make the > >>>>> change > >>>>>>> more > >>>>>>>> complicated. > >>>>>>>> > >>>>>>>> What I tried to say about the migration is the lines without > >>>>> timestamp > >>>>>> or > >>>>>>>> if long timestamp is supported can be migrated mainly with search > & > >>>>>>>> recplace: > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>> > >>>>> > testDriver.pipeInput(recordFactory.create(WordCountLambdaExample.inputTopic, > >>>>>>>> nullKey, "Hello", 1L)); > >>>>>>>> > >>>>>>>> -> > >>>>>>>> > >>>>>>>> *inputTopic*.pipeInput(nullKey, "Hello", 1L); > >>>>>>>> > >>>>>>>> If long is not supported as timestamp, the same is not so easy: > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>> > >>>>> > testDriver.pipeInput(recordFactory.create(WordCountLambdaExample.inputTopic, > >>>>>>>> nullKey, "Hello", 1L)); > >>>>>>>> > >>>>>>>> -> > >>>>>>>> > >>>>>>>> *inputTopic1*.pipeInput(nullKey, "Hello", > Instant.ofEpochMilli(1L)); > >>>>>>>> > >>>>>>>> Also if you need to convert arbitrary long timestamps to proper > time > >>>>>>>> Instants, it require you need to understand the logic of the test. > >>>>> So > >>>>>>>> mechanical search & replace is not possible. > >>>>>>>> > >>>>>>>> > >>>>>>>> I see there are several alternatives for long vs Instant / > Duration: > >>>>>>>> > >>>>>>>> 1. All times as long/Long like in this version: > >>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>> > >>>>> > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=119550011 > >>>>>>>> > >>>>>>>> (startTimestampMs, autoAdvanceMs as parameter of createInputTopic > >>>>>>>> instead of configureTiming) > >>>>>>>> > >>>>>>>> 2. Auto timestamping configured with Instant and Duration, > pipeInput > >>>>>>>> and TestRecord with long: > >>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>> > >>>>> > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=120722523 > >>>>>>>> > >>>>>>>> > >>>>>>>> 3. (CURRENT) Auto timestamping configured with Instant and > Duration, > >>>>>>>> pipeInput and TestRecord with Instant, version with long > deprecated: > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>> > >>>>> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-470%3A+TopologyTestDriver+test+input+and+output+usability+improvements > >>>>>>>> > >>>>>>>> > >>>>>>>> 4. Auto timestamping configured with Instant and Duration, > pipeInput > >>>>>>>> and TestRecord with Instant and long parallel (with long not > >>>>>>>> deprecated): > >>>>>>>> > >>>>>>>> 5. Auto timestamping configured with Instant and Duration, > pipeInput > >>>>>>>> and TestRecord with Instant only > >>>>>>>> > >>>>>>>> I hope to get feedback. > >>>>>>>> > >>>>>>>> My own preference currently is alternative 3. or 4. > >>>>>>>> > >>>>>>>> > >>>>>>>> If somebody want to test, there is a implementation of this > version > >>>>>>>> available in Github: > >>>>>>>> > >>>>>>>> https://github.com/jukkakarvanen/kafka-streams-test-topics > >>>>>>>> > >>>>>>>> which can be used directly from public Maven repository: > >>>>>>>> > >>>>>>>> <dependency> > >>>>>>>> <groupId>com.github.jukkakarvanen</groupId> > >>>>>>>> <artifactId>kafka-streams-test-topics</artifactId> > >>>>>>>> <version>0.0.1-beta3</version> > >>>>>>>> <scope>test</scope> > >>>>>>>> </dependency> > >>>>>>>> > >>>>>>>> Also is this approach in KIP-470 preferred over KIP-456, so can we > >>>>>> close > >>>>>>>> it: > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>> > >>>>> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-456%3A+Helper+classes+to+make+it+simpler+to+write+test+logic+with+TopologyTestDriver > >>>>>>>> > >>>>>>>> Jukka > >>>>>>>> > >>>>>>>> . > >>>>>>>> > >>>>>>>> > >>>>>>>> pe 28. kesäk. 2019 klo 1.10 Matthias J. Sax ( > matth...@confluent.io) > >>>>>>>> kirjoitti: > >>>>>>>> > >>>>>>>>> Thanks Jukka! > >>>>>>>>> > >>>>>>>>> The idea to use `Instant/Duration` was a proposal. If we think > >>>>> it's > >>>>>> not > >>>>>>>>> a good one, we could still stay with `long`. Because > >>>>> `ProducerRecord` > >>>>>>>>> and `ConsumerRecord` are both based on `long,` it might make > >>>>> sense to > >>>>>>>>> keep `long`? > >>>>>>>>> > >>>>>>>>>> The result of converting millis to Instant directly generates > >>>>>>>>>>> rather non readable test code and changing from long to Instant > >>>>>>>>> correctly > >>>>>>>>>>> require understand what is the case it is testing. > >>>>>>>>> > >>>>>>>>> This might be a good indicator the using `Instant/Duration` might > >>>>> not > >>>>>>> be > >>>>>>>>> a good idea. > >>>>>>>>> > >>>>>>>>> Would be nice to get feedback from others. > >>>>>>>>> > >>>>>>>>> About adding new methods that we deprecate immediately: I don't > >>>>> think > >>>>>>> we > >>>>>>>>> should do this. IMHO, there are two kind of users, one that > >>>>>> immediately > >>>>>>>>> rewrite their code to move off deprecated methods. Those won't > use > >>>>>> the > >>>>>>>>> new+deprecated ones anyway. Other uses migrate their code slowly > >>>>> and > >>>>>>>>> would just not rewrite their code at all, and thus also not use > >>>>> the > >>>>>>>>> new+deprecated methods. > >>>>>>>>> > >>>>>>>>>> Checking my own tests I was able to migrate the most of my code > >>>>>> with > >>>>>>>>>> search&replace without further thinking about the logic to this > >>>>> new > >>>>>>>>>> approach. The result of converting millis to Instant directly > >>>>>>> generates > >>>>>>>>>> rather non readable test code and changing from long to Instant > >>>>>>>> correctly > >>>>>>>>>> require understand what is the case it is testing. > >>>>>>>>> > >>>>>>>>> Not sure if I can follow here. You first say, you could easily > >>>>>> migrate > >>>>>>>>> your code, but than you say it was not easily possible? Can you > >>>>>> clarify > >>>>>>>>> your experience upgrading your test code? > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> -Matthias > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> On 6/27/19 12:21 AM, Jukka Karvanen wrote: > >>>>>>>>>> Hi, > >>>>>>>>>> > >>>>>>>>>>>> (4) Should we switch from `long` for timestamps to `Instant` > >>>>> and > >>>>>>>>>> `Duration` ? > >>>>>>>>>>> This version startTimestamp is Instant and autoAdvance > >>>>> Duration in > >>>>>>>>>> Initialization and with manual configured collection pipe > >>>>> methods. > >>>>>>>>>>> Now timestamp of TestRecord is still Long and similarly single > >>>>>>> record > >>>>>>>>>> pipeInput still has long as parameter. > >>>>>>>>>>> Should these also converted to to Instant type? > >>>>>>>>>>> Should there be both long and Instant parallel? > >>>>>>>>>>> I expect there are existing test codebase which would be > >>>>> easier to > >>>>>>>>> migrate > >>>>>>>>>> if long could be still used. > >>>>>>>>>> Now added Instant version to TestRecord and pipeInput method. > >>>>>>>>>> > >>>>>>>>>> Checking my own tests I was able to migrate the most of my code > >>>>>> with > >>>>>>>>>> search&replace without further thinking about the logic to this > >>>>> new > >>>>>>>>>> approach. The result of converting millis to Instant directly > >>>>>>> generates > >>>>>>>>>> rather non readable test code and changing from long to Instant > >>>>>>>> correctly > >>>>>>>>>> require understand what is the case it is testing. > >>>>>>>>>> > >>>>>>>>>> That is why version with long left still as deprecated for > >>>>> easier > >>>>>>>>> migration > >>>>>>>>>> for existing test. > >>>>>>>>>> Also TopologyTestDriver constructor and advanceWallClockTime > >>>>>> method > >>>>>>>>>> modified with same approach. > >>>>>>>>>> > >>>>>>>>>> Jukka > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> ma 24. kesäk. 2019 klo 16.47 Bill Bejeck (bbej...@gmail.com) > >>>>>>>> kirjoitti: > >>>>>>>>>> > >>>>>>>>>>> Hi Jukka > >>>>>>>>>>> > >>>>>>>>>>>> These topic objects are only interfacing TopologyTestDriver, > >>>>> not > >>>>>>>>>>> affecting > >>>>>>>>>>>> the internal functionality of it. In my plan the internal data > >>>>>>>>> structures > >>>>>>>>>>>> are using those Producer/ConsumerRecords as earlier. That way > >>>>> I > >>>>>>> don't > >>>>>>>>> see > >>>>>>>>>>>> how those could be affected. > >>>>>>>>>>> > >>>>>>>>>>> I mistakenly thought the KIP was proposing to completely remove > >>>>>>>>>>> Producer/ConsumerRecords in favor of TestRecord. But taking > >>>>>> another > >>>>>>>>> quick > >>>>>>>>>>> look I can see the plan for using the TestRecord objects. > >>>>> Thanks > >>>>>>> for > >>>>>>>>> the > >>>>>>>>>>> clarification. > >>>>>>>>>>> > >>>>>>>>>>> -Bill > >>>>>>>>>>> > >>>>>>>>>>> On Sat, Jun 22, 2019 at 2:26 AM Jukka Karvanen < > >>>>>>>>>>> jukka.karva...@jukinimi.com> > >>>>>>>>>>> wrote: > >>>>>>>>>>> > >>>>>>>>>>>> Hi All, > >>>>>>>>>>>> Hi Matthias, > >>>>>>>>>>>> > >>>>>>>>>>>>> (1) It's a little confusing that you list all method > >>>>> (existing, > >>>>>>>>> proposed > >>>>>>>>>>>>> to deprecate, and new one) of `TopologyTestDriver` in the > >>>>> KIP. > >>>>>>> Maybe > >>>>>>>>>>>>> only list the ones you propose to deprecate and the new ones > >>>>> you > >>>>>>>> want > >>>>>>>>> to > >>>>>>>>>>>>> add? > >>>>>>>>>>>> > >>>>>>>>>>>> Ok. Unmodified methods removed. > >>>>>>>>>>>> > >>>>>>>>>>>>> (2) `TopologyTestDriver#createInputTopic`: might it be worth > >>>>> to > >>>>>>> add > >>>>>>>>>>>>> overload to initialize the timetamp and auto-advance feature > >>>>>>>> directly? > >>>>>>>>>>>>> Otherwise, uses always need to call `configureTiming` as an > >>>>>> extra > >>>>>>>>> call? > >>>>>>>>>>>> > >>>>>>>>>>>> Added with Instant and Duration parameters. > >>>>>>>>>>>> > >>>>>>>>>>>>> (3) `TestInputTopic#configureTiming()`: maybe rename to > >>>>>>>>>>>> `reconfigureTiming()` ? > >>>>>>>>>>>> > >>>>>>>>>>>> I removed this method when we have now initialization in > >>>>>>> constructor. > >>>>>>>>>>>> You can recreate TestInputTopic if needing to reconfigure > >>>>> timing. > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>>> (4) Should we switch from `long` for timestamps to `Instant` > >>>>> and > >>>>>>>>>>>> `Duration` ? > >>>>>>>>>>>> This version startTimestamp is Instant and autoAdvance > >>>>> Duration > >>>>>> in > >>>>>>>>>>>> Initialization and with manual configured collection pipe > >>>>>> methods. > >>>>>>>>>>>> Now timestamp of TestRecord is still Long and similarly single > >>>>>>> record > >>>>>>>>>>>> pipeInput still has long as parameter. > >>>>>>>>>>>> Should these also converted to to Instant type? > >>>>>>>>>>>> Should there be both long and Instant parallel? > >>>>>>>>>>>> I expect there are existing test codebase which would be > >>>>> easier > >>>>>> to > >>>>>>>>>>> migrate > >>>>>>>>>>>> if long could be still used. > >>>>>>>>>>>> > >>>>>>>>>>>> Also should advanceWallClockTime in TopologyTestDriver > >>>>>> changed(or > >>>>>>>>> added > >>>>>>>>>>>> alternative) for Duration parameter. > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>>> (5) Why do we have redundant getters? Or set with `getX()` > >>>>> and > >>>>>> one > >>>>>>>>>>>> set without `get`-prefix? > >>>>>>>>>>>> > >>>>>>>>>>>> The methods without get-prefix are for compatibility with > >>>>>>>>>>> ProducerRecord / > >>>>>>>>>>>> ConsumerRecord and I expect would make migration to TestRecord > >>>>>>>> easier. > >>>>>>>>>>>> Standard getters in TestRecord enable writing test ignoring > >>>>>>> selected > >>>>>>>>>>> fields > >>>>>>>>>>>> with hamcrest like this: > >>>>>>>>>>>> > >>>>>>>>>>>> assertThat(outputTopic.readRecord(), allOf( > >>>>>>>>>>>> hasProperty("key", equalTo(1L)), > >>>>>>>>>>>> hasProperty("value", equalTo("Hello")), > >>>>>>>>>>>> hasProperty("headers", equalTo(headers)))); > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> That's why I have currently both methods. > >>>>>>>>>>>> > >>>>>>>>>>>> Jukka > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> pe 21. kesäk. 2019 klo 22.20 Matthias J. Sax ( > >>>>>>> matth...@confluent.io) > >>>>>>>>>>>> kirjoitti: > >>>>>>>>>>>> > >>>>>>>>>>>>> Thanks for the KIP. The idea to add InputTopic and > >>>>> OutputTopic > >>>>>>>>>>>>> abstractions is really neat! > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> Couple of minor comment: > >>>>>>>>>>>>> > >>>>>>>>>>>>> (1) It's a little confusing that you list all method > >>>>> (existing, > >>>>>>>>>>> proposed > >>>>>>>>>>>>> to deprecate, and new one) of `TopologyTestDriver` in the > >>>>> KIP. > >>>>>>> Maybe > >>>>>>>>>>>>> only list the ones you propose to deprecate and the new ones > >>>>> you > >>>>>>>> want > >>>>>>>>>>> to > >>>>>>>>>>>>> add? > >>>>>>>>>>>>> > >>>>>>>>>>>>> (Or mark all existing methods clearly -- atm, I need to got > >>>>> back > >>>>>>> to > >>>>>>>>> the > >>>>>>>>>>>>> code to read the KIP and to extract what changes are > >>>>> proposed). > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> (2) `TopologyTestDriver#createInputTopic`: might it be worth > >>>>> to > >>>>>>> add > >>>>>>>>>>>>> overload to initialize the timetamp and auto-advance feature > >>>>>>>> directly? > >>>>>>>>>>>>> Otherwise, uses always need to call `configureTiming` as an > >>>>>> extra > >>>>>>>>> call? > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> (3) `TestInputTopic#configureTiming()`: maybe rename to > >>>>>>>>>>>>> `reconfigureTiming()` ? > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> (4) Should we switch from `long` for timestamps to `Instant` > >>>>> and > >>>>>>>>>>>>> `Duration` ? > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> (5) Why do we have redundant getters? Or set with `getX()` > >>>>> and > >>>>>> one > >>>>>>>> set > >>>>>>>>>>>>> without `get`-prefix? > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> -Matthias > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> On 6/21/19 10:57 AM, Bill Bejeck wrote: > >>>>>>>>>>>>>> Jukka, > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> Thanks for the KIP. I like the changes overall. > >>>>>>>>>>>>>> One thing I wanted to confirm, and this may be me being > >>>>>> paranoid, > >>>>>>>> but > >>>>>>>>>>>>> will > >>>>>>>>>>>>>> the changes for input/output topic affect how the > >>>>>>>> TopologyTestDriver > >>>>>>>>>>>>> works > >>>>>>>>>>>>>> with internal topics when there are sub-topologies created? > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> On Fri, Jun 21, 2019 at 12:05 PM Guozhang Wang < > >>>>>>> wangg...@gmail.com > >>>>>>>>> > >>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>> 1) Got it, could you list this class along with all its > >>>>>>> functions > >>>>>>>> in > >>>>>>>>>>>> the > >>>>>>>>>>>>>>> proposed public APIs as well? > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> 2) Ack, thanks! > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> On Thu, Jun 20, 2019 at 11:27 PM Jukka Karvanen < > >>>>>>>>>>>>>>> jukka.karva...@jukinimi.com> > >>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Hi Guozhang, > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> 1) This TestRecord is new class in my proposal. So it is a > >>>>>>>>>>> simplified > >>>>>>>>>>>>>>>> version of ProducerRecord and ConsumerRecord containing > >>>>> only > >>>>>>> the > >>>>>>>>>>>> fields > >>>>>>>>>>>>>>>> needed to test record content. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> 2) > >>>>>>>>>>>>>>>> public final <K, V> TestInputTopic<K, V> > >>>>>> createInputTopic(final > >>>>>>>>>>>> String > >>>>>>>>>>>>>>>> topicName, final Serde<K> keySerde, final Serde<V> > >>>>>> valueSerde); > >>>>>>>>>>>>>>>> public final <K, V> TestOutputTopic<K, V> > >>>>>>> createOutputTopic(final > >>>>>>>>>>>>> String > >>>>>>>>>>>>>>>> topicName, final Serde<K> keySerde, final Serde<V> > >>>>>> valueSerde); > >>>>>>>>>>>>>>>> The purpose is to create separate object for each input > >>>>> and > >>>>>>>> output > >>>>>>>>>>>>> topic > >>>>>>>>>>>>>>>> you are using. The topic name is given to > >>>>>>> createInput/OutputTopic > >>>>>>>>>>>> when > >>>>>>>>>>>>>>>> initialize topic object. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> For example: > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> final TestInputTopic<Long, String> inputTopic1 = > >>>>>>>>>>>>>>>> testDriver.createInputTopic( INPUT_TOPIC, longSerde, > >>>>>>>> stringSerde); > >>>>>>>>>>>>>>>> final TestInputTopic<Long, String> inputTopic2 = > >>>>>>>>>>>>>>>> testDriver.createInputTopic( INPUT_TOPIC_MAP, longSerde, > >>>>>>>>>>>> stringSerde); > >>>>>>>>>>>>>>>> final TestOutputTopic<Long, String> outputTopic1 = > >>>>>>>>>>>>>>>> testDriver.createOutputTopic(OUTPUT_TOPIC, longSerde, > >>>>>>>> stringSerde); > >>>>>>>>>>>>>>>> final TestOutputTopic<String, Long> outputTopic2 = > >>>>>>>>>>>>>>>> testDriver.createOutputTopic(OUTPUT_TOPIC_MAP, > >>>>> stringSerde, > >>>>>>>>>>>>>>>> longSerde); > >>>>>>>>>>>>>>>> inputTopic1.pipeInput(1L, "Hello"); > >>>>>>>>>>>>>>>> assertThat(outputTopic1.readKeyValue(), equalTo(new > >>>>>>>> KeyValue<>(1L, > >>>>>>>>>>>>>>>> "Hello"))); > >>>>>>>>>>>>>>>> assertThat(outputTopic2.readKeyValue(), equalTo(new > >>>>>>>>>>>> KeyValue<>("Hello", > >>>>>>>>>>>>>>>> 1L))); > >>>>>>>>>>>>>>>> inputTopic2.pipeInput(1L, "Hello"); > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Jukka > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> to 20. kesäk. 2019 klo 23.52 Guozhang Wang ( > >>>>>> wangg...@gmail.com > >>>>>>> ) > >>>>>>>>>>>>>>> kirjoitti: > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Hello Jukka, > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Thanks for writing the KIP, I have a couple of quick > >>>>>>> questions: > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> 1) Is "TestRecord" an existing class that you propose to > >>>>>>>>>>> piggy-back > >>>>>>>>>>>>> on? > >>>>>>>>>>>>>>>>> Right now we have a scala TestRecord case class but I > >>>>> doubt > >>>>>>> that > >>>>>>>>>>> was > >>>>>>>>>>>>>>> your > >>>>>>>>>>>>>>>>> proposal, or are you proposing to add a new Java class? > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> 2) Would the new API only allow a single input / output > >>>>>> topic > >>>>>>>> with > >>>>>>>>>>>>>>>>> `createInput/OutputTopic`? If not, when we call pipeInput > >>>>>> how > >>>>>>> to > >>>>>>>>>>>>>>>> determine > >>>>>>>>>>>>>>>>> which topic this record should be pipe to? > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Guozhang > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> On Mon, Jun 17, 2019 at 1:34 PM John Roesler < > >>>>>>> j...@confluent.io > >>>>>>>>> > >>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> Woah, I wasn't aware of that Hamcrest test style. > >>>>> Awesome! > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> Thanks for the updates. I look forward to hearing what > >>>>>> others > >>>>>>>>>>>> think. > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> -John > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> On Mon, Jun 17, 2019 at 4:12 AM Jukka Karvanen > >>>>>>>>>>>>>>>>>> <jukka.karva...@jukinimi.com> wrote: > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> Wiki page updated: > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>> > >>>>> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-470%3A+TopologyTestDriver+test+input+and+output+usability+improvements > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> ClientRecord removed and replaced with TestRecord in > >>>>>> method > >>>>>>>>>>> calls. > >>>>>>>>>>>>>>>>>>> TestRecordFactory removed (time tracking functionality > >>>>> to > >>>>>> be > >>>>>>>>>>>>>>> included > >>>>>>>>>>>>>>>>> to > >>>>>>>>>>>>>>>>>>> TestInputTopic) > >>>>>>>>>>>>>>>>>>> OutputVerifier deprecated > >>>>>>>>>>>>>>>>>>> TestRecord topic removed and getters added > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> Getters in TestRecord enable writing test ignoring > >>>>>> selected > >>>>>>>>>>> fields > >>>>>>>>>>>>>>>> with > >>>>>>>>>>>>>>>>>>> hamcrest like this: > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> assertThat(outputTopic.readRecord(), allOf( > >>>>>>>>>>>>>>>>>>> hasProperty("key", equalTo(1L)), > >>>>>>>>>>>>>>>>>>> hasProperty("value", equalTo("Hello")), > >>>>>>>>>>>>>>>>>>> hasProperty("headers", equalTo(headers)))); > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> Jukka > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> la 15. kesäk. 2019 klo 1.10 John Roesler ( > >>>>>> j...@confluent.io > >>>>>>> ) > >>>>>>>>>>>>>>>>> kirjoitti: > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> Sounds good. Thanks as always for considering my > >>>>>> feedback! > >>>>>>>>>>>>>>>>>>>> -John > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> On Fri, Jun 14, 2019 at 12:12 PM Jukka Karvanen > >>>>>>>>>>>>>>>>>>>> <jukka.karva...@jukinimi.com> wrote: > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> Ok, I will modify KIP Public Interface in a wiki > >>>>> based > >>>>>> on > >>>>>>>> the > >>>>>>>>>>>>>>>>>> feedback. > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> TestRecordFactory / ConsumerRecordFactory was used by > >>>>>>>>>>>>>>>>> TestInputTopic > >>>>>>>>>>>>>>>>>> with > >>>>>>>>>>>>>>>>>>>>> the version I had with KIP456, but maybe I can merge > >>>>>> That > >>>>>>>>>>>>>>>>>> functionality > >>>>>>>>>>>>>>>>>>>> to > >>>>>>>>>>>>>>>>>>>>> InputTopic or TestRecordFactory can kept non > >>>>> public > >>>>>>> maybe > >>>>>>>>>>>>>>>> moving > >>>>>>>>>>>>>>>>>> it to > >>>>>>>>>>>>>>>>>>>>> internals package. > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> I will make the proposal with a slim down interface. > >>>>>>>>>>>>>>>>>>>>> I don't want to go to so slim as you proposed with > >>>>> only > >>>>>>>>>>>>>>>> TestRecord > >>>>>>>>>>>>>>>>> or > >>>>>>>>>>>>>>>>>>>>> List<TestRecord>, because you then still end up doing > >>>>>>> helper > >>>>>>>>>>>>>>>>> methods > >>>>>>>>>>>>>>>>>> to > >>>>>>>>>>>>>>>>>>>>> construct List of TestRecord. > >>>>>>>>>>>>>>>>>>>>> The list of values is easier to write and clearer to > >>>>>> read > >>>>>>>> than > >>>>>>>>>>>>>>> if > >>>>>>>>>>>>>>>>> you > >>>>>>>>>>>>>>>>>>>> need > >>>>>>>>>>>>>>>>>>>>> to contruct list of TestRecords. > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> For example: > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> final List<String> inputValues = Arrays.asList( > >>>>>>>>>>>>>>>>>>>>> "Apache Kafka Streams Example", > >>>>>>>>>>>>>>>>>>>>> "Using Kafka Streams Test Utils", > >>>>>>>>>>>>>>>>>>>>> "Reading and Writing Kafka Topic" > >>>>>>>>>>>>>>>>>>>>> ); > >>>>>>>>>>>>>>>>>>>>> inputTopic.pipeValueList(inputValues); > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> Let's check after the next iteration is it still > >>>>> worth > >>>>>>>>>>> reducing > >>>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>> methods. > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> Jukka > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> pe 14. kesäk. 2019 klo 18.27 John Roesler ( > >>>>>>>> j...@confluent.io) > >>>>>>>>>>>>>>>>>> kirjoitti: > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> Thanks, Jukka, > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> Ok, I buy this reasoning. > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> Just to echo what I think I read, you would just > >>>>> drop > >>>>>>>>>>>>>>>>> ClientRecord > >>>>>>>>>>>>>>>>>>>>>> from the proposal, and TestRecord would stand on its > >>>>>> own, > >>>>>>>>>>>>>>> with > >>>>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>>>> same methods and properties you proposed, and the > >>>>>> "input > >>>>>>>>>>>>>>> topic" > >>>>>>>>>>>>>>>>>> would > >>>>>>>>>>>>>>>>>>>>>> accept TestRecord, and the "output topic" would > >>>>> produce > >>>>>>>>>>>>>>>>> TestRecord? > >>>>>>>>>>>>>>>>>>>>>> Further, the "input and output topic" classes would > >>>>>>>>>>>>>>> internally > >>>>>>>>>>>>>>>>>> handle > >>>>>>>>>>>>>>>>>>>>>> the conversion to and from ConsumerRecord and > >>>>>>>> ProducerRecord > >>>>>>>>>>>>>>> to > >>>>>>>>>>>>>>>>>> pass > >>>>>>>>>>>>>>>>>>>>>> to and from the TopologyTestDriver? > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> This seems good to me. > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> Since the object coming out of the "output topic" is > >>>>>> much > >>>>>>>>>>>>>>> more > >>>>>>>>>>>>>>>>>>>>>> ergonomic, I suspect we won't need the > >>>>> OutputVerifier > >>>>>> at > >>>>>>>> all. > >>>>>>>>>>>>>>>> It > >>>>>>>>>>>>>>>>>> was > >>>>>>>>>>>>>>>>>>>>>> mostly needed because of all the extra junk in > >>>>>>>> ProducerRecord > >>>>>>>>>>>>>>>> you > >>>>>>>>>>>>>>>>>> need > >>>>>>>>>>>>>>>>>>>>>> to ignore. It seems better to just deprecate it. If > >>>>> in > >>>>>>> the > >>>>>>>>>>>>>>>> future > >>>>>>>>>>>>>>>>>> it > >>>>>>>>>>>>>>>>>>>>>> turns out there is some actual use case for a > >>>>> verifier, > >>>>>>> we > >>>>>>>>>>>>>>> can > >>>>>>>>>>>>>>>>>> have a > >>>>>>>>>>>>>>>>>>>>>> very small KIP to add one. But reading your > >>>>> response, I > >>>>>>>>>>>>>>> suspect > >>>>>>>>>>>>>>>>>> that > >>>>>>>>>>>>>>>>>>>>>> existing test verification libraries would be > >>>>>> sufficient > >>>>>>> on > >>>>>>>>>>>>>>>> their > >>>>>>>>>>>>>>>>>> own. > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> Similarly, it seems like we can shrink the total > >>>>>>> interface > >>>>>>>> by > >>>>>>>>>>>>>>>>>> removing > >>>>>>>>>>>>>>>>>>>>>> the TestRecordFactory from the proposal. If > >>>>> TestRecord > >>>>>>>>>>>>>>> already > >>>>>>>>>>>>>>>>>> offers > >>>>>>>>>>>>>>>>>>>>>> all the constructors you'd want, then the only > >>>>> benefit > >>>>>> of > >>>>>>>> the > >>>>>>>>>>>>>>>>>> factory > >>>>>>>>>>>>>>>>>>>>>> is to auto-increment the timestamps, but then again, > >>>>>> the > >>>>>>>>>>>>>>> "input > >>>>>>>>>>>>>>>>>> topic" > >>>>>>>>>>>>>>>>>>>>>> can already do that (e.g., it can do it if the > >>>>> record > >>>>>>>>>>>>>>> timestamp > >>>>>>>>>>>>>>>>> is > >>>>>>>>>>>>>>>>>> not > >>>>>>>>>>>>>>>>>>>>>> set). > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> Likewise, if the TestRecords are easy to create, > >>>>> then > >>>>>> we > >>>>>>>>>>>>>>> don't > >>>>>>>>>>>>>>>>> need > >>>>>>>>>>>>>>>>>>>>>> all the redundant methods in "input topic" to pipe > >>>>>>> values, > >>>>>>>> or > >>>>>>>>>>>>>>>>>>>>>> key/values, or key/value/timestamp, etc. We can do > >>>>> with > >>>>>>>> just > >>>>>>>>>>>>>>>> two > >>>>>>>>>>>>>>>>>>>>>> methods, one for a single TestRecord and one for a > >>>>>>>> collection > >>>>>>>>>>>>>>>> of > >>>>>>>>>>>>>>>>>> them. > >>>>>>>>>>>>>>>>>>>>>> This reduces API ambiguity and also dramatically > >>>>>>> decreases > >>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>> surface > >>>>>>>>>>>>>>>>>>>>>> area of the interface, which ultimately makes it > >>>>> much > >>>>>>>> easier > >>>>>>>>>>>>>>> to > >>>>>>>>>>>>>>>>>> use. > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> It's best to start with the smallest interface that > >>>>>> will > >>>>>>> do > >>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>> job > >>>>>>>>>>>>>>>>>>>>>> and expand it upon request, rather than throwing in > >>>>>>>>>>>>>>> everything > >>>>>>>>>>>>>>>>> you > >>>>>>>>>>>>>>>>>> can > >>>>>>>>>>>>>>>>>>>>>> think of up front. The extra stuff would be > >>>>> confusing > >>>>>> to > >>>>>>>>>>>>>>> people > >>>>>>>>>>>>>>>>>> facing > >>>>>>>>>>>>>>>>>>>>>> two practically identical paths to accomplish the > >>>>> same > >>>>>>>> goal, > >>>>>>>>>>>>>>>> and > >>>>>>>>>>>>>>>>>> it's > >>>>>>>>>>>>>>>>>>>>>> very difficult to slim the interface down later, > >>>>>> because > >>>>>>> we > >>>>>>>>>>>>>>>> don't > >>>>>>>>>>>>>>>>>>>>>> really know which parts are more popular (i.e., no > >>>>> one > >>>>>>>>>>>>>>> submits > >>>>>>>>>>>>>>>>>>>>>> "feature requests" to _remove_ stuff they don't > >>>>> need, > >>>>>>> only > >>>>>>>> to > >>>>>>>>>>>>>>>>> _add_ > >>>>>>>>>>>>>>>>>>>>>> stuff that they need. > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> But overall, I really like the structure of this > >>>>>> design. > >>>>>>>> I'm > >>>>>>>>>>>>>>>>> super > >>>>>>>>>>>>>>>>>>>>>> excited about this KIP. > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> Thanks, > >>>>>>>>>>>>>>>>>>>>>> -John > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> On Fri, Jun 14, 2019 at 2:55 AM Jukka Karvanen > >>>>>>>>>>>>>>>>>>>>>> <jukka.karva...@jukinimi.com> wrote: > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> Hi, > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> I am not a fan of swapping only ProducerRecord and > >>>>>>>>>>>>>>>>>> ConsumerRecord. > >>>>>>>>>>>>>>>>>>>>>>> As a test writer point of view I do not want to > >>>>> care > >>>>>>> about > >>>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>> difference > >>>>>>>>>>>>>>>>>>>>>>> of those and > >>>>>>>>>>>>>>>>>>>>>>> that way I like to have object type which can be > >>>>> used > >>>>>> to > >>>>>>>>>>>>>>> pipe > >>>>>>>>>>>>>>>>>>>> records in > >>>>>>>>>>>>>>>>>>>>>>> and compare outputs. > >>>>>>>>>>>>>>>>>>>>>>> That way avoid unnecessary conversions between > >>>>>>>>>>>>>>> ProducerRecord > >>>>>>>>>>>>>>>>> and > >>>>>>>>>>>>>>>>>>>>>>> ConsumerRecord. > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> My initial assumption was that ProducerRecord and > >>>>>>>>>>>>>>>>>>>> ConsumerRecord.could > >>>>>>>>>>>>>>>>>>>>>>> implement the same ClientRecord > >>>>>>>>>>>>>>>>>>>>>>> and that way test writer could have used either of > >>>>>>> those, > >>>>>>>>>>>>>>> but > >>>>>>>>>>>>>>>>>> seems > >>>>>>>>>>>>>>>>>>>> that > >>>>>>>>>>>>>>>>>>>>>>> return type of timestamp method long vs Long is not > >>>>>>>>>>>>>>>> compatible. > >>>>>>>>>>>>>>>>>>>>>>> Now the main advantage of ClientRecord is no need > >>>>> to > >>>>>>>>>>>>>>>> duplicate > >>>>>>>>>>>>>>>>>>>>>>> OutputVerifier when it is modified from > >>>>> ProducerRecord > >>>>>>> to > >>>>>>>>>>>>>>>>>>>> ClientRecord. > >>>>>>>>>>>>>>>>>>>>>>> Generally is there need for OutputVerifier. Can we > >>>>>>>>>>>>>>> deprecate > >>>>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>> existing > >>>>>>>>>>>>>>>>>>>>>>> and use standard assertion libraries for new test. > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> If you use hamcrest, assert-j or any other > >>>>> assertion > >>>>>>>>>>>>>>> library > >>>>>>>>>>>>>>>>>> for the > >>>>>>>>>>>>>>>>>>>>>> rest > >>>>>>>>>>>>>>>>>>>>>>> of the test, why not use it with these also. > >>>>>>>>>>>>>>>>>>>>>>> When we have these methods to access only needed > >>>>>> fields > >>>>>>> it > >>>>>>>>>>>>>>> is > >>>>>>>>>>>>>>>>>> easier > >>>>>>>>>>>>>>>>>>>> to > >>>>>>>>>>>>>>>>>>>>>>> write test like this: > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>> assertThat(outputTopic.readValue()).isEqualTo("Hello"); > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> or > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>> assertThat(outputTopic.readRecord()).isEqualTo(expectedRecord); > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> Only value for new OutputVerifier is when needing > >>>>> to > >>>>>>>> ignore > >>>>>>>>>>>>>>>>> some > >>>>>>>>>>>>>>>>>>>> fields > >>>>>>>>>>>>>>>>>>>>>>> ClientRecord actual = outputTopic.readRecord(); > >>>>>>>>>>>>>>>>>>>>>>> assertThat(actual.value()).isEqualTo("Hello"); > >>>>>>>>>>>>>>>>>>>>>>> assertThat(actual.key()).isEqualTo(expectedKey); > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>> assertThat(actual.timestamp()).isEqualTo(expectedTimestamp); > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> So if want to leave client package untouched, I > >>>>> would > >>>>>>>>>>>>>>> modify > >>>>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>> methods > >>>>>>>>>>>>>>>>>>>>>>> with ClientRecord now in InputTopic and > >>>>> OutputTopic to > >>>>>>>> pass > >>>>>>>>>>>>>>>> in > >>>>>>>>>>>>>>>>>> and > >>>>>>>>>>>>>>>>>>>> out > >>>>>>>>>>>>>>>>>>>>>> this > >>>>>>>>>>>>>>>>>>>>>>> TestRecord instead. > >>>>>>>>>>>>>>>>>>>>>>> In that case there would be possibility to add > >>>>> methods > >>>>>>> to > >>>>>>>>>>>>>>>>>> TestRecord > >>>>>>>>>>>>>>>>>>>> to > >>>>>>>>>>>>>>>>>>>>>>> help ignore some fields in assertions like: > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>> > >>>>> > assertThat(outputTopic.readRecord().getValueTimestamp()).isEqualTo(expectedRecord.get > >>>>>>>>>>>>>>>>>>>>>>> ValueTimestamp()); > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> How about this alternative? > >>>>>>>>>>>>>>>>>>>>>>> If this way sounds better I will modify KIP page in > >>>>>>> wiki. > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> Jukka > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> to 13. kesäk. 2019 klo 18.30 John Roesler ( > >>>>>>>>>>>>>>> j...@confluent.io > >>>>>>>>>>>>>>>> ) > >>>>>>>>>>>>>>>>>>>> kirjoitti: > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> Hey, all, maybe we can jump-start this discussion. > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> I think this approach would be very ergonomic for > >>>>>>>>>>>>>>> testing, > >>>>>>>>>>>>>>>>> and > >>>>>>>>>>>>>>>>>>>> would > >>>>>>>>>>>>>>>>>>>>>>>> help reduce boilerplate in tests. > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> The think I like most about it is that it mirrors > >>>>> the > >>>>>>>>>>>>>>>> mental > >>>>>>>>>>>>>>>>>> model > >>>>>>>>>>>>>>>>>>>>>>>> that people already have from Kafka Streams, in > >>>>> which > >>>>>>> you > >>>>>>>>>>>>>>>>>> write to > >>>>>>>>>>>>>>>>>>>> an > >>>>>>>>>>>>>>>>>>>>>>>> "input topic" and then get your results from an > >>>>>> "output > >>>>>>>>>>>>>>>>>> topic". As > >>>>>>>>>>>>>>>>>>>> a > >>>>>>>>>>>>>>>>>>>>>>>> side benefit, the input and output topics in the > >>>>>>> proposal > >>>>>>>>>>>>>>>>> also > >>>>>>>>>>>>>>>>>>>> close > >>>>>>>>>>>>>>>>>>>>>>>> over the serdes, which makes it much less > >>>>> boilerplate > >>>>>>> for > >>>>>>>>>>>>>>>>> test > >>>>>>>>>>>>>>>>>>>> code. > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> If I can offer one suggestion for change, I'm not > >>>>>> sure > >>>>>>>>>>>>>>> I'm > >>>>>>>>>>>>>>>>>> totally > >>>>>>>>>>>>>>>>>>>>>>>> sold on the need for a new abstraction > >>>>> "ClientRecord" > >>>>>>>>>>>>>>> with > >>>>>>>>>>>>>>>> an > >>>>>>>>>>>>>>>>>>>>>>>> implementation for tests "TestRecord". It seems > >>>>> like > >>>>>>> this > >>>>>>>>>>>>>>>>>>>>>>>> unnecessarily complicates the main (non-testing) > >>>>> data > >>>>>>>>>>>>>>>> model. > >>>>>>>>>>>>>>>>> It > >>>>>>>>>>>>>>>>>>>> seems > >>>>>>>>>>>>>>>>>>>>>>>> like it would be sufficient, and just as > >>>>> ergonomic, > >>>>>> to > >>>>>>>>>>>>>>> have > >>>>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>> input > >>>>>>>>>>>>>>>>>>>>>>>> topic accept ProducerRecords and the output topic > >>>>>>> return > >>>>>>>>>>>>>>>>>>>>>>>> ConsumerRecords. I'm open to discussion on this > >>>>>> point, > >>>>>>>>>>>>>>>>> though. > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> Thanks for the proposal, Jukka! > >>>>>>>>>>>>>>>>>>>>>>>> -John > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> On Mon, May 20, 2019 at 7:59 AM Jukka Karvanen > >>>>>>>>>>>>>>>>>>>>>>>> <jukka.karva...@jukinimi.com> wrote: > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>> Hi All, > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>> I would like to start the discussion on KIP-470: > >>>>>>>>>>>>>>>>>>>> TopologyTestDriver > >>>>>>>>>>>>>>>>>>>>>> test > >>>>>>>>>>>>>>>>>>>>>>>>> input and output usability improvements: > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>> > >>>>> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-470%3A+TopologyTestDriver+test+input+and+output+usability+improvements > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>> This KIP is inspired by the Discussion in > >>>>> KIP-456: > >>>>>>>>>>>>>>> Helper > >>>>>>>>>>>>>>>>>>>> classes to > >>>>>>>>>>>>>>>>>>>>>> make > >>>>>>>>>>>>>>>>>>>>>>>>> it simpler to write test logic with > >>>>>>> TopologyTestDriver: > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-456 > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>> > >>>>> > %3A+Helper+classes+to+make+it+simpler+to+write+test+logic+with+TopologyTestDriver > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>> The proposal in KIP-456 > >>>>>>>>>>>>>>>>>>>>>>>>> < > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>> > >>>>> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-456%3A+Helper+classes+to+make+it+simpler+to+write+test+logic+with+TopologyTestDriver > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>> was > >>>>>>>>>>>>>>>>>>>>>>>>> to add alternate way to input and output topic, > >>>>> but > >>>>>>>>>>>>>>> this > >>>>>>>>>>>>>>>>> KIP > >>>>>>>>>>>>>>>>>>>> enhance > >>>>>>>>>>>>>>>>>>>>>>>> those > >>>>>>>>>>>>>>>>>>>>>>>>> classes and deprecate old functionality to make > >>>>>> clear > >>>>>>>>>>>>>>>>>> interface > >>>>>>>>>>>>>>>>>>>> for > >>>>>>>>>>>>>>>>>>>>>> test > >>>>>>>>>>>>>>>>>>>>>>>>> writer to use. > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>> In current KIP-470 proposal, topic objects are > >>>>>> created > >>>>>>>>>>>>>>>> with > >>>>>>>>>>>>>>>>>>>>>> topicName and > >>>>>>>>>>>>>>>>>>>>>>>>> related serders. > >>>>>>>>>>>>>>>>>>>>>>>>> public final <K, V> TestInputTopic<K, V> > >>>>>>>>>>>>>>>>>>>> createInputTopic(final > >>>>>>>>>>>>>>>>>>>>>>>> String > >>>>>>>>>>>>>>>>>>>>>>>>> topicName, final Serde<K> keySerde, final > >>>>> Serde<V> > >>>>>>>>>>>>>>>>>> valueSerde); > >>>>>>>>>>>>>>>>>>>>>>>>> public final <K, V> TestOutputTopic<K, V> > >>>>>>>>>>>>>>>>>>>> createOutputTopic(final > >>>>>>>>>>>>>>>>>>>>>>>> String > >>>>>>>>>>>>>>>>>>>>>>>>> topicName, final Serde<K> keySerde, final > >>>>> Serde<V> > >>>>>>>>>>>>>>>>>> valueSerde); > >>>>>>>>>>>>>>>>>>>>>>>>> One thing I wondered if there way to find out > >>>>> topic > >>>>>>>>>>>>>>>> related > >>>>>>>>>>>>>>>>>> serde > >>>>>>>>>>>>>>>>>>>>>> from > >>>>>>>>>>>>>>>>>>>>>>>>> TopologyTestDriver topology, it would simply > >>>>>> creation > >>>>>>>>>>>>>>> of > >>>>>>>>>>>>>>>>>> these > >>>>>>>>>>>>>>>>>>>> Topic > >>>>>>>>>>>>>>>>>>>>>>>>> objects: > >>>>>>>>>>>>>>>>>>>>>>>>> public final <K, V> TestInputTopic<K, V> > >>>>>>>>>>>>>>>>>>>> createInputTopic(final > >>>>>>>>>>>>>>>>>>>>>>>> String > >>>>>>>>>>>>>>>>>>>>>>>>> topicName); > >>>>>>>>>>>>>>>>>>>>>>>>> public final <K, V> TestOutputTopic<K, V> > >>>>>>>>>>>>>>>>>>>> createOutputTopic(final > >>>>>>>>>>>>>>>>>>>>>>>> String > >>>>>>>>>>>>>>>>>>>>>>>>> topicName); > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>> KIP-456 can be discarded if this KIP get > >>>>> accepted. > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>> Best Regards, > >>>>>>>>>>>>>>>>>>>>>>>>> Jukka > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> -- > >>>>>>>>>>>>>>>>> -- Guozhang > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> -- > >>>>>>>>>>>>>>> -- Guozhang > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>> > >>>>> > >>>>> > >>>>> -- > >>>>> > >>>>> Antony Stubbs > >>>>> > >>>>> Solutions Architect | Confluent > >>>>> > >>>>> +44 (0)7491 833 814 <+447491833814> > >>>>> Follow us: Twitter <https://twitter.com/ConfluentInc> | blog > >>>>> <http://www.confluent.io/blog> > >>>>> > >>>> > >> > > > >