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
>
