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
`NullRecordSerializer`), as opposed to doing so in isolation. Also note that
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.
--
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]
