hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Peter Vary <pv...@cloudera.com>
Subject Re: Review Request 51397: HIVE-14536 Unit test code cleanup
Date Fri, 26 Aug 2016 02:37:56 GMT


> On Aug. 25, 2016, 10:46 p.m., Zoltan Haindrich wrote:
> > This patch is huge...it was very time consuming doing its review - it would have
been much better to split some of it into smaller changes

Hi Zoltan,

I know it is a big patch, and I appreciate your review very much, however I did not see the
possibility to refactor this in smaller sets.

Thanks again, see my responses below,
Peter


> On Aug. 25, 2016, 10:46 p.m., Zoltan Haindrich wrote:
> > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestBeeLineDriver.java, lines
37-39
> > <https://reviews.apache.org/r/51397/diff/1/?file=1485268#file1485268line37>
> >
> >     this is not seems like a junit test 
> >     
> >     can you add the annotations + @Ignore("will be enabled soon ;)")

Thanks - I did this


> On Aug. 25, 2016, 10:46 p.m., Zoltan Haindrich wrote:
> > itests/qtest-accumulo/src/test/java/org/apache/hadoop/hive/cli/TestAccumuloCliDriver.java,
lines 45-52
> > <https://reviews.apache.org/r/51397/diff/1/?file=1485260#file1485260line45>
> >
> >     i think the previous one was more descriptive about what is really happening...by
that dsl-alike style ; because the review board was unable to match those lines i copy one
here:
> >     
> >     {code}
> >             setQueryDir("ql/src/test/queries/clientpositive");
> >     
> >             includesFrom(testConfigProps, "minimr.query.files");
> >     
> >             setResultsDir("ql/src/test/results/clientpositive");
> >             setLogDir("itests/qtest/target/qfile-results/clientpositive");
> >     
> >             setInitScript("q_test_init.sql");
> >             setCleanupScript("q_test_cleanup.sql");
> >     
> >             setHiveConfDir("");
> >             setClusterType(MiniClusterType.mr);
> >     {code}
> >     
> >     i think this is better than having a constructor with 9 arguments..

I removed the possibility of the configuration changes intentionally. This left me with this
version (no setters, just getters).
I think it would be more readable this way:

new CliConfig(
        "accumulo-handler/src/test/queries/positive",
        "accumulo-handler/src/test/results/positive",
        "itests/qtest/target/qfile-results/accumulo-handler/positive",
        null,
        QTestUtil.MiniClusterType.none,
        "",
        "q_test_init.sql",
        "q_test_cleanup.sql",
        true
);

But this does not conform the coding guidelines.

It would be even more descriptive with a bulder like code:
CliConfigBuilder.setQueryDir().setResultsDir()....build();

But I decided against it, because I do not think it is worth the effort.


> On Aug. 25, 2016, 10:46 p.m., Zoltan Haindrich wrote:
> > itests/qtest/src/test/java/org/apache/hadoop/hive/cli/TestCompareCliDriver.java,
line 82
> > <https://reviews.apache.org/r/51397/diff/1/?file=1485270#file1485270line82>
> >
> >     i think it would be better to move out "things" from QTestUtil instead of adding
more...

That was the point, where I stopped my cleanup work, and decided to put that into another
patch.
I agree with you, that the QTestUtil requires a cleanup too.

My view on the problem is, that the test cases are using simmilar code very often, which we
do not want to duplicate. But, we do not want to use inheritance in testcases, which migth
cause problems to us when trying to run them in parallel, so a these functions should be refactored
to a helper class, like QTestUtil.

I think we should revisit this issue again when we clean up the QTestUtil classes.

What do you think?


> On Aug. 25, 2016, 10:46 p.m., Zoltan Haindrich wrote:
> > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliConfig.java, line
42
> > <https://reviews.apache.org/r/51397/diff/1/?file=1485288#file1485288line42>
> >
> >     the earlier abstractcliconfig class was immutable...and it needed a real descendtant
to configure it properly...
> >     
> >     but it served a purpose: it prevented any furter changes once it had been constructed
- and would force anyone wanting to alter the configuration to look for the right place...I've
seen value in that approach...but I might be wrong of course...

I think the configuration is one of the most important part of the Test class - when there
is a problem, it is most natural to look for it there. That is why I put it there.

The only public setters are for the CliConfig class are the excludeQuerySet, includeQuerySet,
etc. which I think were public on the abstractcliconfig too.

I might be missing your point here. Could you help please?


