[GitHub] [kafka] hachikuji commented on a change in pull request #8795: KAFKA-10095: Simplify calls in LogCleanerManagerTest

[GitBox]

hachikuji commented on a change in pull request #8795:
URL: https://github.com/apache/kafka/pull/8795#discussion_r436401217



##########
File path: core/src/test/scala/unit/kafka/log/LogCleanerManagerTest.scala
##########
@@ -571,12 +571,12 @@ class LogCleanerManagerTest extends Logging {
     cleanerManager.setCleaningState(topicPartition, LogCleaningInProgress)
     cleanerManager.doneCleaning(topicPartition, log.dir, 1)
     assertTrue(cleanerManager.cleaningState(topicPartition).isEmpty)
-    
assertTrue(cleanerManager.allCleanerCheckpoints.get(topicPartition).nonEmpty)
+    assertTrue(cleanerManager.allCleanerCheckpoints.contains(topicPartition))

Review comment:
       I think we could make the assertion a little stronger and fix this at 
the same time. We expect the checkpoint to be exactly the `endOffset` provided 
in `doneCleaning`. So maybe we could do something like this?
   ```scala
       val endOffset = 1L
       cleanerManager.doneCleaning(topicPartition, log.dir, endOffset)
       assertTrue(cleanerManager.cleaningState(topicPartition).isEmpty)
       assertEquals(Some(endOffset), 
cleanerManager.allCleanerCheckpoints.get(topicPartition))
   ```




----------------------------------------------------------------
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:
us...@infra.apache.org