----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51397/#review146857 -----------------------------------------------------------
1 major & 1 minor comments. Otherwise, LGTM. Great work, thanks a lot. itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestBeeLineDriver.java (line 39) <https://reviews.apache.org/r/51397/#comment213618> Might worth to have the same explanation as you've had in the commit comment. itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliConfig.java (line 248) <https://reviews.apache.org/r/51397/#comment213642> The spec of runDisabled says to include ".q.disabled" AS WELL. By implementation it seems it will include ONLY ".q.diabled" files. - Gabor Szadovszky On Aug. 25, 2016, 2:50 p.m., Peter Vary wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51397/ > ----------------------------------------------------------- > > (Updated Aug. 25, 2016, 2:50 p.m.) > > > Review request for hive, Ashutosh Chauhan, Gabor Szadovszky, Zoltan > Haindrich, Marta Kuczora, Miklos Csanady, Prasanth_J, Sergey Shelukhin, > Sergio Pena, Siddharth Seth, and Barna Zsombor Klara. > > > Bugs: HIVE-14536 > https://issues.apache.org/jira/browse/HIVE-14536 > > > Repository: hive-git > > > Description > ------- > > Cleaning up the CliDrivers with the following requirements: > - If there is a problem with a specific testcase, it should be trivial to > find the corresponding methods that had been running > - Later it should be possible to run the testcases parallel > - No test result changes in this patch, so validation should be easier > - The QTestUtil classes not refactored - only added functionality which > belongs there - later could be cleaned up as well > > The selected "architecture" > - CliConfig class to store the configurations > - Testcases without inheritance - every beforeclass, before, after, > afterclass should be in this same file > - Repeating codes refactored to the QTestUtil classes > > Beeline driver - created, compiling, but removed the test annotations since > none of the test output files are valid even with the current version - later > should be cleaned up > Accumulo driver - created, compiling, 3 of the tests are ok, another 3 tests > was failing before. Currently this version does the same - later should be > cleaned up > > Open for any suggestions, feel free to criticize! > > > Diffs > ----- > > > itests/qtest-accumulo/src/test/java/org/apache/hadoop/hive/cli/TestAccumuloCliDriver.java > bf50f16 > > itests/qtest-spark/src/test/java/org/apache/hadoop/hive/cli/TestMiniSparkOnYarnCliDriver.java > e84bfce > > itests/qtest-spark/src/test/java/org/apache/hadoop/hive/cli/TestSparkCliDriver.java > 2c8cbee > > itests/qtest-spark/src/test/java/org/apache/hadoop/hive/cli/TestSparkNegativeCliDriver.java > 2db83f4 > itests/qtest/pom.xml ed44bb8 > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/ContribNegativeCliDriver.java > 253cda3 > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/DisabledTestBeeLineDriver.java > cb276e6 > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/DummyCliDriver.java > 965d1dc > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestBeeLineDriver.java > PRE-CREATION > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestCliDriver.java > c4c4f41 > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestCompareCliDriver.java > 944cd32 > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestContribCliDriver.java > 54596f9 > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestContribNegativeCliDriver.java > 1b39ee7 > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestEncryptedHDFSCliDriver.java > 8c6807e > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestHBaseCliDriver.java > 7b6f76a > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestHBaseMinimrCliDriver.java > 934af16 > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestHBaseNegativeCliDriver.java > 88d626c > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestMiniLlapCliDriver.java > ad525fe > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestMiniTezCliDriver.java > c23b0b3 > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestMinimrCliDriver.java > 96a9e8f > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestNegativeCliDriver.java > 1040228 > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestNegativeMinimrCliDriver.java > f7e2caa > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestPerfCliDriver.java > 4df4eeb > > itests/qtest/src/test/java/org/apache/hadoop/hive/ql/parse/TestParseNegativeDriver.java > 4c1224f > > itests/util/src/main/java/org/apache/hadoop/hive/accumulo/AccumuloQTestUtil.java > 88bc0bc > > itests/util/src/main/java/org/apache/hadoop/hive/accumulo/AccumuloTestSetup.java > 73d5f15 > > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/AbstractCliConfig.java > efbd465 > > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliAdapter.java > b89d6e7 > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliConfig.java > PRE-CREATION > > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliConfigs.java > 319a205 > > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreAccumuloCliDriver.java > a5d2711 > > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreBeeLineDriver.java > e5144e3 > > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreCliDriver.java > 5435f9f > > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreCompareCliDriver.java > 71a02bc > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreDummy.java > b7afb48 > > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreHBaseCliDriver.java > 956a42d > > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreHBaseNegativeCliDriver.java > 6225180 > > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreNegativeCliDriver.java > 65b2ce7 > > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CorePerfCliDriver.java > 8620cde > itests/util/src/main/java/org/apache/hadoop/hive/hbase/HBaseQTestUtil.java > 01faaba > itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java 358ba51 > > itests/util/src/main/java/org/apache/hadoop/hive/ql/parse/CoreParseNegative.java > 8dba0bb > > Diff: https://reviews.apache.org/r/51397/diff/ > > > Testing > ------- > > Run the test cases on a single machine. > At least 20 for ever Driver (at least 10 miniutes each). > The results were the same as for the runs without the patch. > Checked the number of the selected queryfiles, and it is matching with the > current number > Run the testcases from intellij, there were some problems (missing > TEST_HADOOP_CLASSPATH), but most of the testcases/queries are ok. > Waiting for the QA, to validate the test results and I will update the patch > if needed > > > Thanks, > > Peter Vary > >
