vy commented on a change in pull request #428:
URL: https://github.com/apache/logging-log4j2/pull/428#discussion_r522189283
##########
File path:
log4j-kafka/src/test/java/org/apache/logging/log4j/kafka/appender/KafkaThreadTest.java
##########
@@ -0,0 +1,33 @@
+package org.apache.logging.log4j.kafka.appender;
+
+import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.junit.LoggerContextRule;
+import org.junit.Rule;
+import org.junit.Test;
+import java.util.Map;
+
+import static org.junit.Assert.assertTrue;
+
+public class KafkaThreadTest {
+
+ @Rule
+ public LoggerContextRule ctx = new
LoggerContextRule("KafkaAppenderTest.xml");
+
+ @Test
+ public void testKafkaThreadLeak(){
+ LoggerContext context = ctx.getLoggerContext();
+ context.getContext(false).reconfigure();
+ context.getContext(false).reconfigure();
+ Map<Thread, StackTraceElement[]> maps = Thread.getAllStackTraces();
+ int kafkaThreadCount = 0;
+ if (maps != null && maps.size() >0) {
+ for (Thread thread : maps.keySet()) {
+ if (thread.getName().startsWith("kafka-producer")){
+ kafkaThreadCount++;
+ }
+ }
+ }
+ assertTrue(kafkaThreadCount <= 5);
+ }
Review comment:
Magical numbers in the tests, e.g., 5 here, have strong potential to
bite us in the back later on. Would you mind revising this test as follows,
please?
```java
@Test
void test_kafka_producer_threads_are_not_leaking_after_context_restart() {
// Determine the initial number of threads.
final LoggerContext context = ctx.getLoggerContext();
final int initialThreadCount = kafkaProducerThreadCount();
// Perform context restarts.
final int contextRestartCount = 3;
for (int i = 0; i < contextRestartCount; i++) {
context.getContext(false).reconfigure();
}
// Verify the final thread count.
final int lastThreadCount = kafkaProducerThreadCount();
assertThat(lastThreadCount).equalsTo(initialThreadCount);
}
private static int kafkaProducerThreadCount() {
// Return the number of threads prefixed with "kafka-producer".
}
```
Note that,
- This is sort of a pseudo-code I jotted in the comment box, hence you need
to fill it in yourself.
- Try to use JUnit 5, if possible. (You can see examples in the source code.)
----------------------------------------------------------------
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:
[email protected]
