chia7712 commented on code in PR #16253:
URL: https://github.com/apache/kafka/pull/16253#discussion_r1634010611
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/distributed/IncrementalCooperativeAssignorTest.java:
##########
@@ -1508,25 +1545,25 @@ private List<Integer>
allocations(Function<ConnectorsAndTasks, ? extends Collect
private void assertNoRevocations() {
returnedAssignments.newlyRevokedConnectors().forEach((worker,
revocations) ->
assertEquals(
- "Expected no revocations to take place during this
round, but connector revocations were issued for worker " + worker,
Collections.emptySet(),
- new HashSet<>(revocations)
+ new HashSet<>(revocations),
+ "Expected no revocations to take place during this
round, but connector revocations were issued for worker " + worker
)
);
returnedAssignments.newlyRevokedTasks().forEach((worker, revocations)
->
assertEquals(
- "Expected no revocations to take place during this
round, but task revocations were issued for worker " + worker,
Collections.emptySet(),
- new HashSet<>(revocations)
Review Comment:
> With rudimentary checking logic (searching for
assertEquals(Collections.empty in the code base), I also see this pattern used
94 times across 16 files in the Connect modules, so it's not exactly uncommon.
IMO it'd be better to make it more common in other parts of the code base
rather than try to "simplify" things in a way that drops information on
assertion failures.
I file https://issues.apache.org/jira/browse/KAFKA-16929 to have more
discussion
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