> On Aug. 25, 2016, 10:46 p.m., Zoltan Haindrich wrote:
> > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliConfig.java, line
70
> > <https://reviews.apache.org/r/51397/diff/1/?file=1485288#file1485288line70>
> >
> >     i'm afraid this is a bit misleading: AFAIK it has nothing to do with the used
hadoop...there are only hadoop version dependent excludes and some file naming tricks this
thing may twist

Thanks, changed the comment, could you please review?


> On Aug. 25, 2016, 10:46 p.m., Zoltan Haindrich wrote:
> > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliConfig.java, lines
90-97
> > <https://reviews.apache.org/r/51397/diff/1/?file=1485288#file1485288line90>
> >
> >     i think no one will really use these new properties...and that last resort default
is totally wrong; an exception would be more appropriate - it will not be usable from maven
because i think these are not passed in pom.xml

The properties do not have to be in pom.xml, if someone specifies them in command line with
-D argument, there will be set.

So if someone wants to regenerate the query results without overwriting the original versions,
-DresultsDirectory=/tmp -Dtest.output.overwrite=true could be used, or if there is a new set
of tests in a directory, then it could be used as well, without changing the code.

So I think they have a merit, and the they do not cause further problems.


> On Aug. 25, 2016, 10:46 p.m., Zoltan Haindrich wrote:
> > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliConfig.java, lines
101-106
> > <https://reviews.apache.org/r/51397/diff/1/?file=1485288#file1485288line101>
> >
> >     the result of this block is either the passed metastoreType argument ; or sql,
but the property value is never get used..
> >     
> >     but either way i think no one will use the property

Maybe I made an error but I think this code will set the metastoreType from the property,
if it is set:

    String metastoreTypeString = System.getProperty("metastoreType");
    if (metastoreTypeString != null && metastoreTypeString.trim().length() !=0) {
      System.err.println("Property: metastoreType used as override with val: "
                             + metastoreTypeString);
      this.metastoreType = MetastoreType.valueOf(metastoreTypeString);

This property could be very usefull, if we want to test the HBase metastore with other testcases
too.

Reopen please, if I made a mistake here, and missread the code above.

Thanks!


> On Aug. 25, 2016, 10:46 p.m., Zoltan Haindrich wrote:
> > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliConfig.java, line
111
> > <https://reviews.apache.org/r/51397/diff/1/?file=1485288#file1485288line111>
> >
> >     I think this valueForString method is bad...and I wanted to get rid of it...as
it does has a fallback when you do a typo...which will do cause unexpected results.
> >     
> >     you have removed my FIXME about that issue...i've only kept it before; because
one of the values was parsed from a different string by this that valueForString...and I didn't
wanted to risk breaking someone elses setup with that patch

Thanks for catching this - I remember changing to valueOf and removing the comment, but later
I put it back, since I think we want backward compatiblity and with valueOf the string literals
whould be changed. But I forget to put back the FIXME.

Added the FIXME, thanks


> On Aug. 25, 2016, 10:46 p.m., Zoltan Haindrich wrote:
> > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliConfig.java, lines
118-122
> > <https://reviews.apache.org/r/51397/diff/1/?file=1485288#file1485288line118>
> >
> >     these defaults are not really usefull...i think an exception would be more appropriate
in case of initScript of cleanupScript is missing

Default:
- hiveConfDir - it is currently used
- initScript - might not need to run any init script for several tests - if it resolves to
a dir ("" means hiveRootDir), then no script will be run - It could be usefull, to run a specific
test where no init tables are needed, since the initialization itself takes too long
- cleanupScript - same as initScript
- hadoopVersion - you get me here, I am not really comfortable with that one, but it should
not cause any problems.


> On Aug. 25, 2016, 10:46 p.m., Zoltan Haindrich wrote:
> > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliConfig.java, line
138
> > <https://reviews.apache.org/r/51397/diff/1/?file=1485288#file1485288line138>
> >
> >     try-with-resources

I am confortable with throwing an exception in test initialization, if there is no testconfiguration.properties,
or there is any error. Since the only thing I could do is catch the exception and throw it
again. I do not think converting it to an assertion failure is any better.


> On Aug. 25, 2016, 10:46 p.m., Zoltan Haindrich wrote:
> > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliConfig.java, line
260
> > <https://reviews.apache.org/r/51397/diff/1/?file=1485288#file1485288line260>
> >
> >     i think it would be better to split this into 2 methods:
> >     above this line is getQueryFiles
> >     belove this is getTestParameters

You are right, that these two are distinct functions, but I think we do not want to expose
or reuse getQueryFiles anyway, and a 5 line method does not really helps the readibility -
added comment to help to understand the function 

If you still think we should split them, feel free to open an issue for it


> On Aug. 25, 2016, 10:46 p.m., Zoltan Haindrich wrote:
> > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliConfig.java, line
275
> > <https://reviews.apache.org/r/51397/diff/1/?file=1485288#file1485288line275>
> >
> >     i think the earlier small notice about that a property override have happend
was usefull in the test outputs

It will be printed here, in the method where the override happens:

  private String getProperty(String systemPropertyKey, String constructorValue, String defaultValue)
{


> On Aug. 25, 2016, 10:46 p.m., Zoltan Haindrich wrote:
> > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliConfig.java, lines
312-313
> > <https://reviews.apache.org/r/51397/diff/1/?file=1485288#file1485288line312>
> >
> >     i dont think these various properties will be used by anyone...this one's name
is an enum value ;)
> >     
> >     anyway i think anyone can already override what he needs by using qfile/qfile_regex
...

I have found some of them in another file:

hive/testutils/ptest2/conf/example-apache-trunk.properties

Plus it is loaded by several pom.xml-s:

      <plugin>
        <groupId>org.codehaus.mojo</groupId>
        <artifactId>properties-maven-plugin</artifactId>
        <version>1.0-alpha-2</version>
        <executions>
          <execution>
            <phase>initialize</phase>
            <goals>
              <goal>read-project-properties</goal>
            </goals>
            <configuration>
              <files>
                <file>${basedir}/../src/test/resources/testconfiguration.properties</file>
              </files>
            </configuration>
          </execution>
        </executions>
      </plugin>
      
So I decided to keep them as it is.


> On Aug. 25, 2016, 10:46 p.m., Zoltan Haindrich wrote:
> > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliConfig.java, line
348
> > <https://reviews.apache.org/r/51397/diff/1/?file=1485288#file1485288line348>
> >
> >     previously we didn't needed a confusing argument like this includeAll...
> >     
> >     i think it would be better to force the user to supply excludes prior to includes
or someething...what could even

The includeAll is not set by the user.
It is to resolve your previous FIXME comment:

  // FIXME: null value is treated differently on the other end..when those filter will be
  // moved...this may change
  
This means, the user used at least one inclusive query list (includeQuerySet, includeQuery),
then this will be set to true, and only the selected files will be used, not the every file
from the queryDirectory.

The comment is improved - hopefully :)


