jolshan commented on a change in pull request #11331:
URL: https://github.com/apache/kafka/pull/11331#discussion_r746217460
##########
File path: core/src/test/scala/unit/kafka/server/FetchSessionTest.scala
##########
@@ -967,6 +967,113 @@ class FetchSessionTest {
.setErrorCode(errorCode)
}
+ @Test
Review comment:
Is this replacing `testUpdatedPartitionResolvesId`? This is definitely
cleaner, but I'm not sure we are covering the same cases here. For context, the
test I mentioned before is testing different update scenarios (I probably named
it poorly). Mostly the idea is that the update method works correctly. (ie, we
update a partition that once had a topic ID to one that does not, etc). Maybe
that is covered in some of the other tests I've added (like `def
maybeUpdateRequestParamsOrName`) and we can just remove that test. What do you
think? I'll also think about this a bit more.
--
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
