Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Sqoop >> mail # dev >> Review Request: Review request for SQOOP-1035 "Add MS Sqoop Connecter tests to repo"

Copy link to this message
Re: Review Request: Review request for SQOOP-1035 "Add MS Sqoop Connecter tests to repo"

This is an automatically generated e-mail. To reply, visit:
Patch looks good except for a few nits.  It is a big patch and adds lots of tests which is badly needed.    There are few places where the editor generated TODO comments have to be removed and the exception handling can be a little bit more precise (instead of printing the stack trace and ignoring the exception), the exceptions can be rethrown after printing the stackTrace for debugging purposes if debug mode is enabled (you can check the logger)

Similar issues on test setup.  If the setup throws an exception during one of the methods (say populate the table), it does not handle it -  rethrow to abort the test.  Similarly getConnection call ignores exception



    Nit - spaces


    Can you please remove the comment - it has been implemented


    please remove this


    Please remove this.   Don't we want to return null in this case?


    Please remove this exception block.  As the method is throwing the SQLException, it should be OK and the caller can handle the exception


    Please remove this block or rethrow the exception


    Same as before.   There are too many of these in the files, can you please address all of them.
- Venkat Ranganathan
On May 8, 2013, 12:30 a.m., Shuaishuai Nie wrote:
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10987/
> -----------------------------------------------------------
> (Updated May 8, 2013, 12:30 a.m.)
> Review request for Sqoop.
> Description
> -------
> Add the MS Sqoop connector tests that test integration scenarios with SQL Server to the repo.
> This addresses bug SQOOP-1035.
>     https://issues.apache.org/jira/browse/SQOOP-1035
> Diffs
> -----
>   conf/MSTest.properties PRE-CREATION
>   src/test/com/cloudera/sqoop/hive/TestHiveImport.java 462ccf1
>   src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java 27860c2
>   src/test/org/apache/sqoop/manager/MSSQLTestData.java PRE-CREATION
>   src/test/org/apache/sqoop/manager/MSSQLTestDataFileParser.java PRE-CREATION
>   src/test/org/apache/sqoop/manager/MSSQLTestUtils.java PRE-CREATION
>   src/test/org/apache/sqoop/manager/ManagerCompatExport.java PRE-CREATION
>   src/test/org/apache/sqoop/manager/SQLServerDatatypeExportDelimitedFileManualTest.java PRE-CREATION
>   src/test/org/apache/sqoop/manager/SQLServerDatatypeExportSequenceFileManualTest.java PRE-CREATION
>   src/test/org/apache/sqoop/manager/SQLServerDatatypeImportDelimitedFileManualTest.java PRE-CREATION
>   src/test/org/apache/sqoop/manager/SQLServerDatatypeImportSequenceFileManualTest.java PRE-CREATION
>   src/test/org/apache/sqoop/manager/SQLServerHiveImportManualTest.java PRE-CREATION
>   src/test/org/apache/sqoop/manager/SQLServerManagerManualTest.java PRE-CREATION
>   src/test/org/apache/sqoop/manager/SQLServerMultiColsManualTest.java PRE-CREATION
>   src/test/org/apache/sqoop/manager/SQLServerMultiMapsManualTest.java PRE-CREATION