KAFKA-15123: Add tests for ChunkedBytesStream #13941
Conversation
clients/src/test/java/org/apache/kafka/common/utils/ChunkedBytesStreamTest.java
Outdated
Show resolved
Hide resolved
clients/src/test/java/org/apache/kafka/common/utils/ChunkedBytesStreamTest.java
Outdated
Show resolved
Hide resolved
| inputBuf.position(inputBuf.capacity()); | ||
| inputBuf.flip(); | ||
|
|
||
| try (InputStream is = new ChunkedBytesStream(new ByteBufferInputStream(inputBuf.duplicate()), supplier, 10, true)) { |
There was a problem hiding this comment.
we want to test this for both cases when skip is pushed down vs not, i.e. last argument is true and when last argument is false.
There was a problem hiding this comment.
you can use @ParameterizedTest for true/false here instead of duplicating code.
clients/src/test/java/org/apache/kafka/common/utils/ChunkedBytesStreamTest.java
Outdated
Show resolved
Hide resolved
|
@divijvaidya Please have a look at my changes resolving your comments :) Also, there are test failures on the CI and also some test failures in my local environemnt. However, I can absolutely not see any connection to my changes. What can I do to resolve this? Do I have to rebase to upstream/trunk and force-push before this can be merged? |
|
Hey @riedelmax |
|
Ups.. overlooked that. Thanks for pointing it out. I will fix it |
|
CI tests have ChunkedBytesStream test as successful - https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-13941/3/testReport/org.apache.kafka.common.utils/ChunkedBytesStreamTest/ Merging this one. |
There was a problem hiding this comment.
Thank you and congratulations for your first contribution to the Apache Kafka @riedelmax ! Looking forward to more contributions from you.
|
Hey @divijvaidya |
Reviewers: Divij Vaidya <diviv@amazon.com>
Committer Checklist (excluded from commit message)