asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Blow (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: [NO ISSUE][TEST] Use logger in place of System.err for TestE...
Date Tue, 30 Jan 2018 19:13:09 GMT
Michael Blow has submitted this change and it was merged.

Change subject: [NO ISSUE][TEST] Use logger in place of System.err for TestExecutor
......................................................................


[NO ISSUE][TEST] Use logger in place of System.err for TestExecutor

Change-Id: Ia53aa832b29d03257a48cc5d198703bcf94dd4ca
Reviewed-on: https://asterix-gerrit.ics.uci.edu/2338
Sonar-Qube: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Reviewed-by: Murtadha Hubail <mhubail@apache.org>
Integration-Tests: Murtadha Hubail <mhubail@apache.org>
Tested-by: Murtadha Hubail <mhubail@apache.org>
---
M asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java
M asterixdb/asterix-app/src/test/java/org/apache/asterix/test/sqlpp/ParserTestExecutor.java
M asterixdb/asterix-app/src/test/resources/log4j2-test.xml
M hyracks-fullstack/src/test/resources/log4j2-test.xml
4 files changed, 45 insertions(+), 48 deletions(-)

Approvals:
  Jenkins: No violations found
  Murtadha Hubail: Looks good to me, approved; Verified; Verified



diff --git a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java
b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java
index deb2c72..9cce657 100644
--- a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java
+++ b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java
@@ -187,9 +187,9 @@
         return path.delete();
     }
 
-    public void runScriptAndCompareWithResult(File scriptFile, PrintWriter print, File expectedFile,
File actualFile,
+    public void runScriptAndCompareWithResult(File scriptFile, File expectedFile, File actualFile,
             ComparisonEnum compare) throws Exception {
-        System.err.println("Expected results file: " + expectedFile.toString());
+        LOGGER.info("Expected results file: {} ", expectedFile);
         BufferedReader readerExpected =
                 new BufferedReader(new InputStreamReader(new FileInputStream(expectedFile),
"UTF-8"));
         BufferedReader readerActual =
@@ -255,7 +255,7 @@
                 throw createLineChangedException(scriptFile, "<EOF>", lineActual, num);
             }
         } catch (Exception e) {
-            System.err.println("Actual results file: " + actualFile.toString());
+            LOGGER.info("Actual results file: {}", actualFile);
             throw e;
         } finally {
             readerExpected.close();
@@ -946,8 +946,7 @@
                         + "_qar.adm");
                 writeOutputToFile(qarFile, resultStream);
                 qbcFile = getTestCaseQueryBeforeCrashFile(actualPath, testCaseCtx, cUnit);
-                runScriptAndCompareWithResult(testFile, new PrintWriter(System.err), qbcFile,
qarFile,
-                        ComparisonEnum.TEXT);
+                runScriptAndCompareWithResult(testFile, qbcFile, qarFile, ComparisonEnum.TEXT);
                 break;
             case "txneu": // eu represents erroneous update
                 try {
@@ -955,12 +954,11 @@
                 } catch (Exception e) {
                     // An exception is expected.
                     failed = true;
-                    System.err.println("testFile " + testFile.toString() + " raised an exception:
" + e);
+                    LOGGER.info("testFile {} raised an (expected) exception", testFile, e.toString());
                 }
                 if (!failed) {
-                    throw new Exception("Test \"" + testFile + "\" FAILED!\n  An exception"
+ "is expected.");
+                    throw new Exception("Test \"" + testFile + "\" FAILED; an exception was
expected");
                 }
-                System.err.println("...but that was expected.");
                 break;
             case "script":
                 try {
@@ -970,7 +968,7 @@
                         throw new Exception(output);
                     }
                 } catch (Exception e) {
-                    throw new Exception("Test \"" + testFile + "\" FAILED!\n", e);
+                    throw new Exception("Test \"" + testFile + "\" FAILED!", e);
                 }
                 break;
             case "sleep":
@@ -983,12 +981,11 @@
                 } catch (Exception e) {
                     // expected error happens
                     failed = true;
-                    System.err.println("testFile " + testFile.toString() + " raised an exception:
" + e);
+                    LOGGER.info("testFile {} raised an (expected) exception", testFile, e.toString());
                 }
                 if (!failed) {
-                    throw new Exception("Test \"" + testFile + "\" FAILED!\n  An exception
is expected.");
+                    throw new Exception("Test \"" + testFile + "\" FAILED; an exception was
expected");
                 }
-                System.err.println("...but that was expected.");
                 break;
             case "get":
             case "post":
@@ -1186,17 +1183,14 @@
         } else {
             if (expectedResultFile == null) {
                 if (testFile.getName().startsWith(DIAGNOSE)) {
-                    System.err.println("Diagnostic Output:");
-                    IOUtils.copy(resultStream, System.err);
-                    System.err.println();
+                    LOGGER.info("Diagnostic output: {}", IOUtils.toString(resultStream, StandardCharsets.UTF_8));
                 } else {
                     Assert.fail("no result file for " + testFile.toString() + "; queryCount:
" + queryCount
                             + ", filectxs.size: " + numResultFiles);
                 }
             } else {
                 writeOutputToFile(actualResultFile, resultStream);
-                runScriptAndCompareWithResult(testFile, new PrintWriter(System.err), expectedResultFile,
-                        actualResultFile, compare);
+                runScriptAndCompareWithResult(testFile, expectedResultFile, actualResultFile,
compare);
             }
         }
         queryCount.increment();
@@ -1240,9 +1234,7 @@
         }
         if (actualResultFile == null) {
             if (testFile.getName().startsWith(DIAGNOSE)) {
-                System.err.println("Diagnostic Output:");
-                IOUtils.copy(resultStream, System.err);
-                System.err.println();
+                LOGGER.info("Diagnostic output: {}", IOUtils.toString(resultStream, StandardCharsets.UTF_8));
             } else {
                 Assert.fail("no result file for " + testFile.toString() + "; queryCount:
" + queryCount
                         + ", filectxs.size: " + numResultFiles);
@@ -1257,8 +1249,7 @@
                         + ", filectxs.size: " + numResultFiles);
             }
         }
