Copilot commented on code in PR #21031:
URL: https://github.com/apache/kafka/pull/21031#discussion_r2577806961
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java:
##########
@@ -2154,9 +2154,7 @@ private CoordinatorResult<StreamsGroupHeartbeatResult,
CoordinatorRecord> stream
)
));
- if (!returnedStatus.isEmpty()) {
- response.setStatus(returnedStatus);
- }
+ response.setStatus(returnedStatus);
Review Comment:
The code change to always set the `status` field will break many existing
tests that don't expect `.setStatus(List.of())` in responses when there are no
status messages.
Looking at the test file, there are numerous tests (e.g.,
`testMemberJoinsEmptyStreamsGroup` at line 16265,
`testStreamsGroupHeartbeatPartialResponseWhenNothingChanges` at line 17397,
`testStreamsUpdatingMemberMetadataTriggersNewTargetAssignment` at line 16986,
and many others) that create expected `StreamsGroupHeartbeatResponseData`
objects without calling `.setStatus()`, which means the field defaults to
`null`.
After this change, the actual responses will have `.setStatus(List.of())`
(an empty list), which won't equal `null`. This will cause test failures. All
affected tests need to be updated to include `.setStatus(List.of())` in their
expected response objects.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
