[GitHub] [kafka] vvcephei commented on a change in pull request #8764: KAFKA-10049: Fixed FKJ bug where wrapped serdes are set incorrectly when using default StreamsConfig serdes

[GitBox <gi...@apache.org>]

vvcephei commented on a change in pull request #8764:
URL: https://github.com/apache/kafka/pull/8764#discussion_r433284161



##########
File path: streams/src/test/java/org/apache/kafka/streams/integration/KTableKTableForeignKeyJoinMaterializationIntegrationTest.java
##########
@@ -77,11 +83,12 @@ public KTableKTableForeignKeyJoinMaterializationIntegrationTest(final boolean ma
     @Before
     public void before() {
         final String safeTestName = safeUniqueTestName(getClass(), testName);
-        streamsConfig = mkProperties(mkMap(
-            mkEntry(StreamsConfig.APPLICATION_ID_CONFIG, "app-" + safeTestName),
-            mkEntry(StreamsConfig.BOOTSTRAP_SERVERS_CONFIG, "asdf:0000"),
-            mkEntry(StreamsConfig.STATE_DIR_CONFIG, TestUtils.tempDirectory().getPath())
-        ));
+        streamsConfig = new Properties();
+        streamsConfig.put(StreamsConfig.APPLICATION_ID_CONFIG, "my-stream-processing-application-2");

Review comment:
       Please use the safeTestName

##########
File path: streams/src/test/java/org/apache/kafka/streams/integration/KTableKTableForeignKeyJoinMaterializationIntegrationTest.java
##########
@@ -94,15 +101,49 @@ public void before() {
         );
     }
 
+    @Test
+    public void shouldEmitRecordWhenJoiningWithDefaultSerdes() {

Review comment:
       Hey @bellemare , do we need to add a new test? It looks like this condition was intended to be covered already in KTableKTableForeignKeyJoinScenarioTest . Should we just fix that test instead?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org