> On Aug. 26, 2016, 2:01 a.m., Siddharth Seth wrote: > > Not a lot here related to the patch itself in this comment. > > Looks like everyone has gotten interested in fixing Hive tests at about the > > same time - and there's a good reason for that :) > > I think it would be better to get changes in via smaller patches. This one > > is at 180KB already (without touching q files). This is both to make it > > easier to review, and to have a smaller window for conflicts. There's > > several jiras open to fix either testing infrastructure, or individual > > tests, or get things working in the IDE. I know Zoltan was driving towards > > getting tests working via the IDE - I don't know how this affects that > > effort.
Hi Siddharth, With Zoltan, we have the same goals, and with his previous patch I was able to run query tests in IDE - in fact I made most of my tests from the IDE, and falled back to mvn only to validate that it is still working. So he made a good progress on that. On the review of one of his last patch, we agreed on jira comments (HIVE-14444), that he will commit his work, and later I will try to remove some of the technical debt from the testframework. This change is big alone, and I made the mistake to add some minor stuff there, where it made my life miserable during the testing: - Making the BeeLine test able to compile - Removing created UDF-s in the refresh method - Merged/added HIVE-14625 changes to the patch since it updated files I removed/changed I could remove those changes, if you think it would be easier to review, but I think the complexity more in the bigger part, which could not be splitted - touching every CliDriver classes, Adapters, QTestUtils. What I think is a good indicator is the results of the QA run. The patch run the same amount of tests as before, and resulted in the same errors as before. So I am quite positive that we have made progress to the good direction, but of course the decision is up to the reviewers. Thanks, Peter - Peter ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51397/#review146909 ----------------------------------------------------------- On Aug. 26, 2016, 2:38 a.m., Peter Vary wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51397/ > ----------------------------------------------------------- > > (Updated Aug. 26, 2016, 2:38 a.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 > db58f1d > > 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 4d4a929 > > itests/util/src/main/java/org/apache/hadoop/hive/ql/parse/CoreParseNegative.java > 8dba0bb > pom.xml 9ed1c19 > > 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 > >
