From f73154048edf6342c62f9b759be0759f75f11715 Mon Sep 17 00:00:00 2001 From: Robert Hou Date: Sat, 11 Feb 2017 22:41:38 -0800 Subject: [PATCH] Added a new failure type, ORDERBY_FAILURE, which occurs if a SQL statement has an order-by clause that cannot be validated by the Test Framework. Please see the README.md file for information on how to construct a SQL statement with an order-by clause that can be validated by the Test Framework. Added a new verification method, "text", which checks if the actual output and expected output are exactly the same, byte by byte. It verifies content as well as order. Every row that is returned must be returned in the same order as in the expected output. Added a whitelist file for orderby tests. Tests that are listed in this file are SQL statements with an order-by clause. The order-by clause cannot be validated. Instead of marking these tests as failed, they are PASSED until they can be fixed in the future. There are 302 tests that are currently failing at the time of this commit. They will need to be fixed at some piont in the future. --- conf/drillTestConfig.properties.example | 5 ++ .../drill/test/framework/DrillTestJdbc.java | 19 +++++++ .../drill/test/framework/DrillTestOdbc.java | 5 ++ .../drill/test/framework/DrillTestScript.java | 2 + .../drill/test/framework/TestDriver.java | 41 ++++++++++++--- .../drill/test/framework/TestVerifier.java | 51 ++++++++++++++----- .../apache/drill/test/framework/Utils.java | 25 +++++++++ 7 files changed, 128 insertions(+), 20 deletions(-) diff --git a/conf/drillTestConfig.properties.example b/conf/drillTestConfig.properties.example index b2dcf9481..bd7079c2f 100644 --- a/conf/drillTestConfig.properties.example +++ b/conf/drillTestConfig.properties.example @@ -77,6 +77,11 @@ TIME_OUT_SECONDS=1200 ######################################## RESTART_DRILL_SCRIPT=resources/bin/drill-restart.sh +######################################## +# Whitelist for orderby tests that cannot be verified +######################################## +DRILL_ORDERBY_WHITELIST_FILE=framework/orderbyWhitelist + ######################################## ### Tests run as the following user ########################################## diff --git a/framework/src/main/java/org/apache/drill/test/framework/DrillTestJdbc.java b/framework/src/main/java/org/apache/drill/test/framework/DrillTestJdbc.java index f2c950ff7..e53013643 100644 --- a/framework/src/main/java/org/apache/drill/test/framework/DrillTestJdbc.java +++ b/framework/src/main/java/org/apache/drill/test/framework/DrillTestJdbc.java @@ -22,6 +22,7 @@ import org.apache.drill.test.framework.TestCaseModeler.TestMatrix; import org.apache.drill.test.framework.TestVerifier.TestStatus; import org.apache.drill.test.framework.TestVerifier.VerificationException; +import org.apache.drill.test.framework.TestVerifier.OrderbyException; import org.apache.log4j.Logger; import java.io.BufferedWriter; @@ -103,9 +104,17 @@ public void run() { testVerifier = new TestVerifier(columnTypes, query, columnLabels, matrix.verificationTypes); if (query.startsWith("explain") || matrix.verificationTypes.get(0).equalsIgnoreCase("regex") || matrix.verificationTypes.get(0).equalsIgnoreCase("regex-no-order") || + // for explain plans where the patterns may match in a different order than + // specified in the expected results file matrix.verificationTypes.get(0).equalsIgnoreCase("filter-ratio")) { + // special verification method for statistics feature implemented in + // Drill 1.9 where two steps in the explain plan have to be + // evaluated together setTestStatus(testVerifier.verifyTextPlan(modeler.expectedFilename, outputFilename)); + } else if (matrix.verificationTypes.get(0).equalsIgnoreCase("text") ) { + setTestStatus(testVerifier.verifyText(modeler.expectedFilename, outputFilename)); } else { + // "in-memory" setTestStatus(testVerifier.verifyResultSet(modeler.expectedFilename, outputFilename)); } @@ -115,6 +124,16 @@ public void run() { } } catch (VerificationException e) { fail(TestStatus.VERIFICATION_FAILURE, e); + } catch (OrderbyException e) { + // check if test is white-listed, in which case the orderby clause is not validated + if (!Utils.isOrderbyWhitelist(modeler.queryFilename)) { + // test is not white-listed, so the orderby clause needs to be validated + fail(TestStatus.ORDERBY_FAILURE, e); + } else { + // test is white-listed, so the orderby clause is not validated, and is skipped + // pass the test for now until the orderby clause can be validated + setTestStatus(TestStatus.PASS); + } } catch (Exception e) { fail(TestStatus.EXECUTION_FAILURE, e); } finally { diff --git a/framework/src/main/java/org/apache/drill/test/framework/DrillTestOdbc.java b/framework/src/main/java/org/apache/drill/test/framework/DrillTestOdbc.java index 21b6f1951..8d8f2ce8c 100644 --- a/framework/src/main/java/org/apache/drill/test/framework/DrillTestOdbc.java +++ b/framework/src/main/java/org/apache/drill/test/framework/DrillTestOdbc.java @@ -22,6 +22,7 @@ import org.apache.drill.test.framework.TestCaseModeler.TestMatrix; import org.apache.drill.test.framework.TestVerifier.TestStatus; import org.apache.drill.test.framework.TestVerifier.VerificationException; +import org.apache.drill.test.framework.TestVerifier.OrderbyException; import org.apache.log4j.Logger; import java.io.BufferedWriter; @@ -84,6 +85,8 @@ public void run() { setTestStatus(testVerifier.verifyResultSet(modeler.expectedFilename, outputFilename)); } catch (VerificationException e) { fail(TestStatus.VERIFICATION_FAILURE, e); + } catch (OrderbyException e) { + fail(TestStatus.ORDERBY_FAILURE, e); }; break; case 1: @@ -100,6 +103,8 @@ public void run() { break; case 5: setTestStatus(TestStatus.CANCELED); + case 6: + setTestStatus(TestStatus.ORDERBY_FAILURE); default: setTestStatus(TestStatus.EXECUTION_FAILURE); } diff --git a/framework/src/main/java/org/apache/drill/test/framework/DrillTestScript.java b/framework/src/main/java/org/apache/drill/test/framework/DrillTestScript.java index 72d7e47b5..a4c48adaf 100644 --- a/framework/src/main/java/org/apache/drill/test/framework/DrillTestScript.java +++ b/framework/src/main/java/org/apache/drill/test/framework/DrillTestScript.java @@ -98,6 +98,8 @@ public void run() { break; case 5: setTestStatus(TestStatus.CANCELED); + case 6: + setTestStatus(TestStatus.ORDERBY_FAILURE); default: setTestStatus(TestStatus.EXECUTION_FAILURE); } diff --git a/framework/src/main/java/org/apache/drill/test/framework/TestDriver.java b/framework/src/main/java/org/apache/drill/test/framework/TestDriver.java index e3e264daa..a8fcadded 100644 --- a/framework/src/main/java/org/apache/drill/test/framework/TestDriver.java +++ b/framework/src/main/java/org/apache/drill/test/framework/TestDriver.java @@ -68,6 +68,8 @@ public class TestDriver { private String version; private long [][] memUsage = new long[2][3]; private String memUsageFilename = null; + private static String orderbyWhitelistFile = drillProperties.get("DRILL_ORDERBY_WHITELIST_FILE"); + public static Set orderbyWhitelist = null; private static Configuration conf = new Configuration(); public static final Options OPTIONS = new Options(); @@ -207,7 +209,8 @@ public void generateReports(List tests, int iteration) { } document.set("query", query); document.set("status", test.getTestStatus().toString()); - if(test.getTestStatus().equals(TestStatus.EXECUTION_FAILURE) || test.getTestStatus().equals(TestStatus.VERIFICATION_FAILURE)) { + if(test.getTestStatus().equals(TestStatus.EXECUTION_FAILURE) || test.getTestStatus().equals(TestStatus.VERIFICATION_FAILURE) || + test.getTestStatus().equals(TestStatus.ORDERBY_FAILURE)) { document.set("errorMessage", test.getException().toString().replaceAll("\n","")); }else{ document.set("errorMessage", "N/A"); @@ -287,6 +290,7 @@ public int runTests() throws Exception { int totalVerificationFailure = 0; int totalCanceledTest = 0; int totalTimeoutFailure = 0; + int totalOrderbyFailure = 0; int i = 0; LOG.info("> TOOK " + stopwatch + " TO SETUP."); @@ -316,6 +320,7 @@ public int runTests() throws Exception { List executionFailures = Lists.newArrayList(); List timeoutFailures = Lists.newArrayList(); List canceledTests = Lists.newArrayList(); + List orderbyFailures = Lists.newArrayList(); for (DrillTest test : tests) { TestStatus testStatus = test.getTestStatus(); @@ -335,6 +340,9 @@ public int runTests() throws Exception { case CANCELED: canceledTests.add(test); break; + case ORDERBY_FAILURE: + orderbyFailures.add(test); + break; default: executionFailures.add(test); } @@ -361,6 +369,12 @@ public int runTests() throws Exception { LOG.info("Query: \n" + test.getQuery()); LOG.info(test.getException().getMessage()); } + LOG.info("Orderby Failures:"); + for (DrillTest test : orderbyFailures) { + LOG.info(test.getInputFile()); + LOG.info("Query: \n" + test.getQuery()); + LOG.info(test.getException().getMessage()); + } LOG.info("Timeout Failures:"); for (DrillTest test : timeoutFailures) { LOG.info(test.getInputFile()); @@ -377,14 +391,18 @@ public int runTests() throws Exception { for (DrillTest test : verificationFailures) { LOG.info(test.getInputFile()); } + LOG.info("Orderby Failures:"); + for (DrillTest test : orderbyFailures) { + LOG.info(test.getInputFile()); + } LOG.info("Timeout Failures:"); for (DrillTest test : timeoutFailures) { LOG.info(test.getInputFile()); } LOG.info(LINE_BREAK); - LOG.info(String.format("\nPassing tests: %d\nExecution Failures: %d\nVerificationFailures: %d" + - "\nTimeouts: %d\nCanceled: %d", passingTests.size(), executionFailures.size(), - verificationFailures.size(), timeoutFailures.size(), canceledTests.size())); + LOG.info(String.format("\nPassing tests: %d\nExecution Failures: %d\nVerification Failures: %d" + + "\nTimeouts: %d\nCanceled: %d\nOrderby Failures: %d", passingTests.size(), executionFailures.size(), + verificationFailures.size(), timeoutFailures.size(), canceledTests.size(), orderbyFailures.size())); if (OPTIONS.trackMemory) { LOG.info(LINE_BREAK); @@ -398,26 +416,33 @@ public int runTests() throws Exception { totalVerificationFailure += verificationFailures.size(); totalTimeoutFailure += timeoutFailures.size(); totalCanceledTest += canceledTests.size(); + totalOrderbyFailure += orderbyFailures.size(); } if (i > 2) { LOG.info(LINE_BREAK); - LOG.info(String.format("\nCompleted %d iterations.\n Passing tests: %d\n Execution Failures: %d\n VerificationFailures: %d" + - "\n Timeouts: %d\n Canceled: %d", i-1, totalPassingTest, totalExecutionFailure, - totalVerificationFailure, totalTimeoutFailure, totalCanceledTest)); + LOG.info(String.format("\nCompleted %d iterations.\n Passing tests: %d\n Execution Failures: %d\n Verification Failures: %d" + + "\n Timeouts: %d\n Canceled: %d\n Orderby Failures: %d", i-1, totalPassingTest, totalExecutionFailure, + totalVerificationFailure, totalTimeoutFailure, totalCanceledTest, totalOrderbyFailure)); LOG.info("\n> TEARING DOWN.."); } teardown(); executor.close(); connectionPool.close(); - return totalExecutionFailure + totalVerificationFailure + totalTimeoutFailure; + return totalExecutionFailure + totalVerificationFailure + totalTimeoutFailure + totalOrderbyFailure; } public void setup() throws IOException, InterruptedException { if (!new File(drillOutputDirName).exists()) { new File(drillOutputDirName).mkdir(); } + // Load orderby white-list file. These tests have orderby clauses + // that cannot be validated. Validation of the orderby clause will be + // skipped for now. + if (new File(orderbyWhitelistFile).exists()) { + orderbyWhitelist = Utils.readOrderbyWhitelistFile(orderbyWhitelistFile); + } String templatePath = CWD + "/conf/plugin-templates/"; LOG.info(templatePath); diff --git a/framework/src/main/java/org/apache/drill/test/framework/TestVerifier.java b/framework/src/main/java/org/apache/drill/test/framework/TestVerifier.java index 9e58068ca..340b89fc4 100755 --- a/framework/src/main/java/org/apache/drill/test/framework/TestVerifier.java +++ b/framework/src/main/java/org/apache/drill/test/framework/TestVerifier.java @@ -62,7 +62,7 @@ public class TestVerifier { public enum TestStatus { PENDING, RUNNING, PASS, EXECUTION_FAILURE, VERIFICATION_FAILURE, ORDER_MISMATCH, TIMEOUT, - CANCELED + CANCELED, ORDERBY_FAILURE }; public TestVerifier(List types, String query, List columnLabels, List verificationType) { @@ -70,6 +70,9 @@ public TestVerifier(List types, String query, List columnLabels this.query = query; this.columnLabels = columnLabels; this.verificationTypes = verificationType; + if (verificationType.get(0).equalsIgnoreCase("text")) { + this.checkType = false; + } } public TestVerifier() { @@ -90,7 +93,7 @@ public TestVerifier() { */ public TestStatus verifySqllineResult(String expectedOutput, String actualOutput, boolean verifyOrderBy) throws IOException, VerificationException, - IllegalAccessException { + IllegalAccessException, OrderbyException { String cleanedUpFile = cleanUpSqllineOutputFile(actualOutput); return verifyResultSet(expectedOutput, cleanedUpFile, verifyOrderBy); } @@ -128,7 +131,7 @@ private String cleanUpSqllineOutputFile(String actualOutput) throws IOException * @throws Exception */ public TestStatus verifyResultSet(String expectedOutput, String actualOutput) - throws IllegalAccessException, IOException, VerificationException { + throws IllegalAccessException, IOException, VerificationException, OrderbyException { return verifyResultSet(expectedOutput, actualOutput, false); } @@ -146,7 +149,7 @@ public TestStatus verifyResultSet(String expectedOutput, String actualOutput) * @throws Exception */ public TestStatus verifyResultSet(String expectedOutput, String actualOutput, boolean verifyOrderBy) - throws IOException, VerificationException, IllegalAccessException { + throws IOException, VerificationException, IllegalAccessException, OrderbyException { if (testStatus == TestStatus.EXECUTION_FAILURE || testStatus == TestStatus.CANCELED) { return testStatus; @@ -540,7 +543,7 @@ private static class IndexAndOrder { */ public TestStatus verifyResultSetOrders(String filename, List columnLabels, Map orderByColumns) - throws IOException, VerificationException, IllegalAccessException { + throws IOException, VerificationException, IllegalAccessException, OrderbyException { loadFromFileToMap(filename, true); List columnIndexAndOrder = getColumnIndexAndOrderList( columnLabels, orderByColumns, true); @@ -548,7 +551,7 @@ public TestStatus verifyResultSetOrders(String filename, // then skip this part of the verification. if (columnIndexAndOrder == null) { LOG.debug("skipping order verification"); - return TestStatus.PASS; + throw new OrderbyException("Order mismatch "); } if (!isOrdered(columnIndexAndOrder, orderByColumns)) { LOG.info("\nOrder mismatch in actual result set."); @@ -558,7 +561,7 @@ public TestStatus verifyResultSetOrders(String filename, } private Map getColumnIndexAndOrder( - List columnLabels, Map orderByColumns) { + List columnLabels, Map orderByColumns) throws OrderbyException { List result = getColumnIndexAndOrderList(columnLabels, orderByColumns, false); if (result == null) { @@ -586,7 +589,7 @@ private Map getColumnIndexAndOrder( */ private List getColumnIndexAndOrderList( List columnLabels, Map orderByColumns, - boolean checkForFields) { + boolean checkForFields) throws OrderbyException { List columnIndexAndOrder = new ArrayList(); List indicesOfOrderByColumns = getIndicesOfOrderByColumns( columnLabels, orderByColumns, checkForFields); @@ -603,7 +606,7 @@ private List getColumnIndexAndOrderList( } private List getIndicesOfOrderByColumns( - List columnLabels, Map orderByColumns) { + List columnLabels, Map orderByColumns) throws OrderbyException { return getIndicesOfOrderByColumns(columnLabels, orderByColumns, false); } @@ -621,7 +624,7 @@ private List getIndicesOfOrderByColumns( private List getIndicesOfOrderByColumns( List columnLabels, Map orderByColumns, - boolean checkForFields) { + boolean checkForFields) throws OrderbyException { List indices = new ArrayList(); for (Map.Entry entry : orderByColumns.entrySet()) { String entryKey = entry.getKey(); @@ -631,10 +634,10 @@ private List getIndicesOfOrderByColumns( } // verify that each order by column is present in the result set by // checking columnLabels, which represents the columns in the result set. - // if an order by column is not in the result set, return null. + // if an order by column is not in the result set, throw OrderbyException int index = columnLabels.indexOf(entryKey); if (index < 0) { - return null; + throw new OrderbyException ("Order by key " + entryKey + " is not projected."); } indices.add(index); } @@ -832,6 +835,23 @@ private static boolean matchAndCompareAll(String actual, String expected) { return true; } + public TestStatus verifyText(String expectedOutput, + String actualOutput) throws IOException, VerificationException { + if (testStatus == TestStatus.EXECUTION_FAILURE + || testStatus == TestStatus.CANCELED) { + return testStatus; + } + String expected = new String(Files.readAllBytes(Paths.get(expectedOutput))); + String actual = new String(Files.readAllBytes(Paths.get(actualOutput))); + if (!expected.equals(actual)) { + StringBuilder sb = new StringBuilder(); + sb.append ("\nExpected and Actual output are different.\n"); + throw new VerificationException(sb.toString()); + } else { + return TestStatus.PASS; + } + } + /** * Create a map containing the names of the columns in the order-by clause and * whether they are being ordered in ascending or descending order. @@ -915,4 +935,11 @@ public VerificationException(String message) { super(message); } } + + public static class OrderbyException extends Exception { + + public OrderbyException(String message) { + super(message); + } + } } diff --git a/framework/src/main/java/org/apache/drill/test/framework/Utils.java b/framework/src/main/java/org/apache/drill/test/framework/Utils.java index e8f3893cc..3c0b8e585 100755 --- a/framework/src/main/java/org/apache/drill/test/framework/Utils.java +++ b/framework/src/main/java/org/apache/drill/test/framework/Utils.java @@ -38,12 +38,14 @@ import java.sql.ResultSetMetaData; import java.util.ArrayList; import java.util.Date; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.PropertyResourceBundle; import java.util.ResourceBundle; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.Set; import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.databind.JsonNode; @@ -604,4 +606,27 @@ public static JsonNode getJsonValue(String jsonString, String key) return rootNode.at(ptrExpr); } + /** + * Load orderby tests from orderby white-list file + * These tests have orderby clauses which cannnot be validated and therefore need to be skipped. + * They will be passed for now until the orderby clause can be validated + */ + public static Set readOrderbyWhitelistFile(String whitelistFile) throws IOException { + BufferedReader reader = new BufferedReader(new FileReader(new File (whitelistFile))); + String line = null; + Set whitelist = new HashSet<>(); + while ((line = reader.readLine()) != null) { + whitelist.add(line); + } + return whitelist; + } + + /** + * Check if this test is white-listed. + * If so, this test will be passed for now until the orderby clause can be validated + */ + public static boolean isOrderbyWhitelist(String queryFile) { + return TestDriver.orderbyWhitelist.contains(queryFile); + } + }