> On Aug. 9, 2016, 12:14 a.m., Peter Vary wrote: > > Hi Zoltan, > > > > Thanks for the patch, I can see, that you were working on it even on the > > weekend. > > > > Please help me to understand the components a little more, so I could help > > with the review. > > As I can see there are 3 levels of the classes for every given test: > > - Configuration > > - Adapter > > - Driver > > > > I have tried to identify the functionality of the given elements, and come > > up with the following: > > - Configuration - The queries to run, the configuration of the clusters, > > and the initial data > > - Adapter - The actual methods for implementing the test, like class, > > method level setup, and test execution > > - Driver - These contain very little code, and they look very simmilar, so > > a lot of code duplication is there - should not be a good idea to merge it > > with the Adapter class? Also it is a little strange, that the Configuration > > has to have a reference to the Adapter. If you decide to merge the Adapter > > and the Driver, then the reference is not needed anymore. > > > > Thanks, > > Peter
Hello, yeah...i wanted to take advantage of the "empty queue" on the ptest executor ;) by the way i think that all hive precommit jobs which end-up on ubuntu-3 will fail with some wierd jdk issue... I think you are getting it right...those classes which have "Driver" in there names are the successors of the old vm files: i don't want to touch them until we have all of them on board. There is some redundancy even between the Driver classes...CliDriver and some others are very similar - it will be easy to drop some of them. Merge with the adapter would possibly remove the common parent - and that would possibly break the factory adapterfactory. The positive side of the current design is that all configurations are in one place...even the core executor selection is in CliConfigs - so you have to look at just one place if you have to modify it. About more refactoring work: reviewboard can pick-up changes in renamed files (which is great) - but if I add more refactors to this patch: it will look like a "20 files remove", "30 files added" - which is not really review friendly ;) it have already lost track of the changes of PerfCliDriver and QTestGenTask. I would like to continue this with a cleanup refactor; after AccumuloCli and BeelineCli is on-board. regards, Zoltan - Zoltan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50906/#review145152 ----------------------------------------------------------- On Aug. 8, 2016, 8:47 p.m., Zoltan Haindrich wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50906/ > ----------------------------------------------------------- > > (Updated Aug. 8, 2016, 8:47 p.m.) > > > Review request for hive. > > > Bugs: HIVE-14444 > https://issues.apache.org/jira/browse/HIVE-14444 > > > Repository: hive-git > > > Description > ------- > > this patch focuses on removing the qtestgen task without introducing > regressions in existing testing services - its not the nicest patch...but > opens the possiblities to continue refactoring of these classes inside a pure > java environment > > this patch contains a bunch of helper classes to provide Tests for the junit4 > executor. > > I've mimiced the old generated testcases behaviour: > > * every test has a config entry in CliConfigs > * every config has a Core executor - these can be looked as the older vm files > * every test has an instance in the appropriate module - this class is > small...copy pasted parameterized junit4 test > > > Diffs > ----- > > ant/src/org/apache/hadoop/hive/ant/QTestGenTask.java > f372d7cb937d91c10d3dff0e4c51e0849c5e3c9b > ant/src/org/apache/hadoop/hive/ant/antlib.xml > 8f663482348448be9aadcf535c42a8c11d8955b1 > hbase-handler/src/test/templates/TestHBaseCliDriver.vm > f513f0374b1e6798677e98b4d071d5969d204bfa > hbase-handler/src/test/templates/TestHBaseNegativeCliDriver.vm > 043bd87a4f617de7fff89e38e654770cd9b84d8f > itests/qtest-accumulo/pom.xml 339c59919295b66c9d3c64a53ea78bd944e3a903 > itests/qtest-spark/pom.xml 3bc9e24893a42bfcaab66d4cb8930dd5586c1db5 > > itests/qtest-spark/src/test/java/org/apache/hadoop/hive/cli/TestMiniSparkOnYarnCliDriver.java > PRE-CREATION > > itests/qtest-spark/src/test/java/org/apache/hadoop/hive/cli/TestSparkCliDriver.java > PRE-CREATION > > itests/qtest-spark/src/test/java/org/apache/hadoop/hive/cli/TestSparkNegativeCliDriver.java > PRE-CREATION > itests/qtest/pom.xml 17968e69559a16a1971f6028ea3073ab693a6678 > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/ContribNegativeCliDriver.java > PRE-CREATION > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/DisabledTestBeeLineDriver.java > PRE-CREATION > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/DummyCliDriver.java > PRE-CREATION > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestCliDriver.java > PRE-CREATION > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestCompareCliDriver.java > PRE-CREATION > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestContribCliDriver.java > PRE-CREATION > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestContribNegativeCliDriver.java > PRE-CREATION > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestEncryptedHDFSCliDriver.java > PRE-CREATION > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestHBaseCliDriver.java > PRE-CREATION > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestHBaseMinimrCliDriver.java > PRE-CREATION > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestHBaseNegativeCliDriver.java > PRE-CREATION > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestMiniLlapCliDriver.java > PRE-CREATION > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestMiniTezCliDriver.java > PRE-CREATION > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestMinimrCliDriver.java > PRE-CREATION > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestNegativeCliDriver.java > PRE-CREATION > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestNegativeMinimrCliDriver.java > PRE-CREATION > > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestPerfCliDriver.java > PRE-CREATION > > itests/qtest/src/test/java/org/apache/hadoop/hive/ql/parse/TestParseNegativeDriver.java > PRE-CREATION > > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/AbstractCliConfig.java > PRE-CREATION > > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliAdapter.java > PRE-CREATION > > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliConfigs.java > PRE-CREATION > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreDummy.java > PRE-CREATION > > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CorePerfCliDriver.java > PRE-CREATION > itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java > b3cf6da56eddc5000515e5ebd34989e64b1c0010 > pom.xml 0217edea03b71522629ec30514b67a4a5313edb6 > ql/src/test/templates/TestBeeLineDriver.vm > 563d7fd03600d391b5694cc8477562d9f2e5c9d9 > ql/src/test/templates/TestCliDriver.vm > 0ccedce7361fa87a72f369db90cd9c57dc8b26e2 > ql/src/test/templates/TestCompareCliDriver.vm > 8d4e96437c19177561f1d83ac4f22c16e98bfa93 > ql/src/test/templates/TestNegativeCliDriver.vm > 592d64f80b74ff76e897e2ae07f8635e46675932 > ql/src/test/templates/TestParseNegative.vm > 9500eced31d64e5029d9b2be429aaa6828252f11 > ql/src/test/templates/TestPerfCliDriver.vm > d2946cb4f9a7e3e9f8b3cc14a75802a483e7f95a > > Diff: https://reviews.apache.org/r/50906/diff/ > > > Testing > ------- > > some manual tests for qfile, initScript ... they should work as before > > ptests's use of minimr.test.files and such have caught me by surprise...but > those are also supported (i think) > > > Thanks, > > Zoltan Haindrich > >
