You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/07/08 11:23:46 UTC
[GitHub] [kafka] akatona84 opened a new pull request #8992: MINOR: Closing resources in SaslClientsWithInvalidCredentialsTest
akatona84 opened a new pull request #8992:
URL: https://github.com/apache/kafka/pull/8992
*More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.*
*Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.*
### 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] omkreddy merged pull request #8992: MINOR: Closing resources in SaslClientsWithInvalidCredentialsTest
Posted by GitBox <gi...@apache.org>.
omkreddy merged pull request #8992:
URL: https://github.com/apache/kafka/pull/8992
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] omkreddy commented on pull request #8992: MINOR: Closing resources in SaslClientsWithInvalidCredentialsTest
Posted by GitBox <gi...@apache.org>.
omkreddy commented on pull request #8992:
URL: https://github.com/apache/kafka/pull/8992#issuecomment-656045969
ok to test
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] akatona84 edited a comment on pull request #8992: MINOR: Closing resources in SaslClientsWithInvalidCredentialsTest
Posted by GitBox <gi...@apache.org>.
akatona84 edited a comment on pull request #8992:
URL: https://github.com/apache/kafka/pull/8992#issuecomment-656038173
Thanks, @omkreddy for explaining how the consumers and prods are closed (I've missed that), updated my PR so only closing things where it is necessary.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] akatona84 edited a comment on pull request #8992: MINOR: Closing resources in SaslClientsWithInvalidCredentialsTest
Posted by GitBox <gi...@apache.org>.
akatona84 edited a comment on pull request #8992:
URL: https://github.com/apache/kafka/pull/8992#issuecomment-655458870
Hi @ijuma , I've encountered a test failure here which caused unexpected threads in the following tests `@Before` phase.
This fixes the issue in case failure happens, only that would be reported (and not together with .classMethod ending failures)
Could you take a look on this please?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] akatona84 commented on pull request #8992: MINOR: Closing resources in SaslClientsWithInvalidCredentialsTest
Posted by GitBox <gi...@apache.org>.
akatona84 commented on pull request #8992:
URL: https://github.com/apache/kafka/pull/8992#issuecomment-655925569
@gcsaba2, here's my PR regarding the test fix
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] akatona84 edited a comment on pull request #8992: MINOR: Closing resources in SaslClientsWithInvalidCredentialsTest
Posted by GitBox <gi...@apache.org>.
akatona84 edited a comment on pull request #8992:
URL: https://github.com/apache/kafka/pull/8992#issuecomment-655925569
@gcsaba2, here's my PR regarding the test fix, thx for the tip for quietly closing consumers
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] akatona84 commented on a change in pull request #8992: MINOR: Closing resources in SaslClientsWithInvalidCredentialsTest
Posted by GitBox <gi...@apache.org>.
akatona84 commented on a change in pull request #8992:
URL: https://github.com/apache/kafka/pull/8992#discussion_r452108786
##########
File path: core/src/test/scala/integration/kafka/api/SaslClientsWithInvalidCredentialsTest.scala
##########
@@ -74,48 +75,58 @@ class SaslClientsWithInvalidCredentialsTest extends IntegrationTestHarness with
@Test
def testProducerWithAuthenticationFailure(): Unit = {
val producer = createProducer()
- verifyAuthenticationException(sendOneRecord(producer, maxWaitMs = 10000))
- verifyAuthenticationException(producer.partitionsFor(topic))
+ try {
+ verifyAuthenticationException(sendOneRecord(producer, maxWaitMs = 10000))
+ verifyAuthenticationException(producer.partitionsFor(topic))
- createClientCredential()
- verifyWithRetry(sendOneRecord(producer))
+ createClientCredential()
+ verifyWithRetry(sendOneRecord(producer))
+ } finally producer.close()
Review comment:
Oh, right, it was suspicious that I did not see consumer thread on the failed .classMethod cases :) Thanks.
testConsumerGroupServiceWithAuthenticationSuccess failed and left adminclient threads for later ones:
```
java.lang.AssertionError: Found unexpected threads during @AfterClass, allThreads=Set(Test worker, metrics-meter-tick-thread-2, main, Signal Dispatcher, Reference Handler, Finalizer, /0:0:0:0:0:0:0:1:55620 to /0:0:0:0:0:0:0:1:33045 workers Thread 2, kafka-admin-client-thread | adminclient-45, /0:0:0:0:0:0:0:1:55620 to /0:0:0:0:0:0:0:1:33045 workers Thread 3, metrics-meter-tick-thread-1), unexpected=Set(kafka-admin-client-thread | adminclient-45)
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] akatona84 commented on a change in pull request #8992: MINOR: Closing resources in SaslClientsWithInvalidCredentialsTest
Posted by GitBox <gi...@apache.org>.
akatona84 commented on a change in pull request #8992:
URL: https://github.com/apache/kafka/pull/8992#discussion_r452108786
##########
File path: core/src/test/scala/integration/kafka/api/SaslClientsWithInvalidCredentialsTest.scala
##########
@@ -74,48 +75,58 @@ class SaslClientsWithInvalidCredentialsTest extends IntegrationTestHarness with
@Test
def testProducerWithAuthenticationFailure(): Unit = {
val producer = createProducer()
- verifyAuthenticationException(sendOneRecord(producer, maxWaitMs = 10000))
- verifyAuthenticationException(producer.partitionsFor(topic))
+ try {
+ verifyAuthenticationException(sendOneRecord(producer, maxWaitMs = 10000))
+ verifyAuthenticationException(producer.partitionsFor(topic))
- createClientCredential()
- verifyWithRetry(sendOneRecord(producer))
+ createClientCredential()
+ verifyWithRetry(sendOneRecord(producer))
+ } finally producer.close()
Review comment:
Oh, right, it was suspicious that I did not see consumer thread on the failed .classMethod cases :) Thanks.
testConsumerGroupServiceWithAuthenticationSuccess failed and left adminclient threads for later ones:
> java.lang.AssertionError: Found unexpected threads during @AfterClass, allThreads=Set(Test worker, metrics-meter-tick-thread-2, main, Signal Dispatcher, Reference Handler, Finalizer, /0:0:0:0:0:0:0:1:55620 to /0:0:0:0:0:0:0:1:33045 workers Thread 2, kafka-admin-client-thread | adminclient-45, /0:0:0:0:0:0:0:1:55620 to /0:0:0:0:0:0:0:1:33045 workers Thread 3, metrics-meter-tick-thread-1), unexpected=Set(kafka-admin-client-thread | adminclient-45)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] akatona84 commented on pull request #8992: MINOR: Closing resources in SaslClientsWithInvalidCredentialsTest
Posted by GitBox <gi...@apache.org>.
akatona84 commented on pull request #8992:
URL: https://github.com/apache/kafka/pull/8992#issuecomment-655458870
Hi @ijuma , I've encountered a test failure here which caused unexpected threads in the following tests `@Before` phase.
This fixes the issue in case failure happen, only that would be reported (and not together with .classMethod ending failures)
Could you take a look on this please?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] omkreddy commented on a change in pull request #8992: MINOR: Closing resources in SaslClientsWithInvalidCredentialsTest
Posted by GitBox <gi...@apache.org>.
omkreddy commented on a change in pull request #8992:
URL: https://github.com/apache/kafka/pull/8992#discussion_r452092180
##########
File path: core/src/test/scala/integration/kafka/api/SaslClientsWithInvalidCredentialsTest.scala
##########
@@ -74,48 +75,58 @@ class SaslClientsWithInvalidCredentialsTest extends IntegrationTestHarness with
@Test
def testProducerWithAuthenticationFailure(): Unit = {
val producer = createProducer()
- verifyAuthenticationException(sendOneRecord(producer, maxWaitMs = 10000))
- verifyAuthenticationException(producer.partitionsFor(topic))
+ try {
+ verifyAuthenticationException(sendOneRecord(producer, maxWaitMs = 10000))
+ verifyAuthenticationException(producer.partitionsFor(topic))
- createClientCredential()
- verifyWithRetry(sendOneRecord(producer))
+ createClientCredential()
+ verifyWithRetry(sendOneRecord(producer))
+ } finally producer.close()
Review comment:
@akatona84 Thanks for the PR. Normally producer/consumer instances created in the test are closed in [IntegrationTestHarness.tearDown ](https://github.com/confluentinc/ce-kafka/blob/master/core/src/test/scala/integration/kafka/api/IntegrationTestHarness.scala#L152). many core tests extends `IntegrationTestHarness` class.
Do we know, which tests were failed?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] akatona84 commented on a change in pull request #8992: MINOR: Closing resources in SaslClientsWithInvalidCredentialsTest
Posted by GitBox <gi...@apache.org>.
akatona84 commented on a change in pull request #8992:
URL: https://github.com/apache/kafka/pull/8992#discussion_r452108786
##########
File path: core/src/test/scala/integration/kafka/api/SaslClientsWithInvalidCredentialsTest.scala
##########
@@ -74,48 +75,58 @@ class SaslClientsWithInvalidCredentialsTest extends IntegrationTestHarness with
@Test
def testProducerWithAuthenticationFailure(): Unit = {
val producer = createProducer()
- verifyAuthenticationException(sendOneRecord(producer, maxWaitMs = 10000))
- verifyAuthenticationException(producer.partitionsFor(topic))
+ try {
+ verifyAuthenticationException(sendOneRecord(producer, maxWaitMs = 10000))
+ verifyAuthenticationException(producer.partitionsFor(topic))
- createClientCredential()
- verifyWithRetry(sendOneRecord(producer))
+ createClientCredential()
+ verifyWithRetry(sendOneRecord(producer))
+ } finally producer.close()
Review comment:
Oh, right, it was suspicious that I did not see consumer thread on the failed .classMethod cases :) Thanks.
testConsumerGroupServiceWithAuthenticationSuccess failed and left adminclient threads for later ones:
> java.lang.AssertionError: Found unexpected threads during `@AfterClass`, allThreads=Set(Test worker, metrics-meter-tick-thread-2, main, Signal Dispatcher, Reference Handler, Finalizer, /0:0:0:0:0:0:0:1:55620 to /0:0:0:0:0:0:0:1:33045 workers Thread 2, kafka-admin-client-thread | adminclient-45, /0:0:0:0:0:0:0:1:55620 to /0:0:0:0:0:0:0:1:33045 workers Thread 3, metrics-meter-tick-thread-1), unexpected=Set(kafka-admin-client-thread | adminclient-45)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] akatona84 commented on pull request #8992: MINOR: Closing resources in SaslClientsWithInvalidCredentialsTest
Posted by GitBox <gi...@apache.org>.
akatona84 commented on pull request #8992:
URL: https://github.com/apache/kafka/pull/8992#issuecomment-656038173
Thanks, @omkreddy for explaining how the consumers and prods are closed, updated my PR so only closing things where it is necessary.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org