> On Aug. 25, 2016, 10:46 p.m., Zoltan Haindrich wrote:
> > itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java, lines 219-237
> > <https://reviews.apache.org/r/51397/diff/1/?file=1485300#file1485300line219>
> >
> >     i failed to find any earlier version of this feature...it looks like a new feature
which will drop functions created during testing...
> >     
> >     anyway...i think this property to control which one to protect should be left
out - I think no one will use it.
> >     
> >     dropping any unknown function is a good idea!

There is a function created by the q_test_init.sql script:

CREATE FUNCTION qtest_get_java_boolean AS 'org.apache.hadoop.hive.ql.udf.generic.GenericUDFTestGetJavaBoolean';

I handled it the same way, as the srcTables.
We should keep in mind this, when we refactor the QTestUtil classes


> On Aug. 25, 2016, 10:46 p.m., Zoltan Haindrich wrote:
> > itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java, lines 2269-2283
> > <https://reviews.apache.org/r/51397/diff/1/?file=1485300#file1485300line2269>
> >
> >     i think all these sysouts should be exceptions (as before)...because when this
happens: the lookup strategy failed -> but that possibly also mean that the whole classpath
is crap...or this method is broken...but either way I don't think we may continue without
throwing an exception - I think avoiding exceptions like this will just raise debugging time.
> >     
> >     why have you removed the exceptions?

In java 7 we could not use lamda-s, and I call this method to calculate the default value
for the hadoopVersion property. See:

 this.hadoopVersion = getProperty("hadoop.version", null,
        QTestUtil.calculateHadoopVersionFromJar());
        
It is not acceptable here to throw an exception - most probably the version calculation would
not fall back to the default value.

I think it might be possible to run several tests without a hadoop jar on the classpath, which
means this calculation will fail.

If you think after these reasoning you still prefer to throw exceptions, feel free to reopen
the issue - I will think this through too tonight :)


> On Aug. 25, 2016, 10:46 p.m., Zoltan Haindrich wrote:
> > itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java, line 2283
> > <https://reviews.apache.org/r/51397/diff/1/?file=1485300#file1485300line2283>
> >
> >     i think this catch block in its current form is just some unreachable code

Yeah.. you are right - I was too eager to catch everything :)
Removed


