You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "frankvicky (via GitHub)" <gi...@apache.org> on 2025/01/18 12:52:02 UTC
[PR] KAFKA-18527: SaslClientsWithInvalidCredentialsTest.java should run for async consumer [kafka]
frankvicky opened a new pull request, #18609:
URL: https://github.com/apache/kafka/pull/18609
JIRA: KAFKA-18527
Enable new consumer protocol for `SaslClientsWithInvalidCredentialsTest.java`
Since `SaslClientsWithInvalidCredentialsTest.scala` has some issues when enabling new consumer protocol, I decide to open a PR for java first.
### 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] KAFKA-18527: SaslClientsWithInvalidCredentialsTest.java should run for async consumer [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #18609:
URL: https://github.com/apache/kafka/pull/18609#discussion_r1931114177
##########
tools/src/test/java/org/apache/kafka/tools/consumer/group/SaslClientsWithInvalidCredentialsTest.java:
##########
@@ -149,7 +149,7 @@ public void testConsumerGroupServiceWithAuthenticationFailure(String quorum, Str
}
@ParameterizedTest(name = "{displayName}.quorum={0}.groupProtocol={1}")
- @MethodSource("getTestQuorumAndGroupProtocolParametersClassicGroupProtocolOnly")
+ @MethodSource("getTestQuorumAndGroupProtocolParametersAll")
public void testConsumerGroupServiceWithAuthenticationSuccess(String quorum, String groupProtocol) throws Exception {
createScramCredentialsViaPrivilegedAdminClient(JaasTestUtils.KAFKA_SCRAM_USER_2, JaasTestUtils.KAFKA_SCRAM_PASSWORD_2);
ConsumerGroupCommand.ConsumerGroupService consumerGroupService = prepareConsumerGroupService();
Review Comment:
ditto
##########
tools/src/test/java/org/apache/kafka/tools/consumer/group/SaslClientsWithInvalidCredentialsTest.java:
##########
@@ -136,7 +136,7 @@ public void tearDown() {
// NOTE: Not able to refer TestInfoUtils#TestWithParameterizedQuorumName() in the ParameterizedTest name.
@ParameterizedTest(name = "{displayName}.quorum={0}.groupProtocol={1}")
- @MethodSource("getTestQuorumAndGroupProtocolParametersClassicGroupProtocolOnly")
+ @MethodSource("getTestQuorumAndGroupProtocolParametersAll")
public void testConsumerGroupServiceWithAuthenticationFailure(String quorum, String groupProtocol) throws Exception {
ConsumerGroupCommand.ConsumerGroupService consumerGroupService = prepareConsumerGroupService();
Review Comment:
As we are modifying this PR, could you please close `consumerGroupService` via `try-release` also? For example:
```java
try (ConsumerGroupCommand.ConsumerGroupService consumerGroupService = prepareConsumerGroupService();
Consumer<byte[], byte[]> consumer = createConsumer()) {
consumer.subscribe(Collections.singletonList(TOPIC));
verifyAuthenticationException(consumerGroupService::listGroups);
}
```
--
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] KAFKA-18527: SaslClientsWithInvalidCredentialsTest.java should run for async consumer [kafka]
Posted by "frankvicky (via GitHub)" <gi...@apache.org>.
frankvicky commented on PR #18609:
URL: https://github.com/apache/kafka/pull/18609#issuecomment-2618318412
This issue will fixed by #18668
--
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] KAFKA-18527: SaslClientsWithInvalidCredentialsTest.java should run for async consumer [kafka]
Posted by "kirktrue (via GitHub)" <gi...@apache.org>.
kirktrue commented on code in PR #18609:
URL: https://github.com/apache/kafka/pull/18609#discussion_r1924367542
##########
tools/src/test/java/org/apache/kafka/tools/consumer/group/SaslClientsWithInvalidCredentialsTest.java:
##########
@@ -149,7 +149,7 @@ public void testConsumerGroupServiceWithAuthenticationFailure(String quorum, Str
}
@ParameterizedTest(name = "{displayName}.quorum={0}.groupProtocol={1}")
Review Comment:
Line 137 states that we can't use `TestInfoUtils.TestWithParameterizedQuorumName()` in the `@ParameterizedTest` annotation. Does it make sense to add that here, too?
--
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] KAFKA-18527: SaslClientsWithInvalidCredentialsTest.java should run for async consumer [kafka]
Posted by "frankvicky (via GitHub)" <gi...@apache.org>.
frankvicky closed pull request #18609: KAFKA-18527: SaslClientsWithInvalidCredentialsTest.java should run for async consumer
URL: https://github.com/apache/kafka/pull/18609
--
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] KAFKA-18527: SaslClientsWithInvalidCredentialsTest.java should run for async consumer [kafka]
Posted by "frankvicky (via GitHub)" <gi...@apache.org>.
frankvicky commented on code in PR #18609:
URL: https://github.com/apache/kafka/pull/18609#discussion_r1925314542
##########
tools/src/test/java/org/apache/kafka/tools/consumer/group/SaslClientsWithInvalidCredentialsTest.java:
##########
@@ -149,7 +149,7 @@ public void testConsumerGroupServiceWithAuthenticationFailure(String quorum, Str
}
@ParameterizedTest(name = "{displayName}.quorum={0}.groupProtocol={1}")
Review Comment:
Interesting, I'm not very sure about this one 🤔
--
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] KAFKA-18527: SaslClientsWithInvalidCredentialsTest.java should run for async consumer [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #18609:
URL: https://github.com/apache/kafka/pull/18609#discussion_r1927455995
##########
tools/src/test/java/org/apache/kafka/tools/consumer/group/SaslClientsWithInvalidCredentialsTest.java:
##########
@@ -149,7 +149,7 @@ public void testConsumerGroupServiceWithAuthenticationFailure(String quorum, Str
}
@ParameterizedTest(name = "{displayName}.quorum={0}.groupProtocol={1}")
Review Comment:
@frankvicky Could you please add the following comment to this test case?
```
// NOTE: Not able to refer TestInfoUtils#TestWithParameterizedQuorumName() in the ParameterizedTest name.
```
--
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] KAFKA-18527: SaslClientsWithInvalidCredentialsTest.java should run for async consumer [kafka]
Posted by "kirktrue (via GitHub)" <gi...@apache.org>.
kirktrue commented on PR #18609:
URL: https://github.com/apache/kafka/pull/18609#issuecomment-2616755422
@lianetm—would you review? Thanks!
--
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