-        runScriptAndCompareWithResult(testFile, new PrintWriter(System.err), expectedResultFile,
actualResultFile,
-                compare);
+        runScriptAndCompareWithResult(testFile, expectedResultFile, actualResultFile, compare);
         if (!reqType.equals("validate")) {
             queryCount.increment();
         }
@@ -1543,28 +1534,26 @@
                         queryCount.setValue(savedQueryCounts[numOfFiles]);
                     }
                 } catch (Exception e) {
-                    System.err.println("testFile " + testFile.toString() + " raised an exception:
" + e);
                     numOfErrors++;
-                    if (isUnExpected(e, expectedErrors, numOfErrors, queryCount)) {
-                        e.printStackTrace();
-                        System.err.println("...Unexpected!");
+                    boolean unexpected = isUnExpected(e, expectedErrors, numOfErrors, queryCount);
+                    if (unexpected) {
+                        LOGGER.error("testFile {} raised an unexpected exception", testFile,
e);
                         if (failedGroup != null) {
                             failedGroup.getTestCase().add(testCaseCtx.getTestCase());
                         }
                         fail(true, testCaseCtx, cUnit, testFileCtxs, pb, testFile, e);
+                    } else {
+                        LOGGER.info("testFile {} raised an (expected) exception", testFile,
e.toString());
                     }
-                } finally {
-                    if (numOfFiles == testFileCtxs.size()) {
-                        if (numOfErrors < cUnit.getExpectedError().size()) {
-                            System.err.println("...Unexpected!");
-                            Exception e = new Exception(
-                                    "Test \"" + cUnit.getName() + "\" FAILED!\nExpected error
was not thrown...");
-                            System.err.println(e);
-                            throw e;
-                        }
-                        LOGGER.info("[TEST]: " + testCaseCtx.getTestCase().getFilePath()
+ "/" + cUnit.getName()
-                                + " PASSED ");
+                }
+                if (numOfFiles == testFileCtxs.size()) {
+                    if (numOfErrors < cUnit.getExpectedError().size()) {
+                        LOGGER.error("Test {} failed to raise (an) expected exception(s)",
cUnit.getName());
+                        throw new Exception(
+                                "Test \"" + cUnit.getName() + "\" FAILED; expected exception
was not thrown...");
                     }
+                    LOGGER.info(
+                            "[TEST]: " + testCaseCtx.getTestCase().getFilePath() + "/" +
cUnit.getName() + " PASSED ");
                 }
             }
         }
@@ -1576,8 +1565,7 @@
             try {
                 // execute diagnostic files
                 Map<String, Object> variableCtx = new HashMap<>();
-                for (ListIterator<TestFileContext> iter = testFileCtxs.listIterator();
iter.hasNext();) {
-                    TestFileContext ctx = iter.next();
+                for (TestFileContext ctx : testFileCtxs) {
                     if (ctx.getFile().getName().startsWith(TestExecutor.DIAGNOSE)) {
                         // execute the file
                         final File file = ctx.getFile();
@@ -1601,11 +1589,9 @@
             // Get the expected exception
             expectedError = expectedErrors.get(numOfErrors - 1);
             if (e.toString().contains(expectedError)) {
-                System.err.println("...but that was expected.");
                 return false;
             } else {
-                System.err
-                        .println("Expected to find the following in error text:\n+++++\n"
+ expectedError + "\n+++++");
+                LOGGER.error("Expected to find the following in error text: +++++{}+++++",
expectedError);
                 return true;
             }
         }
diff --git a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/sqlpp/ParserTestExecutor.java
b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/sqlpp/ParserTestExecutor.java
index 85b8c8a..0d0c1d9 100644
--- a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/sqlpp/ParserTestExecutor.java
+++ b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/sqlpp/ParserTestExecutor.java
@@ -149,8 +149,7 @@
             }
             writer.close();
             // Compares the actual result and the expected result.
-            runScriptAndCompareWithResult(queryFile, new PrintWriter(System.err), expectedFile,
actualResultFile,
-                    ComparisonEnum.TEXT);
+            runScriptAndCompareWithResult(queryFile, expectedFile, actualResultFile, ComparisonEnum.TEXT);
         } catch (Exception e) {
             GlobalConfig.ASTERIX_LOGGER.warn("Failed while testing file " + queryFile);
             throw e;
diff --git a/asterixdb/asterix-app/src/test/resources/log4j2-test.xml b/asterixdb/asterix-app/src/test/resources/log4j2-test.xml
index 3edb700..387e900 100644
--- a/asterixdb/asterix-app/src/test/resources/log4j2-test.xml
+++ b/asterixdb/asterix-app/src/test/resources/log4j2-test.xml
@@ -24,6 +24,9 @@
     <File name="InfoLog" fileName="target/info.log">
         <PatternLayout pattern="%d{HH:mm:ss.SSS} [%t] %-5level %logger{36} - %msg%n"/>
     </File>
+    <Console name="ConsoleTest" target="SYSTEM_OUT">
+      <PatternLayout pattern="%d{HH:mm:ss.SSS} [%t] %c{1} - %msg%n"/>
+    </Console>
   </Appenders>
   <Loggers>
     <Root level="WARN">
@@ -35,8 +38,13 @@
     <Logger name="org.apache.asterix" level="INFO" additivity="false">
       <AppenderRef ref="InfoLog"/>
     </Logger>
-    <Logger name="org.apache.asterix.test" level="WARN">
-      <AppenderRef ref="Console"/>
+    <Logger name="org.apache.hyracks.test" level="INFO" additivity="false">
+      <AppenderRef ref="ConsoleTest"/>
+      <AppenderRef ref="InfoLog"/>
+    </Logger>
+    <Logger name="org.apache.asterix.test" level="INFO" additivity="false">
+      <AppenderRef ref="ConsoleTest"/>
+      <AppenderRef ref="InfoLog"/>
     </Logger>
   </Loggers>
 </Configuration>
diff --git a/hyracks-fullstack/src/test/resources/log4j2-test.xml b/hyracks-fullstack/src/test/resources/log4j2-test.xml
index 71c5612..d56f215 100644
--- a/hyracks-fullstack/src/test/resources/log4j2-test.xml
+++ b/hyracks-fullstack/src/test/resources/log4j2-test.xml
@@ -24,6 +24,9 @@
     <File name="InfoLog" fileName="target/info.log">
       <PatternLayout pattern="%d{HH:mm:ss.SSS} [%t] %-5level %logger{36} - %msg%n"/>
     </File>
+    <Console name="ConsoleTest" target="SYSTEM_OUT">
+      <PatternLayout pattern="%d{HH:mm:ss.SSS} [%t] %c{1} - %msg%n"/>
+    </Console>
   </Appenders>
   <Loggers>
     <Root level="WARN">
@@ -32,8 +35,9 @@
     <Logger name="org.apache.hyracks" level="INFO" additivity="false">
       <AppenderRef ref="InfoLog"/>
     </Logger>
-    <Logger name="org.apache.hyracks.test" level="WARN">
-      <AppenderRef ref="Console"/>
+    <Logger name="org.apache.hyracks.test" level="INFO" additivity="false">
+      <AppenderRef ref="ConsoleTest"/>
+      <AppenderRef ref="InfoLog"/>
     </Logger>
   </Loggers>
 </Configuration>

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/2338
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ia53aa832b29d03257a48cc5d198703bcf94dd4ca
Gerrit-PatchSet: 4
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Michael Blow <mblow@apache.org>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mblow@apache.org>
Gerrit-Reviewer: Murtadha Hubail <mhubail@apache.org>

Mime
View raw message