> On Aug. 25, 2016, 10:46 p.m., Zoltan Haindrich wrote:
> > itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java, line 2263
> > <https://reviews.apache.org/r/51397/diff/1/?file=1485300#file1485300line2263>
> >
> >     i think the word calculate would involve some artithmetics (just joking ;) 
> >     I think a getX method may do anything as long it can provide X ;)
> >     
> >     anyway...this method is superior compared to the config level static setting
- and that could be possibly removed..
> >     but AFAIK hadoop-2 is the only one supported now, so this whole thing might
get removed later

renamed to getHadoopVersionFromJar


On Aug. 25, 2016, 10:46 p.m., Peter Vary wrote:
> > I think it would have been better to do this a bit later...because the previous
classes (even thru had their own problems) - have aided the refactoring of these things; after
this it would be trickier to extract those junit rules from qtestutil and its descendants...
> > 
> > There are quite a few changes beyond the refactor:
> > 
> > * pom.xml dependency change because of compilation of the beeline test
> > * some new udf protection mechanism
> > * merge of negative check with positive
> > * lots of new system properties
> > 
> > not that i'm against these changes, but looking at them separatly in a review friendly
context is much easier, faster...and may produce better feedbacks
> > because of the x files removed y files added nature of this patch - today I had
to look at files from git to see what have changed...
> > 
> > I think these various new properties will never be used...so we should avoid introducing
them
> > I think we should concentrate more on getting rid of the various properties and
keep only the useful ones; the massive property usage is one of the main reasons why executing
tests inside an IDE is hard.
> > 
> > I'm still not entirely convinced that removing the template alike nature of these
tests is the best way to go...i'm afraid that these separate tests will diverge more and more
after a while and there will be fixes which other tests may not get - this have been already
present inside the vm files prior to these changes: accumulocli for example does a full qtest
reinit after every case executed hbasenegativecli have missed a lot of improvments hbasecli
already had ; a single line TestCli change in the earlier one - new equivalent will be to
do copy-pasting it to ~10 classes...or just add it to one...and diverge
> > 
> > cheers.kirk

I agree with you, and I see my mistake in adding more changes than absolutely needed to this
refactor.

My reasoning was:
- Beeline - if there is a test, then it should be able to run, and TestBeeLineDriver was there
after your patch.
- UDF - this was clearly a bug, which made me hard to test single tests - the udf was created,
but not deleted, so the next time I had to clear everything before the test
- Merge negative, positive test runs - these were code duplication, and adding 2, 40 line
length, simmilar methods to a class with this minute differences seemed like a bad idea to
me
- IMHO these system properties does not increase the complexicity of the system too much.
They are only the mirrors of existing properties, and only adding the possibility of overriding
them in one single place. After overriding is done we can forget them, and use the calculated
values as before.

I very much appreciate, that you review my code even though these difficulties!

I agree with you that the unused properties has to be removed, but I do not know which ones
are used by the community members, and which ones are not. As for the new ones, I am sure
I would like use most of them - maybe not the hadoop.version, but all the others. Moreover
the new properties do not have to be set to run the tests - I have run them during my testing
from the IDE, and did not cause any problem for me. If you have any problem feel free to share,
so we can address them.

About the hadoop.version property - what does it supposed to do? As in your previous patch
review we discussed that this might be overriden, in case it is needed.
What happens with the current code:
- If it is specified with a -D parameter, then that value is used
- Else the pom.xml ${hadoop.version} is used
- Else ... I think there is not really an else here - but then comes the getHadoopVersionFromJar
Do you think, the pom.xml hadoop.version is ok? Then we could remove getHadoopVersionFromJar,
and every other stuff too. Is there a usecase to overwrite this? Should it be overwritten
separately, than the pom.xml-s hadoop.version?
Any help is appreciated here.
Thanks!

The template like nature does not prevent the divergence of the tests - See: HIVE-14625 -
The logging is added only for the CoreCliDriver, not the other ones. We should help the test
developers by reviewing the changes, and with clear guidance in the development docs. If the
testcase change is a common one, it should be done in the QTestUtil classes, which will effect
every test case.

As for HIVE-14625, I had to incorporate those changes to my patch as well, since it modified
files that are deleted by my patch. I added the logging to the QTestUtils, so it will be done
for every test - I think this is the way we should go.

Thanks for your helpful comments, and review!

Peter


- Peter


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51397/#review146818
-----------------------------------------------------------


On Aug. 25, 2016, 11:24 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, 11:24 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

>   pom.xml b05a2dc 
> 
> 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
> 
>


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