You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "Parkerhiphop (via GitHub)" <gi...@apache.org> on 2025/02/06 16:07:54 UTC

[PR] MINOR: Align assertFutureThrows method signature with JUnit conventions [kafka]

Parkerhiphop opened a new pull request, #18825:
URL: https://github.com/apache/kafka/pull/18825

   Align assertFutureThrows method signature with JUnit conventions based on the suggestion from https://github.com/apache/kafka/pull/18296#discussion_r1900565069.
   
   From `assertFutureThrows(Future<?> future, Class expectedCauseClassApiException)` to `assertFutureThrows(Class expectedCauseClassApiException, Future<?> future)`
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] MINOR: Align assertFutureThrows method signature with JUnit conventions [kafka]

Posted by "Parkerhiphop (via GitHub)" <gi...@apache.org>.
Parkerhiphop commented on PR #18825:
URL: https://github.com/apache/kafka/pull/18825#issuecomment-2660723112

   > @Parkerhiphop Please can you investigate why the tests timed out.
   
   @AndrewJSchofield Thanks for reminding. I resolve it by merging the latest changes .


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] MINOR: Align assertFutureThrows method signature with JUnit conventions [kafka]

Posted by "Parkerhiphop (via GitHub)" <gi...@apache.org>.
Parkerhiphop commented on code in PR #18825:
URL: https://github.com/apache/kafka/pull/18825#discussion_r1954105679


