You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Veena Basavaraj <vb...@cloudera.com> on 2014/11/10 21:28:44 UTC

Review Request 27578: SQOOP-1348: Remove Data class and test clean up

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

Review request for Sqoop.


Summary (updated)
-----------------

SQOOP-1348: Remove Data class and test clean up


Bugs: SQOOP-1348
    https://issues.apache.org/jira/browse/SQOOP-1348


Repository: sqoop-sqoop2


Description (updated)
-------

see JIRA


Diffs (updated)
-----

  common/src/main/java/org/apache/sqoop/schema/Schema.java 40c362c 
  connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java e0e4061 
  connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestCSVIntermediateDataFormat.java 72e95ed 
  execution/mapreduce/src/main/java/org/apache/sqoop/job/io/Data.java 139883e 
  execution/mapreduce/src/test/java/org/apache/sqoop/job/JobUtils.java dafdeb4 
  execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java 78ae4ec 
  execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 04fb692 
  execution/mapreduce/src/test/java/org/apache/sqoop/job/io/SqoopWritableTest.java 68ce5ed 
  execution/mapreduce/src/test/java/org/apache/sqoop/job/io/TestData.java 4e23bcb 
  execution/mapreduce/src/test/java/org/apache/sqoop/job/io/TestSqoopWritable.java PRE-CREATION 
  execution/mapreduce/src/test/java/org/apache/sqoop/job/mr/TestSqoopOutputFormatLoadExecutor.java 5bd11f0 
  execution/mapreduce/src/test/java/org/apache/sqoop/job/util/MRJobTestUtil.java PRE-CREATION 

Diff: https://reviews.apache.org/r/27578/diff/


Testing
-------


Thanks,

Veena Basavaraj


Re: Review Request 27578: SQOOP-1348: Remove Data class and test clean up

Posted by Veena Basavaraj <vb...@cloudera.com>.

> On Nov. 10, 2014, 1:40 p.m., Abraham Elmahrek wrote:
> > execution/mapreduce/src/test/java/org/apache/sqoop/job/util/MRJobTestUtil.java, line 45
> > <https://reviews.apache.org/r/27578/diff/2/?file=757265#file757265line45>
> >
> >     What is replacing this?

http://stackoverflow.com/questions/21645431/what-are-the-replacement-for-hadoop-job-deprecated-class


- Veena


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


