hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Zoltan Haindrich <k...@rxd.hu>
Subject Re: Review Request 50906: HIVE-14444 Upgrade qtest execution framework to junit4 - migrate most of them
Date Tue, 09 Aug 2016 10:15:18 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message