I was not careful enough when committing the previous patch. Now I look at it 
again, the previous test name 
`testLogsWithRetentionInprogressShouldNotPickedUpForCompactionAndViceVersa` 
seems to verbose. The phrase `IneligibleForCompaction` seems more accurate and 
more concise than the phrase `ShouldNotPickedUpForCompaction`. `AndViceVersa` 
seems redundant as we typically don't have to specify all details in the test 
name. Can we simplify that test name to e.g. 
`testLogsUndeCleanupIneligibleForCompaction`

Regarding the name for the tests added in this PR, 
`testLogsWithRetentionInprogressShouldBePickedUpForLogTruncation` does not seem 
to match what the test does. In particular, we are verifying that log cleanup 
and topic deletion can happen concurrently for the same partition, rather than 
verifying that the log which is under cleanup is also eligible for topic 
deletion, since we do not have explicitly logic to prevent topic deletion in 
this case. I feel the name `testConcurrentLogCleanupAndLogTruncation` probably 
matches the test's implementation better. What do you think?





[ Full content available at: https://github.com/apache/kafka/pull/5694 ]
This message was relayed via gitbox.apache.org for [email protected]

Reply via email to