Merge pull request apache#5202 from hansva/5198

Send general log to SLF4J + cleanup and Junit4 migration, apache#5198

Author: hansva

core/src/main/java/org/apache/hop/core/logging/Slf4jLoggingEventListener.java

@@ -36,16 +36,15 @@ public class Slf4jLoggingEventListener implements IHopLoggingEventListener {
   @VisibleForTesting
   Logger pipelineLogger = LoggerFactory.getLogger("org.apache.hop.pipeline.Pipeline");
 
-  @VisibleForTesting Logger jobLogger = LoggerFactory.getLogger("org.apache.hop.workflow.Workflow");
+  @VisibleForTesting
+  Logger workflowLogger = LoggerFactory.getLogger("org.apache.hop.workflow.Workflow");
 
-  @VisibleForTesting Logger diLogger = LoggerFactory.getLogger("org.apache.hop");
+  @VisibleForTesting Logger hopLogger = LoggerFactory.getLogger("org.apache.hop");
 
   @VisibleForTesting
   Function<String, ILoggingObject> logObjProvider =
       objId -> LoggingRegistry.getInstance().getLoggingObject(objId);
 
-  private static final String SEPARATOR = "/";
-
   public Slf4jLoggingEventListener() {
     // Do nothing
   }
@@ -60,14 +59,16 @@ public void eventAdded(HopLoggingEvent event) {
       if (loggingObject == null) {
         // this can happen if logObject has been discarded while log events are still in flight.
         logToLogger(
-            diLogger, message.getLevel(), message.getSubject() + " " + message.getMessage());
+            hopLogger, message.getLevel(), message.getSubject() + " " + message.getMessage());
       } else if (loggingObject.getObjectType() == PIPELINE
           || loggingObject.getObjectType() == TRANSFORM
           || loggingObject.getObjectType() == DATABASE) {
         logToLogger(pipelineLogger, message.getLevel(), loggingObject, message);
       } else if (loggingObject.getObjectType() == WORKFLOW
           || loggingObject.getObjectType() == ACTION) {
-        logToLogger(jobLogger, message.getLevel(), loggingObject, message);
+        logToLogger(workflowLogger, message.getLevel(), loggingObject, message);
+      } else {
+        logToLogger(hopLogger, message.getLevel(), loggingObject, message);
       }
     }
   }