##########
clients/src/test/java/org/apache/kafka/test/TestUtils.java:
##########
@@ -583,12 +583,12 @@ public static Set<TopicPartition> generateRandomTopicPartitions(int numTopic, in
      * Assert that a future raises an expected exception cause type. Return the exception cause
      * if the assertion succeeds; otherwise raise AssertionError.
      *
-     * @param future The future to await
-     * @param exceptionCauseClass Class of the expected exception cause
      * @param <T> Exception cause type parameter
+     * @param exceptionCauseClass Class of the expected exception cause
+     * @param future The future to await
      * @return The caught exception cause
      */
-    public static <T extends Throwable> T assertFutureThrows(Future<?> future, Class<T> exceptionCauseClass) {
+    public static <T extends Throwable> T assertFutureThrows(Class<T> exceptionCauseClass, Future<?> future) {
         ExecutionException exception = assertThrows(ExecutionException.class, future::get);
         Throwable cause = exception.getCause();
         assertEquals(exceptionCauseClass, cause.getClass(),

Review Comment:
   @AndrewJSchofield I just found [this discussion](https://github.com/apache/kafka/pull/18296#discussion_r1899687406) suggesting the adoption of `assertEquals` instead of `assertInstanceOf`. 
   
   Should I revert to assertEquals here?



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] MINOR: Align assertFutureThrows method signature with JUnit conventions [kafka]

Posted by "Parkerhiphop (via GitHub)" <gi...@apache.org>.
Parkerhiphop commented on code in PR #18825:
URL: https://github.com/apache/kafka/pull/18825#discussion_r1954517279


##########
clients/src/test/java/org/apache/kafka/test/TestUtils.java:
##########
@@ -583,12 +583,12 @@ public static Set<TopicPartition> generateRandomTopicPartitions(int numTopic, in
      * Assert that a future raises an expected exception cause type. Return the exception cause
      * if the assertion succeeds; otherwise raise AssertionError.
      *
-     * @param future The future to await
-     * @param exceptionCauseClass Class of the expected exception cause
      * @param <T> Exception cause type parameter
+     * @param exceptionCauseClass Class of the expected exception cause
+     * @param future The future to await
      * @return The caught exception cause
      */
-    public static <T extends Throwable> T assertFutureThrows(Future<?> future, Class<T> exceptionCauseClass) {
+    public static <T extends Throwable> T assertFutureThrows(Class<T> exceptionCauseClass, Future<?> future) {
         ExecutionException exception = assertThrows(ExecutionException.class, future::get);
         Throwable cause = exception.getCause();
         assertEquals(exceptionCauseClass, cause.getClass(),

Review Comment:
   Thanks @chia7712 for the advice.
   I’ve reverted the assertEquals changes and added a comment above it.



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] MINOR: Align assertFutureThrows method signature with JUnit conventions [kafka]

Posted by "Parkerhiphop (via GitHub)" <gi...@apache.org>.
Parkerhiphop commented on PR #18825:
URL: https://github.com/apache/kafka/pull/18825#issuecomment-2665195936

   @AndrewJSchofield The suggestion is great!
   Using the direct reference to the exception (XxxXXxException.class) makes the code more concise and readable.
   Since I’ve already modified these lines to change the signature, I think this is a good time to make this update as well.
   I’ve updated the code accordingly. Thanks for reviewing.


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] MINOR: Align assertFutureThrows method signature with JUnit conventions [kafka]

Posted by "Parkerhiphop (via GitHub)" <gi...@apache.org>.
Parkerhiphop commented on code in PR #18825:
URL: https://github.com/apache/kafka/pull/18825#discussion_r1954073277


##########
clients/src/test/java/org/apache/kafka/test/TestUtils.java:
##########
@@ -583,12 +583,12 @@ public static Set<TopicPartition> generateRandomTopicPartitions(int numTopic, in
      * Assert that a future raises an expected exception cause type. Return the exception cause
      * if the assertion succeeds; otherwise raise AssertionError.
      *
-     * @param future The future to await
-     * @param exceptionCauseClass Class of the expected exception cause
      * @param <T> Exception cause type parameter
+     * @param exceptionCauseClass Class of the expected exception cause
+     * @param future The future to await
      * @return The caught exception cause
      */
-    public static <T extends Throwable> T assertFutureThrows(Future<?> future, Class<T> exceptionCauseClass) {
+    public static <T extends Throwable> T assertFutureThrows(Class<T> exceptionCauseClass, Future<?> future) {
         ExecutionException exception = assertThrows(ExecutionException.class, future::get);
         Throwable cause = exception.getCause();
         assertEquals(exceptionCauseClass, cause.getClass(),

Review Comment:
   Thanks for reviewing.
   I've changed it to `assertInstanceOf` and merged the latest changes to resolve conflicts.



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] MINOR: Align assertFutureThrows method signature with JUnit conventions [kafka]

Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield commented on PR #18825:
URL: https://github.com/apache/kafka/pull/18825#issuecomment-2660200395

   @Parkerhiphop Please can you investigate why the tests timed out.


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] MINOR: Align assertFutureThrows method signature with JUnit conventions [kafka]

Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield commented on code in PR #18825:
URL: https://github.com/apache/kafka/pull/18825#discussion_r1954247645


##########
clients/src/test/java/org/apache/kafka/test/TestUtils.java:
##########
@@ -583,12 +583,12 @@ public static Set<TopicPartition> generateRandomTopicPartitions(int numTopic, in
      * Assert that a future raises an expected exception cause type. Return the exception cause
      * if the assertion succeeds; otherwise raise AssertionError.
      *
-     * @param future The future to await
-     * @param exceptionCauseClass Class of the expected exception cause
      * @param <T> Exception cause type parameter
+     * @param exceptionCauseClass Class of the expected exception cause
+     * @param future The future to await
      * @return The caught exception cause
      */
-    public static <T extends Throwable> T assertFutureThrows(Future<?> future, Class<T> exceptionCauseClass) {
+    public static <T extends Throwable> T assertFutureThrows(Class<T> exceptionCauseClass, Future<?> future) {
         ExecutionException exception = assertThrows(ExecutionException.class, future::get);
         Throwable cause = exception.getCause();
         assertEquals(exceptionCauseClass, cause.getClass(),

Review Comment:
   Makes sense to me.



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] MINOR: Align assertFutureThrows method signature with JUnit conventions [kafka]

Posted by "Parkerhiphop (via GitHub)" <gi...@apache.org>.
Parkerhiphop commented on code in PR #18825:
URL: https://github.com/apache/kafka/pull/18825#discussion_r1954105679


##########
clients/src/test/java/org/apache/kafka/test/TestUtils.java:
##########
@@ -583,12 +583,12 @@ public static Set<TopicPartition> generateRandomTopicPartitions(int numTopic, in
      * Assert that a future raises an expected exception cause type. Return the exception cause
      * if the assertion succeeds; otherwise raise AssertionError.
      *
-     * @param future The future to await
-     * @param exceptionCauseClass Class of the expected exception cause
      * @param <T> Exception cause type parameter
+     * @param exceptionCauseClass Class of the expected exception cause
+     * @param future The future to await
      * @return The caught exception cause
      */
-    public static <T extends Throwable> T assertFutureThrows(Future<?> future, Class<T> exceptionCauseClass) {
+    public static <T extends Throwable> T assertFutureThrows(Class<T> exceptionCauseClass, Future<?> future) {
         ExecutionException exception = assertThrows(ExecutionException.class, future::get);
         Throwable cause = exception.getCause();
         assertEquals(exceptionCauseClass, cause.getClass(),

Review Comment:
   @AndrewJSchofield I just found [this discussion](https://github.com/apache/kafka/pull/18296#discussion_r1899687406) suggesting the adoption of assertEquals instead of assertInstanceOf. 
   
   Should I revert to assertEquals here?



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] MINOR: Align assertFutureThrows method signature with JUnit conventions [kafka]

Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield commented on code in PR #18825:
URL: https://github.com/apache/kafka/pull/18825#discussion_r1952946178


##########
clients/src/test/java/org/apache/kafka/test/TestUtils.java:
##########
@@ -583,12 +583,12 @@ public static Set<TopicPartition> generateRandomTopicPartitions(int numTopic, in
      * Assert that a future raises an expected exception cause type. Return the exception cause
      * if the assertion succeeds; otherwise raise AssertionError.
      *
-     * @param future The future to await
-     * @param exceptionCauseClass Class of the expected exception cause
      * @param <T> Exception cause type parameter
+     * @param exceptionCauseClass Class of the expected exception cause
+     * @param future The future to await
      * @return The caught exception cause
      */
-    public static <T extends Throwable> T assertFutureThrows(Future<?> future, Class<T> exceptionCauseClass) {
+    public static <T extends Throwable> T assertFutureThrows(Class<T> exceptionCauseClass, Future<?> future) {
         ExecutionException exception = assertThrows(ExecutionException.class, future::get);
         Throwable cause = exception.getCause();
         assertEquals(exceptionCauseClass, cause.getClass(),

Review Comment:
   This could probably be `assertInstanceOf` since we're cleaning up the use of assertions here.



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] MINOR: Align assertFutureThrows method signature with JUnit conventions [kafka]

Posted by "Parkerhiphop (via GitHub)" <gi...@apache.org>.
Parkerhiphop commented on code in PR #18825:
URL: https://github.com/apache/kafka/pull/18825#discussion_r1954105679


##########
clients/src/test/java/org/apache/kafka/test/TestUtils.java:
##########
@@ -583,12 +583,12 @@ public static Set<TopicPartition> generateRandomTopicPartitions(int numTopic, in
      * Assert that a future raises an expected exception cause type. Return the exception cause
      * if the assertion succeeds; otherwise raise AssertionError.
      *
-     * @param future The future to await
-     * @param exceptionCauseClass Class of the expected exception cause
      * @param <T> Exception cause type parameter
+     * @param exceptionCauseClass Class of the expected exception cause
+     * @param future The future to await
      * @return The caught exception cause
      */
-    public static <T extends Throwable> T assertFutureThrows(Future<?> future, Class<T> exceptionCauseClass) {
+    public static <T extends Throwable> T assertFutureThrows(Class<T> exceptionCauseClass, Future<?> future) {
         ExecutionException exception = assertThrows(ExecutionException.class, future::get);
         Throwable cause = exception.getCause();
         assertEquals(exceptionCauseClass, cause.getClass(),

Review Comment:
   @AndrewJSchofield I just found [this discussion](https://github.com/apache/kafka/pull/18296#discussion_r1899687406) suggesting the adoption of `assertEquals` instead of `assertInstanceOf`. 
   
   Is this still valid in the context of the cleanup process?



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] MINOR: Align assertFutureThrows method signature with JUnit conventions [kafka]

Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #18825:
URL: https://github.com/apache/kafka/pull/18825#discussion_r1954133711


##########
clients/src/test/java/org/apache/kafka/test/TestUtils.java:
##########
@@ -583,12 +583,12 @@ public static Set<TopicPartition> generateRandomTopicPartitions(int numTopic, in
      * Assert that a future raises an expected exception cause type. Return the exception cause
      * if the assertion succeeds; otherwise raise AssertionError.
      *
-     * @param future The future to await
-     * @param exceptionCauseClass Class of the expected exception cause
      * @param <T> Exception cause type parameter
+     * @param exceptionCauseClass Class of the expected exception cause
+     * @param future The future to await
      * @return The caught exception cause
      */
-    public static <T extends Throwable> T assertFutureThrows(Future<?> future, Class<T> exceptionCauseClass) {
+    public static <T extends Throwable> T assertFutureThrows(Class<T> exceptionCauseClass, Future<?> future) {
         ExecutionException exception = assertThrows(ExecutionException.class, future::get);
         Throwable cause = exception.getCause();
         assertEquals(exceptionCauseClass, cause.getClass(),

Review Comment:
   Yes, in the previous PR we preferred to do strict check for the type of exception. Maybe we can keep the origin behavior but this PR can add comments to avoid potential confusion in the future.



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] MINOR: Align assertFutureThrows method signature with JUnit conventions [kafka]

Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield commented on code in PR #18825:
URL: https://github.com/apache/kafka/pull/18825#discussion_r1957107949


##########
clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java:
##########
@@ -1419,10 +1419,10 @@ public void testDeleteTopicsDontRetryThrottlingExceptionWhenDisabled() throws Ex
                 new DeleteTopicsOptions().retryOnQuotaViolation(false));
 
             assertNull(result.topicNameValues().get("topic1").get());
-            ThrottlingQuotaExceededException e = TestUtils.assertFutureThrows(result.topicNameValues().get("topic2"),
-                ThrottlingQuotaExceededException.class);
+            ThrottlingQuotaExceededException e = TestUtils.assertFutureThrows(ThrottlingQuotaExceededException.class, result.topicNameValues().get("topic2")
+            );

Review Comment:
   nit: Orphaned `)` on its own line.



##########
clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java:
##########
@@ -2653,10 +2653,10 @@ public void testCreatePartitionsDontRetryThrottlingExceptionWhenDisabled() throw
                 counts, new CreatePartitionsOptions().retryOnQuotaViolation(false));
 
             assertNull(result.values().get("topic1").get());
-            ThrottlingQuotaExceededException e = TestUtils.assertFutureThrows(result.values().get("topic2"),
-                ThrottlingQuotaExceededException.class);
+            ThrottlingQuotaExceededException e = TestUtils.assertFutureThrows(ThrottlingQuotaExceededException.class, result.values().get("topic2")
+            );

Review Comment:
   nit: Orphaned `)` on its own line.



##########
clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java:
##########
@@ -1152,10 +1152,10 @@ public void testCreateTopicsDontRetryThrottlingExceptionWhenDisabled() throws Ex
                 new CreateTopicsOptions().retryOnQuotaViolation(false));
 
             assertNull(result.values().get("topic1").get());
-            ThrottlingQuotaExceededException e = TestUtils.assertFutureThrows(result.values().get("topic2"),
-                ThrottlingQuotaExceededException.class);
+            ThrottlingQuotaExceededException e = TestUtils.assertFutureThrows(ThrottlingQuotaExceededException.class, result.values().get("topic2")
+            );

Review Comment:
   nit: Orphaned `)` on its own line.



##########
clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java:
##########
@@ -1493,7 +1493,7 @@ public void testInvalidTopicNames() throws Exception {
 
             Map<String, KafkaFuture<Void>> createFutures = env.adminClient().createTopics(newTopics).values();
             for (String sillyTopicName : sillyTopicNames) {
-                TestUtils.assertFutureThrows(createFutures .get(sillyTopicName), InvalidTopicException.class);
+                TestUtils.assertFutureThrows(InvalidTopicException.class, createFutures .get(sillyTopicName));

Review Comment:
   nit: Unnecessary space in `createFutures .get`.



##########
clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java:
##########
@@ -1440,10 +1440,10 @@ public void testDeleteTopicsDontRetryThrottlingExceptionWhenDisabled() throws Ex
                     new DeleteTopicsOptions().retryOnQuotaViolation(false));
 
             assertNull(resultIds.topicIdValues().get(topicId1).get());
-            e = TestUtils.assertFutureThrows(resultIds.topicIdValues().get(topicId2),
-                    ThrottlingQuotaExceededException.class);
+            e = TestUtils.assertFutureThrows(ThrottlingQuotaExceededException.class, resultIds.topicIdValues().get(topicId2)
+            );

Review Comment:
   nit: Orphaned `)` on its own line.



##########
clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java:
##########
@@ -2625,10 +2625,10 @@ public void testCreatePartitionsRetryThrottlingExceptionWhenEnabledUntilRequestT
             time.sleep(defaultApiTimeout + 1);
 
             assertNull(result.values().get("topic1").get());
-            ThrottlingQuotaExceededException e = TestUtils.assertFutureThrows(result.values().get("topic2"),
-                ThrottlingQuotaExceededException.class);
+            ThrottlingQuotaExceededException e = TestUtils.assertFutureThrows(ThrottlingQuotaExceededException.class, result.values().get("topic2")
+            );

Review Comment:
   nit: Orphaned `)` on its own line.



##########
clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java:
##########
@@ -1395,10 +1395,10 @@ public void testDeleteTopicsRetryThrottlingExceptionWhenEnabledUntilRequestTimeO
             time.sleep(defaultApiTimeout + 1);
 
             assertNull(resultIds.topicIdValues().get(topicId1).get());
-            e = TestUtils.assertFutureThrows(resultIds.topicIdValues().get(topicId2),
-                    ThrottlingQuotaExceededException.class);
+            e = TestUtils.assertFutureThrows(ThrottlingQuotaExceededException.class, resultIds.topicIdValues().get(topicId2)
+            );

Review Comment:
   nit: Orphaned `)` on its own line.



##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -2225,14 +2227,14 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest {
         val expectedOperations = AclEntry.supportedOperations(ResourceType.GROUP)
         assertEquals(expectedOperations, testGroupDescription.authorizedOperations())
 
-        // Test that the fake group throws GroupIdNotFoundException
-        assertTrue(describeWithFakeGroupResult.describedGroups().containsKey(fakeGroupId))
-        assertFutureThrows(describeWithFakeGroupResult.describedGroups().get(fakeGroupId),
-          classOf[GroupIdNotFoundException], s"Group $fakeGroupId not found.")
+          // Test that the fake group throws GroupIdNotFoundException

Review Comment:
   The indentation is changed in several changes in this file.



##########
clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java:
##########
@@ -1358,10 +1358,10 @@ public void testDeleteTopicsRetryThrottlingExceptionWhenEnabledUntilRequestTimeO
             time.sleep(defaultApiTimeout + 1);
 
             assertNull(result.topicNameValues().get("topic1").get());
-            ThrottlingQuotaExceededException e = TestUtils.assertFutureThrows(result.topicNameValues().get("topic2"),
-                ThrottlingQuotaExceededException.class);
+            ThrottlingQuotaExceededException e = TestUtils.assertFutureThrows(ThrottlingQuotaExceededException.class, result.topicNameValues().get("topic2")
+            );

Review Comment:
   nit: Orphaned `)` on its own line.



##########
clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java:
##########
@@ -1125,10 +1125,10 @@ public void testCreateTopicsRetryThrottlingExceptionWhenEnabledUntilRequestTimeO
             time.sleep(defaultApiTimeout + 1);
 
             assertNull(result.values().get("topic1").get());
-            ThrottlingQuotaExceededException e = TestUtils.assertFutureThrows(result.values().get("topic2"),
-                ThrottlingQuotaExceededException.class);
+            ThrottlingQuotaExceededException e = TestUtils.assertFutureThrows(ThrottlingQuotaExceededException.class, result.values().get("topic2")
+            );

Review Comment:
   nit: Orphaned `)` on its own line.



##########
clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java:
##########
@@ -1888,11 +1888,11 @@ public void testDescribeAcls() throws Exception {
             env.kafkaClient().prepareResponse(new DescribeAclsResponse(new DescribeAclsResponseData()
                 .setErrorCode(Errors.SECURITY_DISABLED.code())
                 .setErrorMessage("Security is disabled"), ApiKeys.DESCRIBE_ACLS.latestVersion()));
-            TestUtils.assertFutureThrows(env.adminClient().describeAcls(FILTER2).values(), SecurityDisabledException.class);
+            TestUtils.assertFutureThrows(SecurityDisabledException.class, env.adminClient().describeAcls(FILTER2).values());
 
             // Test a call where we supply an invalid filter.
-            TestUtils.assertFutureThrows(env.adminClient().describeAcls(UNKNOWN_FILTER).values(),
-                InvalidRequestException.class);
+            TestUtils.assertFutureThrows(InvalidRequestException.class, env.adminClient().describeAcls(UNKNOWN_FILTER).values()
+            );

Review Comment:
   nit: Orphaned `)` on its own line.



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] MINOR: Align assertFutureThrows method signature with JUnit conventions [kafka]

Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield merged PR #18825:
URL: https://github.com/apache/kafka/pull/18825


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org