[GitHub] [kafka] chia7712 commented on pull request #9906: KAFKA-10885 Refactor MemoryRecordsBuilderTest/MemoryRecordsTest to avoid a lot of…

[GitBox <gi...@apache.org>]

chia7712 commented on pull request #9906:
URL: https://github.com/apache/kafka/pull/9906#issuecomment-762364993


   @g1geordie Personally, current approach is a bit hard to read to me. It seems to me the "parameterized" arguments should be simple and readable. We should be able to understand what is injected by reading the "value" of ```ArgumentsSource```.
   
   For example, ```MemoryRecordsBuilderTest``` needs two kind of "parameters":
   1) exclude zstd
       - testWriteEmptyRecordSet
       - testCompressionRateV0
       - testCompressionRateV1
       - buildUsingLogAppendTime
       - buildUsingCreateTime
       - writePastLimit
       - convertToV1WithMixedV0AndV2Data
       - shouldThrowIllegalStateExceptionOnBuildWhenAborted
       - shouldResetBufferToInitialPositionOnAbort
       - shouldThrowIllegalStateExceptionOnCloseWhenAborted
       - shouldThrowIllegalStateExceptionOnAppendWhenAborted
   2) other test cases can run with all parameters
   
   Hence, we can add a new provider to offer parameters excluding zstd and then remove the ```assumeTrue``` from ```MemoryRecordsBuilderTest```. WDYT?
   


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