Skip to content

GH-4008: Improve ReplyingKafkaTemplateTests with modular validation #4031

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
Merged
merged 1 commit into from
Aug 1, 2025
Merged

GH-4008: Improve ReplyingKafkaTemplateTests with modular validation #4031

merged 1 commit into from
Aug 1, 2025

Conversation

@snowykte0426
Copy link
Contributor

@snowykte0426 snowykte0426 commented Jul 30, 2025

Summary

This PR addresses issue #4008 by refactoring ReplyingKafkaTemplateTests
to improve test maintainability and readability through better
separation of concerns.

  • Refactored listener methods to return Message<?> objects instead of
    handling validation internally
  • Extracted header validation logic into separate utility methods
    (validateHeaders, validateSuccessfulResponse)
  • Simplified test methods by moving validation concerns outside of
    listener logic

Changes Made

  • Listener Methods: Simplified
    handleCustomReplyHeaderNoReplyPartition and
    handleCustomReplyHeaderDefaultPartitionHeader to focus only on business
    logic
  • Validation Utilities: Added validateHeaders() and
    validateSuccessfulResponse() methods for external header verification
  • Test Methods: Updated testCustomReplyTopicHeaderIsNotDuplicated
    and testCustomReplyHeadersAreNotDuplicated to use the new validation
    approach

Fixes #4008

@snowykte0426 snowykte0426 mentioned this pull request Jul 30, 2025
Closed
artembilan
artembilan requested changes Jul 30, 2025
View reviewed changes
Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

Thank you for refactoring this hard-to-read logic!

artembilan
artembilan requested changes Jul 30, 2025
View reviewed changes
...g-kafka/src/test/java/org/springframework/kafka/requestreply/ReplyingKafkaTemplateTests.java Outdated

assertThat(resultingMessage.getPayload()).isEqualTo("OK");
assertThat(resultingMessage.getHeaders()).containsKey("originalPayload");
assertThat(resultingMessage.getHeaders().get("originalPayload")).isEqualTo("expected_message");
Copy link
Member

@artembilan artembilan Jul 30, 2025

Choose a reason for hiding this comment

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

There is an API in AssertJ to do both at the same time:

assertThat(resultingMessage.getHeaders()).containsEntry("originalPayload", "expected_message");

Copy link
Contributor Author

@snowykte0426 snowykte0426 Jul 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

There is an API in AssertJ to do both at the same time:

assertThat(resultingMessage.getHeaders()).containsEntry("originalPayload", "expected_message");

@artembilan Thank you for the suggestion! I’ve implemented your feedback in commit e8baed9 by using AssertJ’s containsEntry() method. This replaces the separate assertions with the more concise approach you recommended.

artembilan
artembilan requested changes Jul 31, 2025
View reviewed changes
Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

Thank you for the update!
It is really very close to merge.
However I’ve noticed the problem:

Signed-off-by: snowykte0426 snowykte0426@naver.com

this one has to come with your official name to satisfy all the legal requirements.
Please, revise your local Git client config.
Squash and push a commit with with right sign-off.

@snowykte0426
GH-4008: Improve ReplyingKafkaTemplateTests with modular validation
b965e86 
Improve test validation logic by:
- Using direct assertThat() assertions instead of custom validation methods
- Replacing separate header assertions with single containsEntry() method
- Fixing checkstyle violations (trailing whitespace and extra blank lines)

Signed-off-by: Kim Tae Eun <snowykte0426@naver.com>
@snowykte0426
Copy link
Contributor Author

snowykte0426 commented Jul 31, 2025

Thank you for the update! It is really very close to merge. However I’ve noticed the problem:

Signed-off-by: snowykte0426 snowykte0426@naver.com

this one has to come with your official name to satisfy all the legal requirements. Please, revise your local Git client config. Squash and push a commit with with right sign-off.

Thank you for the review feedback. I've squashed the commits and updated
the Signed-off-by with my official name as requested (commit b965e86).

@artembilan artembilan merged commit 952f231 into spring-projects:main Aug 1, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@artembilan artembilan artembilan requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Test improvement in ReplyingKafkaTemplateTests

2 participants

@snowykte0426 @artembilan