Log4j2 and removing outputcaptor (#56)

Author: akshaisarma

README.md

@@ -514,6 +514,7 @@ Version | Notes
 0.7.0   | Added support for running queries in parallel
 0.7.1   | Fixed a bug with parallel query execution in REST and Hive engines. Upgraded to Apache HttpClient 5.x which supports HTTP2 in REST engine.
 0.7.2   | Added query priority to allow executing parallel queries in groups
+0.7.2   | Log4j2
 
 ## Members
 

pom.xml

@@ -46,6 +46,8 @@
 
     <properties>
         <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+        <slf4j.version>1.7.32</slf4j.version>
+        <log4j.version>2.17.1</log4j.version>
     </properties>
 
     <build>
@@ -139,7 +141,7 @@
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-shade-plugin</artifactId>
-                <version>2.4.1</version>
+                <version>3.2.1</version>
                 <executions>
                     <execution>
                         <phase>package</phase>
@@ -166,6 +168,11 @@
                                     <pattern>org.objectweb.asm</pattern>
                                     <shadedPattern>org.shaded.objectweb.asm</shadedPattern>
                                 </relocation>
+                                <!-- Log4j-->
+                                <relocation>
+                                    <pattern>org.apache.logging.log4j</pattern>
+                                    <shadedPattern>org.shaded.apache.logging.log4j</shadedPattern>
+                                </relocation>
                             </relocations>
                         </configuration>
                     </execution>
@@ -364,14 +371,14 @@
          <version>4.6</version>
       </dependency>
       <dependency>
-         <groupId>log4j</groupId>
-         <artifactId>log4j</artifactId>
-         <version>1.2.17</version>
+          <groupId>org.slf4j</groupId>
+          <artifactId>slf4j-api</artifactId>
+          <version>${slf4j.version}</version>
       </dependency>
       <dependency>
-          <groupId>org.slf4j</groupId>
-          <artifactId>slf4j-log4j12</artifactId>
-          <version>1.7.10</version>
+          <groupId>org.apache.logging.log4j</groupId>
+          <artifactId>log4j-slf4j-impl</artifactId>
+          <version>${log4j.version}</version>
       </dependency>
       <dependency>
           <groupId>org.projectlombok</groupId>
@@ -467,12 +474,24 @@
           <groupId>org.apache.hadoop</groupId>
           <artifactId>hadoop-common</artifactId>
           <version>2.6.0</version>
+          <exclusions>
+              <exclusion>
+                  <groupId>org.slf4j</groupId>
+                  <artifactId>slf4j-log4j12</artifactId>
+              </exclusion>
+          </exclusions>
           <scope>provided</scope>
       </dependency>
       <dependency>
           <groupId>org.apache.hadoop</groupId>
           <artifactId>hadoop-client</artifactId>
           <version>2.6.0</version>
+          <exclusions>
+              <exclusion>
+                  <groupId>org.slf4j</groupId>
+                  <artifactId>slf4j-log4j12</artifactId>
+              </exclusion>
+          </exclusions>
           <scope>provided</scope>
       </dependency>
       <dependency>

src/main/resources/log4j.properties

@@ -0,0 +1,9 @@
+status=warn
+
+appender.console.type=Console
+appender.console.name=STDOUT
+appender.console.layout.type=PatternLayout
+appender.console.layout.pattern=[%-5p] %d{ISO8601} %c - %m%n
+
+rootLogger.level=info
+rootLogger.appenderRef.stdout.ref=STDOUT

src/main/resources/log4j2.properties

@@ -5,93 +5,16 @@
 package com.yahoo.validatar;
 
 import org.apache.commons.io.output.NullOutputStream;
-import org.apache.log4j.Appender;
-import org.apache.log4j.Level;
-import org.apache.log4j.Logger;
-import org.apache.log4j.spi.LoggingEvent;
-import org.mockito.ArgumentCaptor;
 
 import java.io.FileDescriptor;
 import java.io.FileOutputStream;
 import java.io.PrintStream;
-import java.util.List;
 
-import static org.mockito.Mockito.atLeastOnce;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.never;
-import static org.mockito.Mockito.verify;
-
-/**
- * Extend this class to capture logging output without printing to stdout
- * Call setupMockedAppender and teardownMockedAppender to start and stop
- * the capturing and use isStringInLog to check if desired output in log
- * was seen.
- */
 public class OutputCaptor {
-    protected Appender mockedAppender;
-    protected Level originalLevel;
-    protected Appender originalAppender;
-
     public static final PrintStream NULL = new PrintStream(NullOutputStream.NULL_OUTPUT_STREAM);
     public static final PrintStream OUT = new PrintStream(new FileOutputStream(FileDescriptor.out));
     public static final PrintStream ERR = new PrintStream(new FileOutputStream(FileDescriptor.err));
 
-    private boolean contains(String source, String target, boolean isCaseSensitive) {
-        String superString = source;
-        String subString = target;
-        if (!isCaseSensitive) {
-            superString = superString.toLowerCase();
-            subString = subString.toLowerCase();
-        }
-        return superString.contains(subString);
-    }
-
-    protected boolean isStringInLog(String message) {
-        return isStringInLog(message, false);
-    }
-
-    protected boolean isStringInLog(String message, boolean isCaseSensitive) {
-        ArgumentCaptor<LoggingEvent> arguments = ArgumentCaptor.forClass(LoggingEvent.class);
-        verify(mockedAppender, atLeastOnce()).doAppend(arguments.capture());
-        List<LoggingEvent> events = arguments.getAllValues();
-        for (LoggingEvent event : events) {
-            String log = event.getRenderedMessage();
-            if (contains(log, message, isCaseSensitive)) {
-                return true;
-            }
-        }
-        return false;
-    }
-
-    protected boolean noStringInLog() {
-        ArgumentCaptor<LoggingEvent> arguments = ArgumentCaptor.forClass(LoggingEvent.class);
-        try {
-            verify(mockedAppender, never()).doAppend(arguments.capture());
-        } catch (Exception e) {
-            return false;
-        }
-        return true;
-    }
-
-    protected void setupMockedAppender() {
-        setupMockedAppender(Level.ERROR);
-    }
-
-    protected void setupMockedAppender(Level level) {
-        originalLevel = Logger.getRootLogger().getLevel();
-        originalAppender = Logger.getRootLogger().getAppender("stdout");
-        Logger.getRootLogger().removeAllAppenders();
-        mockedAppender = mock(Appender.class);
-        Logger.getRootLogger().setLevel(level);
-        Logger.getRootLogger().addAppender(mockedAppender);
-    }
-
-    protected void teardownMockedAppender() {
-        Logger.getRootLogger().removeAllAppenders();
-        Logger.getRootLogger().setLevel(originalLevel);
-        Logger.getRootLogger().addAppender(originalAppender);
-    }
-
     public static void redirectToDevNull() {
         System.setOut(NULL);
         System.setErr(NULL);
@@ -108,4 +31,3 @@ public static void runWithoutOutput(Runnable function) {
         redirectToStandard();
     }
 }
-

src/test/java/com/yahoo/validatar/OutputCaptor.java

@@ -4,14 +4,12 @@
  */
 package com.yahoo.validatar.execution;
 
-import com.yahoo.validatar.OutputCaptor;
 import com.yahoo.validatar.common.Column;
 import com.yahoo.validatar.common.Query;
 import com.yahoo.validatar.common.Result;
 import com.yahoo.validatar.common.TypeSystem;
 import com.yahoo.validatar.common.TypedObject;
 import org.testng.Assert;
-import org.testng.annotations.AfterMethod;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
@@ -22,8 +20,9 @@
 import java.util.List;
 import java.util.Map;
 
+import static com.yahoo.validatar.OutputCaptor.runWithoutOutput;
 
-public class EngineManagerTest extends OutputCaptor {
+public class EngineManagerTest {
 
     private class MockFailingEngine implements Engine {
         public static final String ENGINE_NAME = "FAILER";
@@ -155,7 +154,6 @@ public String getName() {
 
     @BeforeMethod
     public void setup() {
-        setupMockedAppender();
         query = new Query();
         query.engine = MockPassingEngine.ENGINE_NAME;
         queries = new ArrayList<>();
@@ -168,33 +166,25 @@ public void setup() {
         manager = new EngineManager(args);
     }
 
-    @AfterMethod
-    public void teardown() {
-        teardownMockedAppender();
-    }
-
     @Test
     public void testEngineFailToStart() {
         query.engine = MockFailingEngine.ENGINE_NAME;
         manager.setEngines(engines);
         Assert.assertFalse(manager.startEngines(queries));
-        Assert.assertTrue(isStringInLog("Required engine " + MockFailingEngine.ENGINE_NAME + " could not be setup"));
     }
 
     @Test
     public void testEngineNotPresent() {
         query.engine = "FAKE_ENGINE";
         manager.setEngines(engines);
         Assert.assertFalse(manager.startEngines(queries));
-        Assert.assertTrue(isStringInLog("Engine FAKE_ENGINE not loaded but required by query"));
     }
 
     @Test
     public void testEngineNullQueriesNotNull() {
         query.engine = null;
         manager.setEngines(engines);
         Assert.assertFalse(manager.startEngines(queries));
-        Assert.assertTrue(isStringInLog("Engine null not loaded but required by query"));
     }
 
     @Test(expectedExceptions = {NullPointerException.class})

src/test/java/com/yahoo/validatar/execution/EngineManagerTest.java

@@ -0,0 +1,9 @@
+status=warn
+
+appender.console.type=Console
+appender.console.name=STDOUT
+appender.console.layout.type=PatternLayout
+appender.console.layout.pattern=[%-5p] %d{ISO8601} %c - %m%n
+
+rootLogger.level=fatal
+rootLogger.appenderRef.stdout.ref=STDOUT