On Nov. 10, 2014, 12:49 p.m., Veena Basavaraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27578/
> -----------------------------------------------------------
> 
> (Updated Nov. 10, 2014, 12:49 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1348
>     https://issues.apache.org/jira/browse/SQOOP-1348
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> see JIRA
> 
> - removed Data and TestData
> - use IDF in tests, add a utlity method to set schema and IDF, 
> - amend CSV IDF to take schema since it is required to have schema, and add tests to check for null schema not just emptys schema in IDF
> - rename JobUtils to MRJobTestUtil
> - rename the SqoopWritabelTest to TestSQ...since we want to keep this as a convetion for all test classes, helps with code coverage tools
> - fixes some names to indicate the dataformat class instead of data
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/sqoop/schema/Schema.java 40c362c 
>   connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java e0e4061 
>   connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestCSVIntermediateDataFormat.java 72e95ed 
>   execution/mapreduce/src/main/java/org/apache/sqoop/job/io/Data.java 139883e 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/JobUtils.java dafdeb4 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java 78ae4ec 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 04fb692 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/io/SqoopWritableTest.java 68ce5ed 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/io/TestData.java 4e23bcb 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/io/TestSqoopWritable.java PRE-CREATION 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/mr/TestSqoopOutputFormatLoadExecutor.java 5bd11f0 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/util/MRJobTestUtil.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/27578/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Veena Basavaraj
> 
>


Re: Review Request 27578: SQOOP-1348: Remove Data class and test clean up

Posted by Veena Basavaraj <vb...@cloudera.com>.

> On Nov. 10, 2014, 1:40 p.m., Abraham Elmahrek wrote:
> > execution/mapreduce/src/test/java/org/apache/sqoop/job/util/MRJobTestUtil.java, line 45
> > <https://reviews.apache.org/r/27578/diff/2/?file=757265#file757265line45>
> >
> >     What is replacing this?
> 
> Veena Basavaraj wrote:
>     http://stackoverflow.com/questions/21645431/what-are-the-replacement-for-hadoop-job-deprecated-class

will remember to clean up it as part of the 1452 haddop upgrade ticket.


- Veena


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


On Nov. 10, 2014, 12:49 p.m., Veena Basavaraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27578/
> -----------------------------------------------------------
> 
> (Updated Nov. 10, 2014, 12:49 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1348
>     https://issues.apache.org/jira/browse/SQOOP-1348
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> see JIRA
> 
> - removed Data and TestData
> - use IDF in tests, add a utlity method to set schema and IDF, 
> - amend CSV IDF to take schema since it is required to have schema, and add tests to check for null schema not just emptys schema in IDF
> - rename JobUtils to MRJobTestUtil
> - rename the SqoopWritabelTest to TestSQ...since we want to keep this as a convetion for all test classes, helps with code coverage tools
> - fixes some names to indicate the dataformat class instead of data
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/sqoop/schema/Schema.java 40c362c 
>   connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java e0e4061 
>   connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestCSVIntermediateDataFormat.java 72e95ed 
>   execution/mapreduce/src/main/java/org/apache/sqoop/job/io/Data.java 139883e 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/JobUtils.java dafdeb4 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java 78ae4ec 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 04fb692 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/io/SqoopWritableTest.java 68ce5ed 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/io/TestData.java 4e23bcb 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/io/TestSqoopWritable.java PRE-CREATION 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/mr/TestSqoopOutputFormatLoadExecutor.java 5bd11f0 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/util/MRJobTestUtil.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/27578/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Veena Basavaraj
> 
>


Re: Review Request 27578: SQOOP-1348: Remove Data class and test clean up

Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27578/#review60683
-----------------------------------------------------------

Ship it!



execution/mapreduce/src/test/java/org/apache/sqoop/job/util/MRJobTestUtil.java
<https://reviews.apache.org/r/27578/#comment102059>

    What is replacing this?


- Abraham Elmahrek


On Nov. 10, 2014, 8:49 p.m., Veena Basavaraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27578/
> -----------------------------------------------------------
> 
> (Updated Nov. 10, 2014, 8:49 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1348
>     https://issues.apache.org/jira/browse/SQOOP-1348
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> see JIRA
> 
> - removed Data and TestData
> - use IDF in tests, add a utlity method to set schema and IDF, 
> - amend CSV IDF to take schema since it is required to have schema, and add tests to check for null schema not just emptys schema in IDF
> - rename JobUtils to MRJobTestUtil
> - rename the SqoopWritabelTest to TestSQ...since we want to keep this as a convetion for all test classes, helps with code coverage tools
> - fixes some names to indicate the dataformat class instead of data
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/sqoop/schema/Schema.java 40c362c 
>   connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java e0e4061 
>   connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestCSVIntermediateDataFormat.java 72e95ed 
>   execution/mapreduce/src/main/java/org/apache/sqoop/job/io/Data.java 139883e 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/JobUtils.java dafdeb4 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java 78ae4ec 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 04fb692 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/io/SqoopWritableTest.java 68ce5ed 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/io/TestData.java 4e23bcb 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/io/TestSqoopWritable.java PRE-CREATION 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/mr/TestSqoopOutputFormatLoadExecutor.java 5bd11f0 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/util/MRJobTestUtil.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/27578/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Veena Basavaraj
> 
>


Re: Review Request 27578: SQOOP-1348: Remove Data class and test clean up

Posted by Veena Basavaraj <vb...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27578/
-----------------------------------------------------------

(Updated Nov. 10, 2014, 12:49 p.m.)


Review request for Sqoop.


Bugs: SQOOP-1348
    https://issues.apache.org/jira/browse/SQOOP-1348


Repository: sqoop-sqoop2


Description
-------

see JIRA

- removed Data and TestData
- use IDF in tests, add a utlity method to set schema and IDF, 
- amend CSV IDF to take schema since it is required to have schema, and add tests to check for null schema not just emptys schema in IDF
- rename JobUtils to MRJobTestUtil
- rename the SqoopWritabelTest to TestSQ...since we want to keep this as a convetion for all test classes, helps with code coverage tools
- fixes some names to indicate the dataformat class instead of data


Diffs (updated)
-----

  common/src/main/java/org/apache/sqoop/schema/Schema.java 40c362c 
  connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java e0e4061 
  connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestCSVIntermediateDataFormat.java 72e95ed 
  execution/mapreduce/src/main/java/org/apache/sqoop/job/io/Data.java 139883e 
  execution/mapreduce/src/test/java/org/apache/sqoop/job/JobUtils.java dafdeb4 
  execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java 78ae4ec 
  execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 04fb692 
  execution/mapreduce/src/test/java/org/apache/sqoop/job/io/SqoopWritableTest.java 68ce5ed 
  execution/mapreduce/src/test/java/org/apache/sqoop/job/io/TestData.java 4e23bcb 
  execution/mapreduce/src/test/java/org/apache/sqoop/job/io/TestSqoopWritable.java PRE-CREATION 
  execution/mapreduce/src/test/java/org/apache/sqoop/job/mr/TestSqoopOutputFormatLoadExecutor.java 5bd11f0 
  execution/mapreduce/src/test/java/org/apache/sqoop/job/util/MRJobTestUtil.java PRE-CREATION 

Diff: https://reviews.apache.org/r/27578/diff/


Testing
-------


Thanks,

Veena Basavaraj


Re: Review Request 27578: SQOOP-1348: Remove Data class and test clean up

Posted by Veena Basavaraj <vb...@cloudera.com>.

> On Nov. 10, 2014, 12:44 p.m., Abraham Elmahrek wrote:
> > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java, line 176
> > <https://reviews.apache.org/r/27578/diff/1/?file=757160#file757160line176>
> >
> >     I don't think schema == null is possible. No sense in not being defensive though.

schema can be null if we dont call set schema, that happened in my test when I just did new IDF and did not set this,

It was throwing a NPE instead of telling me the exact reaaons


> On Nov. 10, 2014, 12:44 p.m., Abraham Elmahrek wrote:
> > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java, lines 73-78
> > <https://reviews.apache.org/r/27578/diff/1/?file=757160#file757160line73>
> >
> >     How will the intermediate data format normally be created? Will these constructors ever be used out of testing? If not, why not call setSchema directly in tests?

i can remove this if we want to keep the set schema as the only options


- Veena


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


On Nov. 10, 2014, 12:38 p.m., Veena Basavaraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27578/
> -----------------------------------------------------------
> 
> (Updated Nov. 10, 2014, 12:38 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1348
>     https://issues.apache.org/jira/browse/SQOOP-1348
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> see JIRA
> 
> - removed Data and TestData
> - use IDF in tests, add a utlity method to set schema and IDF, 
> - amend CSV IDF to take schema since it is required to have schema, and add tests to check for null schema not just emptys schema in IDF
> - rename JobUtils to MRJobTestUtil
> - rename the SqoopWritabelTest to TestSQ...since we want to keep this as a convetion for all test classes, helps with code coverage tools
> - fixes some names to indicate the dataformat class instead of data
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/sqoop/schema/Schema.java 40c362c 
>   connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java e0e4061 
>   connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestCSVIntermediateDataFormat.java 72e95ed 
>   execution/mapreduce/src/main/java/org/apache/sqoop/job/io/Data.java 139883e 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/JobUtils.java dafdeb4 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java 78ae4ec 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 04fb692 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/io/SqoopWritableTest.java 68ce5ed 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/io/TestData.java 4e23bcb 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/io/TestSqoopWritable.java PRE-CREATION 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/mr/TestSqoopOutputFormatLoadExecutor.java 5bd11f0 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/util/MRJobTestUtil.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/27578/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Veena Basavaraj
> 
>


Re: Review Request 27578: SQOOP-1348: Remove Data class and test clean up

Posted by Veena Basavaraj <vb...@cloudera.com>.

> On Nov. 10, 2014, 12:44 p.m., Abraham Elmahrek wrote:
> > common/src/main/java/org/apache/sqoop/schema/Schema.java, line 125
> > <https://reviews.apache.org/r/27578/diff/1/?file=757159#file757159line125>
> >
> >     columns.isEmpty() ?

even better.


- Veena


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


On Nov. 10, 2014, 12:38 p.m., Veena Basavaraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27578/
> -----------------------------------------------------------
> 
> (Updated Nov. 10, 2014, 12:38 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1348
>     https://issues.apache.org/jira/browse/SQOOP-1348
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> see JIRA
> 
> - removed Data and TestData
> - use IDF in tests, add a utlity method to set schema and IDF, 
> - amend CSV IDF to take schema since it is required to have schema, and add tests to check for null schema not just emptys schema in IDF
> - rename JobUtils to MRJobTestUtil
> - rename the SqoopWritabelTest to TestSQ...since we want to keep this as a convetion for all test classes, helps with code coverage tools
> - fixes some names to indicate the dataformat class instead of data
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/sqoop/schema/Schema.java 40c362c 
>   connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java e0e4061 
>   connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestCSVIntermediateDataFormat.java 72e95ed 
>   execution/mapreduce/src/main/java/org/apache/sqoop/job/io/Data.java 139883e 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/JobUtils.java dafdeb4 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java 78ae4ec 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 04fb692 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/io/SqoopWritableTest.java 68ce5ed 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/io/TestData.java 4e23bcb 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/io/TestSqoopWritable.java PRE-CREATION 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/mr/TestSqoopOutputFormatLoadExecutor.java 5bd11f0 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/util/MRJobTestUtil.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/27578/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Veena Basavaraj
> 
>


Re: Review Request 27578: SQOOP-1348: Remove Data class and test clean up

Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27578/#review60665
-----------------------------------------------------------



common/src/main/java/org/apache/sqoop/schema/Schema.java
<https://reviews.apache.org/r/27578/#comment102038>

    columns.isEmpty() ?



connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java
<https://reviews.apache.org/r/27578/#comment102040>

    How will the intermediate data format normally be created? Will these constructors ever be used out of testing? If not, why not call setSchema directly in tests?



connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java
<https://reviews.apache.org/r/27578/#comment102037>

    I don't think schema == null is possible. No sense in not being defensive though.


- Abraham Elmahrek


On Nov. 10, 2014, 8:38 p.m., Veena Basavaraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27578/
> -----------------------------------------------------------
> 
> (Updated Nov. 10, 2014, 8:38 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1348
>     https://issues.apache.org/jira/browse/SQOOP-1348
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> see JIRA
> 
> - removed Data and TestData
> - use IDF in tests, add a utlity method to set schema and IDF, 
> - amend CSV IDF to take schema since it is required to have schema, and add tests to check for null schema not just emptys schema in IDF
> - rename JobUtils to MRJobTestUtil
> - rename the SqoopWritabelTest to TestSQ...since we want to keep this as a convetion for all test classes, helps with code coverage tools
> - fixes some names to indicate the dataformat class instead of data
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/sqoop/schema/Schema.java 40c362c 
>   connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java e0e4061 
>   connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestCSVIntermediateDataFormat.java 72e95ed 
>   execution/mapreduce/src/main/java/org/apache/sqoop/job/io/Data.java 139883e 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/JobUtils.java dafdeb4 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java 78ae4ec 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 04fb692 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/io/SqoopWritableTest.java 68ce5ed 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/io/TestData.java 4e23bcb 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/io/TestSqoopWritable.java PRE-CREATION 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/mr/TestSqoopOutputFormatLoadExecutor.java 5bd11f0 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/util/MRJobTestUtil.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/27578/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Veena Basavaraj
> 
>


Re: Review Request 27578: SQOOP-1348: Remove Data class and test clean up

Posted by Veena Basavaraj <vb...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27578/
-----------------------------------------------------------

(Updated Nov. 10, 2014, 12:38 p.m.)


Review request for Sqoop.


Bugs: SQOOP-1348
    https://issues.apache.org/jira/browse/SQOOP-1348


Repository: sqoop-sqoop2


Description (updated)
-------

see JIRA

- removed Data and TestData
- use IDF in tests, add a utlity method to set schema and IDF, 
- amend CSV IDF to take schema since it is required to have schema, and add tests to check for null schema not just emptys schema in IDF
- rename JobUtils to MRJobTestUtil
- rename the SqoopWritabelTest to TestSQ...since we want to keep this as a convetion for all test classes, helps with code coverage tools
- fixes some names to indicate the dataformat class instead of data


Diffs
-----

  common/src/main/java/org/apache/sqoop/schema/Schema.java 40c362c 
  connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java e0e4061 
  connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestCSVIntermediateDataFormat.java 72e95ed 
  execution/mapreduce/src/main/java/org/apache/sqoop/job/io/Data.java 139883e 
  execution/mapreduce/src/test/java/org/apache/sqoop/job/JobUtils.java dafdeb4 
  execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java 78ae4ec 
  execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 04fb692 
  execution/mapreduce/src/test/java/org/apache/sqoop/job/io/SqoopWritableTest.java 68ce5ed 
  execution/mapreduce/src/test/java/org/apache/sqoop/job/io/TestData.java 4e23bcb 
  execution/mapreduce/src/test/java/org/apache/sqoop/job/io/TestSqoopWritable.java PRE-CREATION 
  execution/mapreduce/src/test/java/org/apache/sqoop/job/mr/TestSqoopOutputFormatLoadExecutor.java 5bd11f0 
  execution/mapreduce/src/test/java/org/apache/sqoop/job/util/MRJobTestUtil.java PRE-CREATION 

Diff: https://reviews.apache.org/r/27578/diff/


Testing
-------


Thanks,

Veena Basavaraj