> On July 9, 2013, 6:14 p.m., Jarek Cecho wrote:
> > test/src/main/java/org/apache/sqoop/test/hadoop/HadoopStandaloneCluster.java,
> >  line 23
> > <https://reviews.apache.org/r/10964/diff/4/?file=318881#file318881line23>
> >
> >     Nit: I'm afraid that the word "cluster" in the name can be quite 
> > confusing as this is not using any cluster at all. Would it be possible to 
> > rename it to something like LocalMode or LocalRunner?

I changed every thing to be suffixed with the word "Runner". I feel as though 
this overlaps with MapReduce nomenclature. Any thoughts?


> On July 9, 2013, 6:14 p.m., Jarek Cecho wrote:
> > test/src/main/java/org/apache/sqoop/test/hadoop/HadoopMiniCluster.java, 
> > lines 50-51
> > <https://reviews.apache.org/r/10964/diff/4/?file=318880#file318880line50>
> >
> >     The getDataDir seems to be overloaded. Here it's used for local files 
> > that are generated by the MiniClusters, however we are also using it for 
> > the HDFS files. I would recommend to distinguish those two usages.

Added 'getTestDirectory' to symbolically discern between data directory and 
test directory.


> On July 9, 2013, 6:14 p.m., Jarek Cecho wrote:
> > test/src/main/java/org/apache/sqoop/test/testcases/TomcatTestCase.java, 
> > lines 62-75
> > <https://reviews.apache.org/r/10964/diff/4/?file=318883#file318883line62>
> >
> >     I think that the main temporary path should be owned by this class 
> > because we are storing there a lot of information required for the Sqoop 
> > Server itself (logs, metastore, configuration, ....). Those files exists 
> > outside of the mapreduce/hdfs.

Doesn't it make more sense for the sqoop minicluster classes to manage their 
own temporary path? It seems like we should only provide a base directory.


> On July 9, 2013, 6:14 p.m., Jarek Cecho wrote:
> > test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java, 
> > lines 52-54
> > <https://reviews.apache.org/r/10964/diff/4/?file=318882#file318882line52>
> >
> >     I think that we should be able to put the HadoopCluster initialization 
> > into method annotated with @BeforeClass and override this method in child 
> > if necessary. This way we should save some time bootstrapping the 
> > Minicluster for test cases that have more then one test method.

Works!


- Abraham


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


On July 8, 2013, 8:47 p.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10964/
> -----------------------------------------------------------
> 
> (Updated July 8, 2013, 8:47 p.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Bugs: SQOOP-927
>     https://issues.apache.org/jira/browse/SQOOP-927
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit ff7dba55a09dd7789a34136233000c625759e583
> Author: Abraham Elmahrek <[email protected]>
> Date:   Fri Apr 26 15:10:24 2013 -0700
> 
>     SQOOP-927 Sqoop2: Integration: Mapreduce specific tests should be running 
> on MiniCluster
>     
>     Handle MiniDFSCluster and MiniMRClientCluster on own.
>     
>     Set yarn.application.classpath to get over classpath errors.
>     Set to use fair scheduler.
> 
> :100644 100644 0abbb18... f09704b... M        pom.xml
> :100644 100644 6eb3184... a4c2a5b... M        
> test/src/test/java/org/apache/sqoop/integration/TomcatTestCase.java
> :100644 100644 0f48a8b... 758c885... M        
> test/src/test/java/org/apache/sqoop/integration/server/VersionTest.java
> 
> 
> Diffs
> -----
> 
>   pom.xml 8785e01000cc6e7d5a74ffeb96b83d236a657df8 
>   test/src/main/java/org/apache/sqoop/test/asserts/HdfsAsserts.java 
> 056e6124b918ce5821c389e388a0cdfa68fd7fc0 
>   test/src/main/java/org/apache/sqoop/test/hadoop/HadoopCluster.java 
> PRE-CREATION 
>   test/src/main/java/org/apache/sqoop/test/hadoop/HadoopClusterFactory.java 
> PRE-CREATION 
>   test/src/main/java/org/apache/sqoop/test/hadoop/HadoopMiniCluster.java 
> PRE-CREATION 
>   
> test/src/main/java/org/apache/sqoop/test/hadoop/HadoopStandaloneCluster.java 
> PRE-CREATION 
>   test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java 
> 6aeadd4e54e47e5644270b15813b2d9c4cedc059 
>   test/src/main/java/org/apache/sqoop/test/testcases/TomcatTestCase.java 
> ca77e64486d80b38306b5b30e185e6278ad7eaf1 
>   test/src/main/java/org/apache/sqoop/test/utils/HdfsUtils.java 
> 95dd177a11c5d822e35ca54e5d93785f0a40fbfc 
> 
> Diff: https://reviews.apache.org/r/10964/diff/
> 
> 
> Testing
> -------
> 
> Ran integration tests without and with miniclusters.
> Currently need to use both miniclusters or neither for tests to work.
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>

Reply via email to