adixitconfluent commented on PR #19823:
URL: https://github.com/apache/kafka/pull/19823#issuecomment-2913779258

   > The exception is because of the call `RecordBatch firstOverlapBatch = 
iterator.next();` in ShareFetchUtils when MemoryRecords is Empty. In the actual 
runs this scenario cannot happen because AcquiredRecords will be empty as well 
when the fetched records are empty. We should fix the test cases accordingly 
rather than just passing the any non-empty MemoryRecords. Isn't 
`createShareAcquiredRecords` returned data is incorrect?
   
   @apoorvmittal10 , the tests in `DelayedShareFetchTest` should only test 
whether the functionalities of `DelayedShareFetch` are working. Considering 
that, the changed functions only affect `DelayedShareFetchTest` tests. Now, all 
the functionalities that are utilized post `DelayedShareFetch` are mocked, 
example - acquire functionality for share partition. So, memory slicing mock 
data should not affect these tests since that functionality is not being 
utilized in `DelayedShareFetch` but in `ShareFetchUtils`. Whether 
`AcquiredRecords` are empty and we are passing non-empty Memory records does 
not affect testing of `DelayedShareFetch` functionalities, hence I feel simply 
changing to non-empty memory records is right.


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

Reply via email to