cadonna commented on a change in pull request #10958:
URL: https://github.com/apache/kafka/pull/10958#discussion_r662865972
##########
File path:
streams/test-utils/src/test/java/org/apache/kafka/streams/TopologyTestDriverTest.java
##########
@@ -98,6 +98,8 @@
mkEntry(StreamsConfig.STATE_DIR_CONFIG,
TestUtils.tempDirectory().getAbsolutePath())
));
config.putAll(overrides);
+ config.putIfAbsent(StreamsConfig.DEFAULT_KEY_SERDE_CLASS_CONFIG,
Serdes.ByteArraySerde.class);
+ config.putIfAbsent(StreamsConfig.DEFAULT_VALUE_SERDE_CLASS_CONFIG,
Serdes.ByteArraySerde.class);
Review comment:
Could you put these config settings before the overrides on line 100?
Would be not so good to silently add the default serdes if the the overrides
possibly unset them. I would also just use `put()` instead of `putIfAbsent()`.
##########
File path:
streams/test-utils/src/main/java/org/apache/kafka/streams/TopologyTestDriver.java
##########
@@ -315,6 +316,9 @@ private TopologyTestDriver(final InternalTopologyBuilder
builder,
configCopy.putIfAbsent(StreamsConfig.BOOTSTRAP_SERVERS_CONFIG,
"dummy-bootstrap-host:0");
// provide randomized dummy app-id if it's not specified
configCopy.putIfAbsent(StreamsConfig.APPLICATION_ID_CONFIG,
"dummy-topology-test-driver-app-id-" + ThreadLocalRandom.current().nextInt());
+ // provide default serde since we change default serde to be null in
KIP-741
+ configCopy.putIfAbsent(StreamsConfig.DEFAULT_KEY_SERDE_CLASS_CONFIG,
Serdes.ByteArraySerde.class);
+ configCopy.putIfAbsent(StreamsConfig.DEFAULT_VALUE_SERDE_CLASS_CONFIG,
Serdes.ByteArraySerde.class);
Review comment:
Since the `TopologyTestDriver` should test a topology and somehow the
default serdes are part of the topology under test, the `TopologyTestDriver`
should fail, if no default serdes are provided by the users but needed. Hence,
I think, it is not a good idea to silently add the default serdes here.
That means, you have to provide deserializers and serializers in some tests
in `TopologyTestDriverTest`. For that look for methods called `setup*()` (e.g.
`setupSourceSinkTopology()`) and add the deserializers and serializers there.
In some tests, you need to add them directly in the test.
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