@@ -107,7 +108,7 @@ private String getDetailedSubject(ILoggingObject loggingObject) {
     while (loggingObject != null) {
       if (loggingObject.getObjectType() == PIPELINE || loggingObject.getObjectType() == WORKFLOW) {
         String filename = loggingObject.getFilename();
-        if (filename != null && filename.length() > 0) {
+        if (filename != null && !filename.isEmpty()) {
           subjects.add(filename);
         }
       }

core/src/test/java/org/apache/hop/core/logging/Slf4jLoggingEventListenerTest.java

@@ -19,22 +19,23 @@
 
 import static org.apache.hop.core.logging.LogLevel.BASIC;
 import static org.apache.hop.core.logging.LogLevel.ERROR;
+import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.when;
 
 import java.util.function.Function;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.Mock;
-import org.mockito.junit.MockitoJUnitRunner;
+import org.mockito.junit.jupiter.MockitoExtension;
 import org.slf4j.Logger;
 
-@RunWith(MockitoJUnitRunner.class)
-public class Slf4jLoggingEventListenerTest {
+@ExtendWith(MockitoExtension.class)
+class Slf4jLoggingEventListenerTest {
 
-  @Mock private Logger pipelineLogger, jobLogger, diLogger;
+  @Mock private Logger pipelineLogger, workflowLogger, hopLogger;
   @Mock private HopLoggingEvent logEvent;
   @Mock private ILoggingObject loggingObject;
   @Mock private LogMessage message;
@@ -47,61 +48,61 @@ public class Slf4jLoggingEventListenerTest {
 
   private Slf4jLoggingEventListener listener = new Slf4jLoggingEventListener();
 
-  @Before
-  public void before() {
+  @BeforeEach
+  void before() {
     listener.pipelineLogger = pipelineLogger;
-    listener.jobLogger = jobLogger;
-    listener.diLogger = diLogger;
+    listener.workflowLogger = workflowLogger;
+    listener.hopLogger = hopLogger;
     listener.logObjProvider = logObjProvider;
-    when(logEvent.getMessage()).thenReturn(message);
-    when(message.getLogChannelId()).thenReturn(logChannelId);
-    when(message.getLevel()).thenReturn(logLevel);
-    when(message.getMessage()).thenReturn(msgText);
-    when(message.getSubject()).thenReturn(messageSub);
+    lenient().when(logEvent.getMessage()).thenReturn(message);
+    lenient().when(message.getLogChannelId()).thenReturn(logChannelId);
+    lenient().when(message.getLevel()).thenReturn(logLevel);
+    lenient().when(message.getMessage()).thenReturn(msgText);
+    lenient().when(message.getSubject()).thenReturn(messageSub);
   }
 
   @Test
-  public void testAddLogEventNoRegisteredLogObject() {
+  void testAddLogEventNoRegisteredLogObject() {
     listener.eventAdded(logEvent);
-    verify(diLogger).info(messageSub + " " + msgText);
+    verify(hopLogger).info(String.format("%s %s", messageSub, msgText));
 
     when(message.getLevel()).thenReturn(ERROR);
     listener.eventAdded(logEvent);
-    verify(diLogger).error(messageSub + " " + msgText);
+    verify(hopLogger).error(String.format("%s %s", messageSub, msgText));
     verifyNoInteractions(pipelineLogger);
-    verifyNoInteractions(jobLogger);
+    verifyNoInteractions(workflowLogger);
   }
 
   @Test
-  public void testAddLogEventPipeline() {
+  void testAddLogEventPipeline() {
     when(logObjProvider.apply(logChannelId)).thenReturn(loggingObject);
     when(loggingObject.getObjectType()).thenReturn(LoggingObjectType.PIPELINE);
     when(loggingObject.getFilename()).thenReturn("filename");
     when(message.getLevel()).thenReturn(LogLevel.BASIC);
     listener.eventAdded(logEvent);
 
-    verify(pipelineLogger).info("[filename]  " + msgText);
+    verify(pipelineLogger).info(String.format("[filename]  %s", msgText));
     when(message.getLevel()).thenReturn(LogLevel.ERROR);
     listener.eventAdded(logEvent);
-    verify(pipelineLogger).error("[filename]  " + msgText);
-    verifyNoInteractions(diLogger);
-    verifyNoInteractions(jobLogger);
+    verify(pipelineLogger).error(String.format("[filename]  %s", msgText));
+    verifyNoInteractions(hopLogger);
+    verifyNoInteractions(workflowLogger);
   }
 
   @Test
-  public void testAddLogEventJob() {
+  void testAddLogEventJob() {
     when(logObjProvider.apply(logChannelId)).thenReturn(loggingObject);
     when(loggingObject.getObjectType()).thenReturn(LoggingObjectType.WORKFLOW);
     when(loggingObject.getFilename()).thenReturn("filename");
     when(message.getLevel()).thenReturn(LogLevel.BASIC);
     listener.eventAdded(logEvent);
 
-    verify(jobLogger).info("[filename]  " + msgText);
+    verify(workflowLogger).info(String.format("[filename]  %s", msgText));
 
     when(message.getLevel()).thenReturn(LogLevel.ERROR);
     listener.eventAdded(logEvent);
-    verify(jobLogger).error("[filename]  " + msgText);
-    verifyNoInteractions(diLogger);
+    verify(workflowLogger).error(String.format("[filename]  %s", msgText));
+    verifyNoInteractions(hopLogger);
     verifyNoInteractions(pipelineLogger);
   }
 }

pom.xml

@@ -201,6 +201,12 @@
             <version>${mockito-core.version}</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-junit-jupiter</artifactId>
+            <version>${mockito-core.version}</version>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>org.objenesis</groupId>
             <artifactId>objenesis</artifactId>