Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MINOR: Add missing unit tests for {Local|Remote}LeaderEndpoint classes #13272

Merged
merged 3 commits into from
Feb 22, 2023
Merged

MINOR: Add missing unit tests for {Local|Remote}LeaderEndpoint classes #13272

merged 3 commits into from
Feb 22, 2023

Conversation

kowshik
Copy link
Contributor

@kowshik kowshik commented Feb 17, 2023

I've added unit tests that were previously missing for the LeaderEndpoint.fetchEpochEndOffsets() public method.

@kowshik
Copy link
Contributor Author

kowshik commented Feb 17, 2023

Hi @junrao / @satishd / @mattwong949 -- Please could you help review this PR?

junrao
junrao reviewed Feb 17, 2023
View reviewed changes
Copy link
Contributor

@junrao junrao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kowshik : Thanks for the PR. Just one comment.

core/src/test/scala/kafka/server/LocalLeaderEndPointTest.scala Show resolved Hide resolved
@kowshik
Copy link
Contributor Author

kowshik commented Feb 18, 2023

@junrao Thanks for the review! I've addressed the comment in bc94d6d.

satishd
satishd approved these changes Feb 21, 2023
View reviewed changes
Copy link
Member

@satishd satishd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @kowshik for adding the missing test for fetching EpochEndOffsets API, LGTM.

junrao
junrao reviewed Feb 21, 2023
View reviewed changes
Copy link
Contributor

@junrao junrao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kowshik : Thanks for the updated PR. One more comment. Also, are the test failures related?

core/src/test/scala/kafka/server/LocalLeaderEndPointTest.scala Show resolved Hide resolved
@kowshik
Copy link
Contributor Author

kowshik commented Feb 21, 2023

@junrao Thanks for the review! I've addressed the comment, PR is ready for another pass. The CI test failures look unrelated to this PR.

junrao
junrao approved these changes Feb 22, 2023
View reviewed changes
Copy link
Contributor

@junrao junrao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kowshik : Thanks for the PR. LGTM

@junrao junrao merged commit db6beb9 into apache:trunk Feb 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@satishd satishd satishd approved these changes

@junrao junrao junrao approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kowshik @junrao @satishd