Skip to content

Commit

Permalink
Added a new failure type, ORDERBY_FAILURE, which occurs if a SQL stat…
Browse files Browse the repository at this point in the history
…ement 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.
  • Loading branch information
Robert Hou committed Feb 13, 2017
1 parent 3549fae commit 1d3c515
Show file tree
Hide file tree
Showing 6 changed files with 123 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
}

Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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:
Expand All @@ -100,6 +103,8 @@ public void run() {
break;
case 5:
setTestStatus(TestStatus.CANCELED);
case 6:
setTestStatus(TestStatus.ORDERBY_FAILURE);
default:
setTestStatus(TestStatus.EXECUTION_FAILURE);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ public void run() {
break;
case 5:
setTestStatus(TestStatus.CANCELED);
case 6:
setTestStatus(TestStatus.ORDERBY_FAILURE);
default:
setTestStatus(TestStatus.EXECUTION_FAILURE);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> orderbyWhitelist = null;

private static Configuration conf = new Configuration();
public static final Options OPTIONS = new Options();
Expand Down Expand Up @@ -207,7 +209,8 @@ public void generateReports(List<DrillTest> 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");
Expand Down Expand Up @@ -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.");

Expand Down Expand Up @@ -316,6 +320,7 @@ public int runTests() throws Exception {
List<DrillTest> executionFailures = Lists.newArrayList();
List<DrillTest> timeoutFailures = Lists.newArrayList();
List<DrillTest> canceledTests = Lists.newArrayList();
List<DrillTest> orderbyFailures = Lists.newArrayList();

for (DrillTest test : tests) {
TestStatus testStatus = test.getTestStatus();
Expand All @@ -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);
}
Expand All @@ -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());
Expand All @@ -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);
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,17 @@ public class TestVerifier {

public enum TestStatus {
PENDING, RUNNING, PASS, EXECUTION_FAILURE, VERIFICATION_FAILURE, ORDER_MISMATCH, TIMEOUT,
CANCELED
CANCELED, ORDERBY_FAILURE
};

public TestVerifier(List<Integer> types, String query, List<String> columnLabels, List<String> verificationType) {
this.types = types;
this.query = query;
this.columnLabels = columnLabels;
this.verificationTypes = verificationType;
if (verificationType.get(0).equalsIgnoreCase("text")) {
this.checkType = false;
}
}

public TestVerifier() {
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
}

Expand All @@ -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;
Expand Down Expand Up @@ -540,15 +543,15 @@ private static class IndexAndOrder {
*/
public TestStatus verifyResultSetOrders(String filename,
List<String> columnLabels, Map<String, String> orderByColumns)
throws IOException, VerificationException, IllegalAccessException {
throws IOException, VerificationException, IllegalAccessException, OrderbyException {
loadFromFileToMap(filename, true);
List<IndexAndOrder> columnIndexAndOrder = getColumnIndexAndOrderList(
columnLabels, orderByColumns, true);
// if one or more order-by columns is not present in the result set,
// 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.");
Expand All @@ -558,7 +561,7 @@ public TestStatus verifyResultSetOrders(String filename,
}

private Map<Integer, String> getColumnIndexAndOrder(
List<String> columnLabels, Map<String, String> orderByColumns) {
List<String> columnLabels, Map<String, String> orderByColumns) throws OrderbyException {
List<IndexAndOrder> result = getColumnIndexAndOrderList(columnLabels,
orderByColumns, false);
if (result == null) {
Expand Down Expand Up @@ -586,7 +589,7 @@ private Map<Integer, String> getColumnIndexAndOrder(
*/
private List<IndexAndOrder> getColumnIndexAndOrderList(
List<String> columnLabels, Map<String, String> orderByColumns,
boolean checkForFields) {
boolean checkForFields) throws OrderbyException {
List<IndexAndOrder> columnIndexAndOrder = new ArrayList<IndexAndOrder>();
List<Integer> indicesOfOrderByColumns = getIndicesOfOrderByColumns(
columnLabels, orderByColumns, checkForFields);
Expand All @@ -603,7 +606,7 @@ private List<IndexAndOrder> getColumnIndexAndOrderList(
}

private List<Integer> getIndicesOfOrderByColumns(
List<String> columnLabels, Map<String, String> orderByColumns) {
List<String> columnLabels, Map<String, String> orderByColumns) throws OrderbyException {
return getIndicesOfOrderByColumns(columnLabels, orderByColumns, false);
}

Expand All @@ -621,7 +624,7 @@ private List<Integer> getIndicesOfOrderByColumns(
private List<Integer> getIndicesOfOrderByColumns(
List<String> columnLabels,
Map<String, String> orderByColumns,
boolean checkForFields) {
boolean checkForFields) throws OrderbyException {
List<Integer> indices = new ArrayList<Integer>();
for (Map.Entry<String, String> entry : orderByColumns.entrySet()) {
String entryKey = entry.getKey();
Expand All @@ -631,10 +634,10 @@ private List<Integer> 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);
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -915,4 +935,11 @@ public VerificationException(String message) {
super(message);
}
}

public static class OrderbyException extends Exception {

public OrderbyException(String message) {
super(message);
}
}
}
25 changes: 25 additions & 0 deletions framework/src/main/java/org/apache/drill/test/framework/Utils.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String> readOrderbyWhitelistFile(String whitelistFile) throws IOException {
BufferedReader reader = new BufferedReader(new FileReader(new File (whitelistFile)));
String line = null;
Set<String> 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);
}

}

0 comments on commit 1d3c515

Please sign in to comment.