MINOR: Remove ConsumerTestBuilder.java#16691
Conversation
|
@chia7712 can you take a look at this PR? It is related to a recent PR that you had reviewed for me. |
|
Sure, but I am going to travel to CommunityOverCode Asia 2024. Will take a look next Monday if this great patch is not merged 😄 |
|
Thank you! That will be no problem if it has to wait until Monday, I appreciate your help! |
| private static final int DEFAULT_HEARTBEAT_INTERVAL_MS = 1000; | ||
| private static final int DEFAULT_MAX_POLL_INTERVAL_MS = 10000; | ||
| private static final long DEFAULT_RETRY_BACKOFF_MAX_MS = 1000; | ||
| private static final long DEFAULT_RETRY_BACKOFF_MS = 80; | ||
| private static final String DEFAULT_GROUP_ID = "group-id"; |
There was a problem hiding this comment.
For the share groups case, there is a ShareConsumerTestBuilder that already contains all these. I think we should use it here
There was a problem hiding this comment.
Really the pattern established for the consumer tests ought to be adopted for share groups tests too. We are just starting to align them across the board.
There was a problem hiding this comment.
sure, that makes total sense. My point is simply to keep the share tests using the ShareConsumerTestBuilder consistently, and not start partially migrating them as part of this PR.
There was a problem hiding this comment.
I updated the test to use ShareConsumerTestBuilder instead of ConsumerTestBuilder now. Let me know if that works, all tests still pass.
|
@brenden20 Could you please fix the import order? |
|
LGTM again. I will merge it tomorrow if there is no objection :) |
|
@chia7712 sounds good, thank you! |
….7.x (apache#16691) Bump commonsIo to version 2.16.0 on branch 7.7.x
4 participants
The purpose of this PR is to remove ConsumerTestBuilder.java since it is no longer needed. The following PRs have eliminated the use of ConsumerTestBuilder:
#14930
#16140
#16200
#16312
Committer Checklist (excluded from commit message)