JingGe commented on a change in pull request #18397:
URL: https://github.com/apache/flink/pull/18397#discussion_r790600948
##########
File path:
flink-connectors/flink-connector-kafka/src/test/java/org/apache/flink/connector/kafka/sink/KafkaRecordSerializationSchemaBuilderTest.java
##########
@@ -53,11 +54,15 @@
private static final String DEFAULT_TOPIC = "test";
+ private static Map<String, ?> configurableConfiguration;
Review comment:
I understood your concern. I've considered this option and chose the
current solution with the following reasons:
- the business cases `ConfigurableStringSerializer` and
`SimpleStringSerializer` simulated are isolated, i.e. there is no way to used
them simultaneously. The test env should stick to the real business case as
more as possible, therefore shared map should not be used to mix them up.
- test code will be maintained for long time and by many different
developers. It has higher priority to make the test code straightforward, i.e.
KISS and being able to avoid/reduce human mistakes as far as possible. By
default, the cost of using a new HashMap is about 144 bytes (could be reduced
to be less), i.e. 16 empty buckets when the hashMap has been created plus
loadFactor, modCount, threshold, size, and header. The benefit is a much clean
and safe way to write the test. The benefit is large enough with the minimal
cost. I think the ROI is great.
--
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]
