[GitHub] [kafka] jolshan commented on a diff in pull request #13769: MINOR: Covering all epoch cases in add partitions to txn manager

["jolshan (via GitHub)" <gi...@apache.org>]

jolshan commented on code in PR #13769:
URL: https://github.com/apache/kafka/pull/13769#discussion_r1213389003


##########
core/src/test/scala/unit/kafka/server/AddPartitionsToTxnManagerTest.scala:
##########
@@ -93,17 +93,22 @@ class AddPartitionsToTxnManagerTest {
 
     val transaction1AgainErrorsOldEpoch = mutable.Map[TopicPartition, Errors]()
     val transaction1AgainErrorsNewEpoch = mutable.Map[TopicPartition, Errors]()
+    val transaction1AgainErrorsOldEpochAgain = mutable.Map[TopicPartition, Errors]()
+
     // Trying to add more transactional data for the same transactional ID, producer ID, and epoch should simply replace the old data and send a retriable response.
     addPartitionsToTxnManager.addTxnData(node0, transactionData(transactionalId1, producerId1), setErrors(transaction1AgainErrorsOldEpoch))
     val expectedNetworkErrors = topicPartitions.map(_ -> Errors.NETWORK_EXCEPTION).toMap
     assertEquals(expectedNetworkErrors, transaction1Errors)

Review Comment:
   I never populate those fields. 
   We either populate the field via the callback when we return early due to the epoch/retry case or we return after receiving the response. Since this test is just checking about the addTxnData path, I didn't mock receiving a response. Because of that, the callback is never executed and the field is not populated.



-- 
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-unsubscribe@kafka.apache.org

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