salvalcantara commented on code in PR #21186:
URL: https://github.com/apache/flink/pull/21186#discussion_r1009072959
##########
flink-connectors/flink-connector-kafka/src/test/java/org/apache/flink/connector/kafka/sink/KafkaWriterITCase.java:
##########
@@ -148,6 +148,15 @@ public void testIncreasingRecordBasedCounters() throws
Exception {
assertThat(numRecordsOutErrors.getCount()).isEqualTo(0);
assertThat(numRecordsSendErrors.getCount()).isEqualTo(0);
+ // elements that cannot be serialized should be silently skipped
+ writer.write(null, SINK_WRITER_CONTEXT);
+ timeService.trigger();
+ assertThat(numBytesOut.getCount()).isEqualTo(0L);
+ assertThat(numRecordsOut.getCount()).isEqualTo(0);
+ assertThat(numRecordsOutErrors.getCount()).isEqualTo(0);
+ assertThat(numRecordsSendErrors.getCount()).isEqualTo(0);
+
+ // but properly serialized elements should count just normally
Review Comment:
I considered that too, indeed that was my initial plan:
- Add another dummy serializer, e.g., `NullRecordSerializer`, which simply
returns `null` regardless of the input element
- Add a separate test, e.g., `testWriteNullProducerRecord`, along the lines
of your suggestion
In the end, I didn't do so because it seems simpler and at the same time
more realistic to me having the current test
(`testIncreasingRecordBasedCounters`) check that we get the counting right when
interleaving both valid and invalid elements (using the same
`DummyRecordSerializer`), as opposed to checking those cases in isolation.
Also, the new, separate, test would essentially be a clone of the existing one,
plus require yet another`createWriterWithConfiguration` method/override that
considers the new dummy serializer instead.
In summary, I think `testIncreasingRecordBasedCounters` is actually a good
spot for checking the new functionality. Having said that, we can of course
create a separate test if my arguments are not convincing enough. Whatever we
decide!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
