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
>

Reply via email to