You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Boglarka Egyed <eg...@gmail.com> on 2017/04/06 14:21:24 UTC
Review Request 58233: SQOOP-3167: Evalutation and automation of
SQLServer Manual tests
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58233/
-----------------------------------------------------------
Review request for Sqoop, Anna Szonyi, Szabolcs Vasas, and Liz Szilagyi.
Bugs: SQOOP-3167
https://issues.apache.org/jira/browse/SQOOP-3167
Repository: sqoop-trunk
Description
-------
Automated SQLServer Manual tests including test case correction and minor rework too:
- modified connection string setup to make them able to run automatically
- fixed failing test cases
- excluded invalid test cases
- added database cleanup logic in tearDown part
- updated java docs
- removed unused imports
- changing names to add them to the 3rd party test suite
Note: A more extensive refactor of the test classes in org.apache.sqoop.manager.sqlserver could be made, it will be addressed in another JIRA as that is a different scope.
Diffs
-----
build.xml 73db28b272c50b4f76fef8421e6b9dfe5fed40f4
src/test/com/cloudera/sqoop/hive/TestHiveImport.java 33e0cc41f6f379bac2085431e0f1adc60bce6bce
src/test/com/cloudera/sqoop/manager/SQLServerManagerExportManualTest.java 9a92479245fa35c210d8e49f847292ee53d6f9b1
src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java 1f69725da8408853ac55b1f316ce1b9ef015e674
src/test/org/apache/sqoop/manager/sqlserver/MSSQLTestUtils.java 851bf49614e829d07de252b83f4ad550d0cb043b
src/test/org/apache/sqoop/manager/sqlserver/ManagerCompatExport.java 8c5176ad61aae61b96c7458d3b4b83dc11960268
src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeExportDelimitedFileManualTest.java 099d7344beb428c58b32d926af5ea079211da490
src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeExportSequenceFileManualTest.java 21676f02510693dcdd856a1d9dfba7d05eace023
src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeImportDelimitedFileManualTest.java 519fb525bdbb167520368d404667036669925041
src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeImportSequenceFileManualTest.java a0dad8a60b99d522ad3691e15b8b16c56e4b5858
src/test/org/apache/sqoop/manager/sqlserver/SQLServerHiveImportManualTest.java 1999272181421a539318ed195ea4257f52b2ed08
src/test/org/apache/sqoop/manager/sqlserver/SQLServerManagerManualTest.java 1178e3c79de4d0b5c7a96c6ad7eb316ed15e47c4
src/test/org/apache/sqoop/manager/sqlserver/SQLServerMultiColsManualTest.java 6a8ab51967237f471044b868615fdb3e057b1d92
src/test/org/apache/sqoop/manager/sqlserver/SQLServerMultiMapsManualTest.java c9a5b5ef596cfc1b28948c2a071935dfb9500cde
src/test/org/apache/sqoop/manager/sqlserver/SQLServerParseMethodsManualTest.java cd05aecf1ae5bd79fb485325d58b33a73e9df290
src/test/org/apache/sqoop/manager/sqlserver/SQLServerQueryManualTest.java 0057ac9df562c8e92cf7b9014c5e4239886a8104
src/test/org/apache/sqoop/manager/sqlserver/SQLServerSplitByManualTest.java f85245ab8cdd66da983ac9017d356f251f22e7db
src/test/org/apache/sqoop/manager/sqlserver/SQLServerWhereManualTest.java 10ae03b324b15f5ea0cc3cbbc04d3a5041233dd9
Diff: https://reviews.apache.org/r/58233/diff/1/
Testing
-------
ant clean test, ant test
ant clean test -Dthirdparty=true -Dsqoop.thirdparty.lib.dir=3rdpartylib -Dsqoop.test.sqlserver.connectstring.host_url=sqlserver -Dsqoop.test.sqlserver.database=databasename -Dms.sqlserver.username=username -Dms.sqlserver.password=password -Dtestcase=SQLServer*
ant clean test -Dthirdparty=true -Dsqoop.thirdparty.lib.dir=3rdpartylib -Dsqoop.test.mysql.connectstring.host_url=mysql -Dsqoop.test.mysql.databasename=databasename -Dsqoop.test.mysql.password=password -Dsqoop.test.mysql.username=username -Dsqoop.test.oracle.connectstring=oracle -Dsqoop.test.postgresql.connectstring.host_url=postgre -Dsqoop.test.cubrid.connectstring.host_url=cubrid -Dsqoop.test.cubrid.connectstring.username=username -Dsqoop.test.cubrid.connectstring.database=database -Dsqoop.test.cubrid.connectstring.password=password -Dmapred.child.java.opts="-Djava.security.egd=file:/dev/../dev/urandom" -Dtest.timeout=1000000 -Dsqoop.test.sqlserver.connectstring.host_url=sqlserver -Dsqoop.test.sqlserver.database=database -Dms.sqlserver.username=username -Dms.sqlserver.password=password
Thanks,
Boglarka Egyed
Re: Review Request 58233: SQOOP-3167: Evalutation and automation of
SQLServer Manual tests
Posted by Boglarka Egyed <eg...@gmail.com>.
> On April 6, 2017, 3:16 p.m., Anna Szonyi wrote:
> > Hey Bogi,
> >
> > Thanks for looking through, cleaning up and adding the SQLServer tests to the other thirdparty tests, great work!
> >
> > Please see my two comments below.
> >
> > Thanks,
> > Anna
Thanks for the review, Anna!
> On April 6, 2017, 3:16 p.m., Anna Szonyi wrote:
> > src/test/com/cloudera/sqoop/hive/TestHiveImport.java
> > Line 385 (original), 385 (patched)
> > <https://reviews.apache.org/r/58233/diff/1/?file=1686059#file1686059line385>
> >
> > While you didn't name this method, do you think it would make sense to rename it to getTypes or something similar?
I have changed it, thanks.
> On April 6, 2017, 3:16 p.m., Anna Szonyi wrote:
> > src/test/com/cloudera/sqoop/hive/TestHiveImport.java
> > Line 463 (original), 463 (patched)
> > <https://reviews.apache.org/r/58233/diff/1/?file=1686059#file1686059line463>
> >
> > Why was the AsParquet removed from the method name?
It was by mistake, thanks for noticing!
- Boglarka
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58233/#review171209
-----------------------------------------------------------
On April 6, 2017, 2:21 p.m., Boglarka Egyed wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58233/
> -----------------------------------------------------------
>
> (Updated April 6, 2017, 2:21 p.m.)
>
>
> Review request for Sqoop, Anna Szonyi, Szabolcs Vasas, and Liz Szilagyi.
>
>
> Bugs: SQOOP-3167
> https://issues.apache.org/jira/browse/SQOOP-3167
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Automated SQLServer Manual tests including test case correction and minor rework too:
> - modified connection string setup to make them able to run automatically
> - fixed failing test cases
> - excluded invalid test cases
> - added database cleanup logic in tearDown part
> - updated java docs
> - removed unused imports
> - changing names to add them to the 3rd party test suite
>
> Note: A more extensive refactor of the test classes in org.apache.sqoop.manager.sqlserver could be made, it will be addressed in another JIRA as that is a different scope.
>
>
> Diffs
> -----
>
> build.xml 73db28b272c50b4f76fef8421e6b9dfe5fed40f4
> src/test/com/cloudera/sqoop/hive/TestHiveImport.java 33e0cc41f6f379bac2085431e0f1adc60bce6bce
> src/test/com/cloudera/sqoop/manager/SQLServerManagerExportManualTest.java 9a92479245fa35c210d8e49f847292ee53d6f9b1
> src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java 1f69725da8408853ac55b1f316ce1b9ef015e674
> src/test/org/apache/sqoop/manager/sqlserver/MSSQLTestUtils.java 851bf49614e829d07de252b83f4ad550d0cb043b
> src/test/org/apache/sqoop/manager/sqlserver/ManagerCompatExport.java 8c5176ad61aae61b96c7458d3b4b83dc11960268
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeExportDelimitedFileManualTest.java 099d7344beb428c58b32d926af5ea079211da490
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeExportSequenceFileManualTest.java 21676f02510693dcdd856a1d9dfba7d05eace023
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeImportDelimitedFileManualTest.java 519fb525bdbb167520368d404667036669925041
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeImportSequenceFileManualTest.java a0dad8a60b99d522ad3691e15b8b16c56e4b5858
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerHiveImportManualTest.java 1999272181421a539318ed195ea4257f52b2ed08
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerManagerManualTest.java 1178e3c79de4d0b5c7a96c6ad7eb316ed15e47c4
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerMultiColsManualTest.java 6a8ab51967237f471044b868615fdb3e057b1d92
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerMultiMapsManualTest.java c9a5b5ef596cfc1b28948c2a071935dfb9500cde
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerParseMethodsManualTest.java cd05aecf1ae5bd79fb485325d58b33a73e9df290
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerQueryManualTest.java 0057ac9df562c8e92cf7b9014c5e4239886a8104
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerSplitByManualTest.java f85245ab8cdd66da983ac9017d356f251f22e7db
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerWhereManualTest.java 10ae03b324b15f5ea0cc3cbbc04d3a5041233dd9
>
>
> Diff: https://reviews.apache.org/r/58233/diff/1/
>
>
> Testing
> -------
>
> ant clean test, ant test
>
> ant clean test -Dthirdparty=true -Dsqoop.thirdparty.lib.dir=3rdpartylib -Dsqoop.test.sqlserver.connectstring.host_url=sqlserver -Dsqoop.test.sqlserver.database=databasename -Dms.sqlserver.username=username -Dms.sqlserver.password=password -Dtestcase=SQLServer*
>
> ant clean test -Dthirdparty=true -Dsqoop.thirdparty.lib.dir=3rdpartylib -Dsqoop.test.mysql.connectstring.host_url=mysql -Dsqoop.test.mysql.databasename=databasename -Dsqoop.test.mysql.password=password -Dsqoop.test.mysql.username=username -Dsqoop.test.oracle.connectstring=oracle -Dsqoop.test.postgresql.connectstring.host_url=postgre -Dsqoop.test.cubrid.connectstring.host_url=cubrid -Dsqoop.test.cubrid.connectstring.username=username -Dsqoop.test.cubrid.connectstring.database=database -Dsqoop.test.cubrid.connectstring.password=password -Dmapred.child.java.opts="-Djava.security.egd=file:/dev/../dev/urandom" -Dtest.timeout=1000000 -Dsqoop.test.sqlserver.connectstring.host_url=sqlserver -Dsqoop.test.sqlserver.database=database -Dms.sqlserver.username=username -Dms.sqlserver.password=password
>
>
> Thanks,
>
> Boglarka Egyed
>
>
Re: Review Request 58233: SQOOP-3167: Evalutation and automation of
SQLServer Manual tests
Posted by Anna Szonyi <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58233/#review171209
-----------------------------------------------------------
Hey Bogi,
Thanks for looking through, cleaning up and adding the SQLServer tests to the other thirdparty tests, great work!
Please see my two comments below.
Thanks,
Anna
src/test/com/cloudera/sqoop/hive/TestHiveImport.java
Line 385 (original), 385 (patched)
<https://reviews.apache.org/r/58233/#comment244090>
While you didn't name this method, do you think it would make sense to rename it to getTypes or something similar?
src/test/com/cloudera/sqoop/hive/TestHiveImport.java
Line 463 (original), 463 (patched)
<https://reviews.apache.org/r/58233/#comment244089>
Why was the AsParquet removed from the method name?
- Anna Szonyi
On April 6, 2017, 2:21 p.m., Boglarka Egyed wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58233/
> -----------------------------------------------------------
>
> (Updated April 6, 2017, 2:21 p.m.)
>
>
> Review request for Sqoop, Anna Szonyi, Szabolcs Vasas, and Liz Szilagyi.
>
>
> Bugs: SQOOP-3167
> https://issues.apache.org/jira/browse/SQOOP-3167
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Automated SQLServer Manual tests including test case correction and minor rework too:
> - modified connection string setup to make them able to run automatically
> - fixed failing test cases
> - excluded invalid test cases
> - added database cleanup logic in tearDown part
> - updated java docs
> - removed unused imports
> - changing names to add them to the 3rd party test suite
>
> Note: A more extensive refactor of the test classes in org.apache.sqoop.manager.sqlserver could be made, it will be addressed in another JIRA as that is a different scope.
>
>
> Diffs
> -----
>
> build.xml 73db28b272c50b4f76fef8421e6b9dfe5fed40f4
> src/test/com/cloudera/sqoop/hive/TestHiveImport.java 33e0cc41f6f379bac2085431e0f1adc60bce6bce
> src/test/com/cloudera/sqoop/manager/SQLServerManagerExportManualTest.java 9a92479245fa35c210d8e49f847292ee53d6f9b1
> src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java 1f69725da8408853ac55b1f316ce1b9ef015e674
> src/test/org/apache/sqoop/manager/sqlserver/MSSQLTestUtils.java 851bf49614e829d07de252b83f4ad550d0cb043b
> src/test/org/apache/sqoop/manager/sqlserver/ManagerCompatExport.java 8c5176ad61aae61b96c7458d3b4b83dc11960268
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeExportDelimitedFileManualTest.java 099d7344beb428c58b32d926af5ea079211da490
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeExportSequenceFileManualTest.java 21676f02510693dcdd856a1d9dfba7d05eace023
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeImportDelimitedFileManualTest.java 519fb525bdbb167520368d404667036669925041
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeImportSequenceFileManualTest.java a0dad8a60b99d522ad3691e15b8b16c56e4b5858
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerHiveImportManualTest.java 1999272181421a539318ed195ea4257f52b2ed08
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerManagerManualTest.java 1178e3c79de4d0b5c7a96c6ad7eb316ed15e47c4
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerMultiColsManualTest.java 6a8ab51967237f471044b868615fdb3e057b1d92
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerMultiMapsManualTest.java c9a5b5ef596cfc1b28948c2a071935dfb9500cde
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerParseMethodsManualTest.java cd05aecf1ae5bd79fb485325d58b33a73e9df290
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerQueryManualTest.java 0057ac9df562c8e92cf7b9014c5e4239886a8104
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerSplitByManualTest.java f85245ab8cdd66da983ac9017d356f251f22e7db
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerWhereManualTest.java 10ae03b324b15f5ea0cc3cbbc04d3a5041233dd9
>
>
> Diff: https://reviews.apache.org/r/58233/diff/1/
>
>
> Testing
> -------
>
> ant clean test, ant test
>
> ant clean test -Dthirdparty=true -Dsqoop.thirdparty.lib.dir=3rdpartylib -Dsqoop.test.sqlserver.connectstring.host_url=sqlserver -Dsqoop.test.sqlserver.database=databasename -Dms.sqlserver.username=username -Dms.sqlserver.password=password -Dtestcase=SQLServer*
>
> ant clean test -Dthirdparty=true -Dsqoop.thirdparty.lib.dir=3rdpartylib -Dsqoop.test.mysql.connectstring.host_url=mysql -Dsqoop.test.mysql.databasename=databasename -Dsqoop.test.mysql.password=password -Dsqoop.test.mysql.username=username -Dsqoop.test.oracle.connectstring=oracle -Dsqoop.test.postgresql.connectstring.host_url=postgre -Dsqoop.test.cubrid.connectstring.host_url=cubrid -Dsqoop.test.cubrid.connectstring.username=username -Dsqoop.test.cubrid.connectstring.database=database -Dsqoop.test.cubrid.connectstring.password=password -Dmapred.child.java.opts="-Djava.security.egd=file:/dev/../dev/urandom" -Dtest.timeout=1000000 -Dsqoop.test.sqlserver.connectstring.host_url=sqlserver -Dsqoop.test.sqlserver.database=database -Dms.sqlserver.username=username -Dms.sqlserver.password=password
>
>
> Thanks,
>
> Boglarka Egyed
>
>
Re: Review Request 58233: SQOOP-3167: Evalutation and automation of
SQLServer Manual tests
Posted by Boglarka Egyed <eg...@gmail.com>.
> On April 7, 2017, 8:11 a.m., Attila Szabo wrote:
> > Hey Bogi,
> >
> > I've got two concerns related to this change:
> > - The change set is quite huge, and thus not easy to review. Could we please split up to smaller pieces?
> > - Besides the fact that this modification will provide some way to execute these tests through Junit and some ant tasks, I can't really spot how this change will make the test cases more "automated". Will these tests be included in the CI cycle? Do we already have a SqlServer instance connected to the Apache CI system we can test against? Which CI cycle would include the execution of this test suite?
> >
> > Thanks for your answers and clarification,
> >
> > Attila
Hi Attila,
Thanks for your inputs.
However it is a bigger change set, I would not split it into smaller pieces as I think this is a coherent whole representing one logical change. Every file contains similar, limited amount of modifications which makes easier to review it IMHO.
Automation means several things here. First, these all were manual tests meaning these were executed possibly never or a really long time ago as there were numerous tests failing. Also, these were able to be executed by some manual ant task which was quite difficult because of the hard coded DB related variables and thus were possibly avoided instead. But from now they can be executed as part of the 3rd party test suite by setting the DB connect string and credentials through system properties. This definitely makes easier to test changes which possibly affect integration with DBs as we already have the same practice for MySQL, Postgre, etc.
Unfortunately, even the 3rd party test suite is not a part of CI cycle, however, it totally makes sense to add it too and it should be. Fortunately, AFAIR there are plans to improve Sqoop CI cycles, the related communication has already started at dev@ and is an ongoing effort thus I wouldn't think that it should be part of the scope of this change - this change is a good first step toward it, however.
Many thanks,
Bogi
- Boglarka
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58233/#review171319
-----------------------------------------------------------
On April 6, 2017, 5:27 p.m., Boglarka Egyed wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58233/
> -----------------------------------------------------------
>
> (Updated April 6, 2017, 5:27 p.m.)
>
>
> Review request for Sqoop, Anna Szonyi, Szabolcs Vasas, and Liz Szilagyi.
>
>
> Bugs: SQOOP-3167
> https://issues.apache.org/jira/browse/SQOOP-3167
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Automated SQLServer Manual tests including test case correction and minor rework too:
> - modified connection string setup to make them able to run automatically
> - fixed failing test cases
> - excluded invalid test cases
> - added database cleanup logic in tearDown part
> - updated java docs
> - removed unused imports
> - changing names to add them to the 3rd party test suite
>
> Note: A more extensive refactor of the test classes in org.apache.sqoop.manager.sqlserver could be made, it will be addressed in another JIRA as that is a different scope.
>
>
> Diffs
> -----
>
> build.xml 73db28b272c50b4f76fef8421e6b9dfe5fed40f4
> src/test/com/cloudera/sqoop/hive/TestHiveImport.java 33e0cc41f6f379bac2085431e0f1adc60bce6bce
> src/test/com/cloudera/sqoop/manager/SQLServerManagerExportManualTest.java 9a92479245fa35c210d8e49f847292ee53d6f9b1
> src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java 1f69725da8408853ac55b1f316ce1b9ef015e674
> src/test/org/apache/sqoop/manager/sqlserver/MSSQLTestUtils.java 851bf49614e829d07de252b83f4ad550d0cb043b
> src/test/org/apache/sqoop/manager/sqlserver/ManagerCompatExport.java 8c5176ad61aae61b96c7458d3b4b83dc11960268
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeExportDelimitedFileManualTest.java 099d7344beb428c58b32d926af5ea079211da490
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeExportSequenceFileManualTest.java 21676f02510693dcdd856a1d9dfba7d05eace023
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeImportDelimitedFileManualTest.java 519fb525bdbb167520368d404667036669925041
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeImportSequenceFileManualTest.java a0dad8a60b99d522ad3691e15b8b16c56e4b5858
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerHiveImportManualTest.java 1999272181421a539318ed195ea4257f52b2ed08
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerManagerManualTest.java 1178e3c79de4d0b5c7a96c6ad7eb316ed15e47c4
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerMultiColsManualTest.java 6a8ab51967237f471044b868615fdb3e057b1d92
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerMultiMapsManualTest.java c9a5b5ef596cfc1b28948c2a071935dfb9500cde
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerParseMethodsManualTest.java cd05aecf1ae5bd79fb485325d58b33a73e9df290
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerQueryManualTest.java 0057ac9df562c8e92cf7b9014c5e4239886a8104
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerSplitByManualTest.java f85245ab8cdd66da983ac9017d356f251f22e7db
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerWhereManualTest.java 10ae03b324b15f5ea0cc3cbbc04d3a5041233dd9
>
>
> Diff: https://reviews.apache.org/r/58233/diff/2/
>
>
> Testing
> -------
>
> ant clean test, ant test
>
> ant clean test -Dthirdparty=true -Dsqoop.thirdparty.lib.dir=3rdpartylib -Dsqoop.test.sqlserver.connectstring.host_url=sqlserver -Dsqoop.test.sqlserver.database=databasename -Dms.sqlserver.username=username -Dms.sqlserver.password=password -Dtestcase=SQLServer*
>
> ant clean test -Dthirdparty=true -Dsqoop.thirdparty.lib.dir=3rdpartylib -Dsqoop.test.mysql.connectstring.host_url=mysql -Dsqoop.test.mysql.databasename=databasename -Dsqoop.test.mysql.password=password -Dsqoop.test.mysql.username=username -Dsqoop.test.oracle.connectstring=oracle -Dsqoop.test.postgresql.connectstring.host_url=postgre -Dsqoop.test.cubrid.connectstring.host_url=cubrid -Dsqoop.test.cubrid.connectstring.username=username -Dsqoop.test.cubrid.connectstring.database=database -Dsqoop.test.cubrid.connectstring.password=password -Dmapred.child.java.opts="-Djava.security.egd=file:/dev/../dev/urandom" -Dtest.timeout=1000000 -Dsqoop.test.sqlserver.connectstring.host_url=sqlserver -Dsqoop.test.sqlserver.database=database -Dms.sqlserver.username=username -Dms.sqlserver.password=password
>
>
> Thanks,
>
> Boglarka Egyed
>
>
Re: Review Request 58233: SQOOP-3167: Evalutation and automation of
SQLServer Manual tests
Posted by Attila Szabo <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58233/#review171319
-----------------------------------------------------------
Hey Bogi,
I've got two concerns related to this change:
- The change set is quite huge, and thus not easy to review. Could we please split up to smaller pieces?
- Besides the fact that this modification will provide some way to execute these tests through Junit and some ant tasks, I can't really spot how this change will make the test cases more "automated". Will these tests be included in the CI cycle? Do we already have a SqlServer instance connected to the Apache CI system we can test against? Which CI cycle would include the execution of this test suite?
Thanks for your answers and clarification,
Attila
- Attila Szabo
On April 6, 2017, 5:27 p.m., Boglarka Egyed wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58233/
> -----------------------------------------------------------
>
> (Updated April 6, 2017, 5:27 p.m.)
>
>
> Review request for Sqoop, Anna Szonyi, Szabolcs Vasas, and Liz Szilagyi.
>
>
> Bugs: SQOOP-3167
> https://issues.apache.org/jira/browse/SQOOP-3167
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Automated SQLServer Manual tests including test case correction and minor rework too:
> - modified connection string setup to make them able to run automatically
> - fixed failing test cases
> - excluded invalid test cases
> - added database cleanup logic in tearDown part
> - updated java docs
> - removed unused imports
> - changing names to add them to the 3rd party test suite
>
> Note: A more extensive refactor of the test classes in org.apache.sqoop.manager.sqlserver could be made, it will be addressed in another JIRA as that is a different scope.
>
>
> Diffs
> -----
>
> build.xml 73db28b272c50b4f76fef8421e6b9dfe5fed40f4
> src/test/com/cloudera/sqoop/hive/TestHiveImport.java 33e0cc41f6f379bac2085431e0f1adc60bce6bce
> src/test/com/cloudera/sqoop/manager/SQLServerManagerExportManualTest.java 9a92479245fa35c210d8e49f847292ee53d6f9b1
> src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java 1f69725da8408853ac55b1f316ce1b9ef015e674
> src/test/org/apache/sqoop/manager/sqlserver/MSSQLTestUtils.java 851bf49614e829d07de252b83f4ad550d0cb043b
> src/test/org/apache/sqoop/manager/sqlserver/ManagerCompatExport.java 8c5176ad61aae61b96c7458d3b4b83dc11960268
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeExportDelimitedFileManualTest.java 099d7344beb428c58b32d926af5ea079211da490
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeExportSequenceFileManualTest.java 21676f02510693dcdd856a1d9dfba7d05eace023
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeImportDelimitedFileManualTest.java 519fb525bdbb167520368d404667036669925041
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeImportSequenceFileManualTest.java a0dad8a60b99d522ad3691e15b8b16c56e4b5858
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerHiveImportManualTest.java 1999272181421a539318ed195ea4257f52b2ed08
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerManagerManualTest.java 1178e3c79de4d0b5c7a96c6ad7eb316ed15e47c4
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerMultiColsManualTest.java 6a8ab51967237f471044b868615fdb3e057b1d92
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerMultiMapsManualTest.java c9a5b5ef596cfc1b28948c2a071935dfb9500cde
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerParseMethodsManualTest.java cd05aecf1ae5bd79fb485325d58b33a73e9df290
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerQueryManualTest.java 0057ac9df562c8e92cf7b9014c5e4239886a8104
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerSplitByManualTest.java f85245ab8cdd66da983ac9017d356f251f22e7db
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerWhereManualTest.java 10ae03b324b15f5ea0cc3cbbc04d3a5041233dd9
>
>
> Diff: https://reviews.apache.org/r/58233/diff/2/
>
>
> Testing
> -------
>
> ant clean test, ant test
>
> ant clean test -Dthirdparty=true -Dsqoop.thirdparty.lib.dir=3rdpartylib -Dsqoop.test.sqlserver.connectstring.host_url=sqlserver -Dsqoop.test.sqlserver.database=databasename -Dms.sqlserver.username=username -Dms.sqlserver.password=password -Dtestcase=SQLServer*
>
> ant clean test -Dthirdparty=true -Dsqoop.thirdparty.lib.dir=3rdpartylib -Dsqoop.test.mysql.connectstring.host_url=mysql -Dsqoop.test.mysql.databasename=databasename -Dsqoop.test.mysql.password=password -Dsqoop.test.mysql.username=username -Dsqoop.test.oracle.connectstring=oracle -Dsqoop.test.postgresql.connectstring.host_url=postgre -Dsqoop.test.cubrid.connectstring.host_url=cubrid -Dsqoop.test.cubrid.connectstring.username=username -Dsqoop.test.cubrid.connectstring.database=database -Dsqoop.test.cubrid.connectstring.password=password -Dmapred.child.java.opts="-Djava.security.egd=file:/dev/../dev/urandom" -Dtest.timeout=1000000 -Dsqoop.test.sqlserver.connectstring.host_url=sqlserver -Dsqoop.test.sqlserver.database=database -Dms.sqlserver.username=username -Dms.sqlserver.password=password
>
>
> Thanks,
>
> Boglarka Egyed
>
>
Re: Review Request 58233: SQOOP-3167: Evalutation and automation of
SQLServer Manual tests
Posted by Liz Szilagyi <er...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58233/#review171350
-----------------------------------------------------------
Ship it!
Bogi,
Thank you for working diligently on our tests and thus improving quality!
I found while reading through the changes nothing really stood out as something that could be separated into another ticket, so personally I support keeping these changes together as one.
I also found the instructions included on how to run these tests very helpful.
Thank you,
Liz
- Liz Szilagyi
On April 6, 2017, 7:27 p.m., Boglarka Egyed wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58233/
> -----------------------------------------------------------
>
> (Updated April 6, 2017, 7:27 p.m.)
>
>
> Review request for Sqoop, Anna Szonyi, Szabolcs Vasas, and Liz Szilagyi.
>
>
> Bugs: SQOOP-3167
> https://issues.apache.org/jira/browse/SQOOP-3167
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Automated SQLServer Manual tests including test case correction and minor rework too:
> - modified connection string setup to make them able to run automatically
> - fixed failing test cases
> - excluded invalid test cases
> - added database cleanup logic in tearDown part
> - updated java docs
> - removed unused imports
> - changing names to add them to the 3rd party test suite
>
> Note: A more extensive refactor of the test classes in org.apache.sqoop.manager.sqlserver could be made, it will be addressed in another JIRA as that is a different scope.
>
>
> Diffs
> -----
>
> build.xml 73db28b272c50b4f76fef8421e6b9dfe5fed40f4
> src/test/com/cloudera/sqoop/hive/TestHiveImport.java 33e0cc41f6f379bac2085431e0f1adc60bce6bce
> src/test/com/cloudera/sqoop/manager/SQLServerManagerExportManualTest.java 9a92479245fa35c210d8e49f847292ee53d6f9b1
> src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java 1f69725da8408853ac55b1f316ce1b9ef015e674
> src/test/org/apache/sqoop/manager/sqlserver/MSSQLTestUtils.java 851bf49614e829d07de252b83f4ad550d0cb043b
> src/test/org/apache/sqoop/manager/sqlserver/ManagerCompatExport.java 8c5176ad61aae61b96c7458d3b4b83dc11960268
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeExportDelimitedFileManualTest.java 099d7344beb428c58b32d926af5ea079211da490
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeExportSequenceFileManualTest.java 21676f02510693dcdd856a1d9dfba7d05eace023
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeImportDelimitedFileManualTest.java 519fb525bdbb167520368d404667036669925041
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeImportSequenceFileManualTest.java a0dad8a60b99d522ad3691e15b8b16c56e4b5858
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerHiveImportManualTest.java 1999272181421a539318ed195ea4257f52b2ed08
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerManagerManualTest.java 1178e3c79de4d0b5c7a96c6ad7eb316ed15e47c4
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerMultiColsManualTest.java 6a8ab51967237f471044b868615fdb3e057b1d92
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerMultiMapsManualTest.java c9a5b5ef596cfc1b28948c2a071935dfb9500cde
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerParseMethodsManualTest.java cd05aecf1ae5bd79fb485325d58b33a73e9df290
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerQueryManualTest.java 0057ac9df562c8e92cf7b9014c5e4239886a8104
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerSplitByManualTest.java f85245ab8cdd66da983ac9017d356f251f22e7db
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerWhereManualTest.java 10ae03b324b15f5ea0cc3cbbc04d3a5041233dd9
>
>
> Diff: https://reviews.apache.org/r/58233/diff/2/
>
>
> Testing
> -------
>
> ant clean test, ant test
>
> ant clean test -Dthirdparty=true -Dsqoop.thirdparty.lib.dir=3rdpartylib -Dsqoop.test.sqlserver.connectstring.host_url=sqlserver -Dsqoop.test.sqlserver.database=databasename -Dms.sqlserver.username=username -Dms.sqlserver.password=password -Dtestcase=SQLServer*
>
> ant clean test -Dthirdparty=true -Dsqoop.thirdparty.lib.dir=3rdpartylib -Dsqoop.test.mysql.connectstring.host_url=mysql -Dsqoop.test.mysql.databasename=databasename -Dsqoop.test.mysql.password=password -Dsqoop.test.mysql.username=username -Dsqoop.test.oracle.connectstring=oracle -Dsqoop.test.postgresql.connectstring.host_url=postgre -Dsqoop.test.cubrid.connectstring.host_url=cubrid -Dsqoop.test.cubrid.connectstring.username=username -Dsqoop.test.cubrid.connectstring.database=database -Dsqoop.test.cubrid.connectstring.password=password -Dmapred.child.java.opts="-Djava.security.egd=file:/dev/../dev/urandom" -Dtest.timeout=1000000 -Dsqoop.test.sqlserver.connectstring.host_url=sqlserver -Dsqoop.test.sqlserver.database=database -Dms.sqlserver.username=username -Dms.sqlserver.password=password
>
>
> Thanks,
>
> Boglarka Egyed
>
>
Re: Review Request 58233: SQOOP-3167: Evalutation and automation of
SQLServer Manual tests
Posted by Attila Szabo <ma...@apache.org>.
> On April 7, 2017, 5:25 p.m., Attila Szabo wrote:
> > Hi Bogi, Liz,
> >
> > Most probably I did not phrase my thoughts precise enough in the previous round, so please let me express myself in a bit more direct and clear way:
> > My problem is not with the size of the patch file (although IMHO ~1000+ changed lines is quite a few), but with the fact that this change tries to achieve multiple things, which I think would make more sense as part of multiple issues/commits. Here are the things I've identified as the effects/consequences of this changeset:
> > - It delivers improvements/fixes around the way connections are handled in the MSSqlServer related tests, which is a great thing, makes the usage much more flexible.
> > -- Although around the deletes (were those codes really dead?)/fixes I'm not sure if those things would be in sync with the design goals of the SqlServer connector, but I'm not used to consider myself as a MSSqlServer expert.
> > - Pushes the SqlServer related manual tests into the third party ant test cycle. IMHO this is not the best decision in the current test + CI architecture, as from that moment this patch would have been committed, it would force every contributor to have a working MSSqlServer instance (on the top of the currently existing ones including MySQL, PostgreSQL, Oracle, Cubrid, etc.) on their dev infrastructure, or facing with the fact that some of the third party tests will fail continuously on their side (which does not sound like a best practice). Most probably on your side it didn't appear as a problem as you do have standalone instances for yourself, but we cannot depend on that this is true in case of every contributor. Although the test files themselves contains some very basic instructions how to make the tests work, but still in the current version it would need manual interactions+installs, thus won't work out of the box (needless to say that the instructions are very much not
detailed enough to someone who is not expert in installing MSSqlServer).
> > -- As a subpart of this section, I have to highlight that this way of changes alternates the intention of the original devs (and I'm pretty sure they had good reason why these tests were not automated but manual).
> > -- Introducing just another external dependency won't help to onboard new contributors (just another new -D param for a process which is not quite good documented already)
> > -- It also won't make the CI integration easier in the future
> > - The title+description JIRA is missleading as it does not solve the true automation (neither on CI level, nor on the side of the devs).
> >
> > So if you trust in my experience and wisdom as a committer you would consider splitting up this changeset into 2-3 parts, and only after pushing the tests into the 3rd party cycle, when the SqlServer deployment is solved out of the box (e.g. with docker or global apache installation or ansible or so).
> >
> > Please also consider fixing that part what I've identified as an issue
> >
> > My 2 cents,
> > Attila
I suggest discussing this on dev@sqoop list.
If the PMC agrees on adding SqlServer integration tests to the current test suite, I'd be happy to merge your patch.
- Attila
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58233/#review171363
-----------------------------------------------------------
On April 6, 2017, 5:27 p.m., Boglarka Egyed wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58233/
> -----------------------------------------------------------
>
> (Updated April 6, 2017, 5:27 p.m.)
>
>
> Review request for Sqoop, Anna Szonyi, Szabolcs Vasas, and Liz Szilagyi.
>
>
> Bugs: SQOOP-3167
> https://issues.apache.org/jira/browse/SQOOP-3167
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Automated SQLServer Manual tests including test case correction and minor rework too:
> - modified connection string setup to make them able to run automatically
> - fixed failing test cases
> - excluded invalid test cases
> - added database cleanup logic in tearDown part
> - updated java docs
> - removed unused imports
> - changing names to add them to the 3rd party test suite
>
> Note: A more extensive refactor of the test classes in org.apache.sqoop.manager.sqlserver could be made, it will be addressed in another JIRA as that is a different scope.
>
>
> Diffs
> -----
>
> build.xml 73db28b272c50b4f76fef8421e6b9dfe5fed40f4
> src/test/com/cloudera/sqoop/hive/TestHiveImport.java 33e0cc41f6f379bac2085431e0f1adc60bce6bce
> src/test/com/cloudera/sqoop/manager/SQLServerManagerExportManualTest.java 9a92479245fa35c210d8e49f847292ee53d6f9b1
> src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java 1f69725da8408853ac55b1f316ce1b9ef015e674
> src/test/org/apache/sqoop/manager/sqlserver/MSSQLTestUtils.java 851bf49614e829d07de252b83f4ad550d0cb043b
> src/test/org/apache/sqoop/manager/sqlserver/ManagerCompatExport.java 8c5176ad61aae61b96c7458d3b4b83dc11960268
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeExportDelimitedFileManualTest.java 099d7344beb428c58b32d926af5ea079211da490
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeExportSequenceFileManualTest.java 21676f02510693dcdd856a1d9dfba7d05eace023
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeImportDelimitedFileManualTest.java 519fb525bdbb167520368d404667036669925041
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeImportSequenceFileManualTest.java a0dad8a60b99d522ad3691e15b8b16c56e4b5858
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerHiveImportManualTest.java 1999272181421a539318ed195ea4257f52b2ed08
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerManagerManualTest.java 1178e3c79de4d0b5c7a96c6ad7eb316ed15e47c4
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerMultiColsManualTest.java 6a8ab51967237f471044b868615fdb3e057b1d92
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerMultiMapsManualTest.java c9a5b5ef596cfc1b28948c2a071935dfb9500cde
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerParseMethodsManualTest.java cd05aecf1ae5bd79fb485325d58b33a73e9df290
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerQueryManualTest.java 0057ac9df562c8e92cf7b9014c5e4239886a8104
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerSplitByManualTest.java f85245ab8cdd66da983ac9017d356f251f22e7db
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerWhereManualTest.java 10ae03b324b15f5ea0cc3cbbc04d3a5041233dd9
>
>
> Diff: https://reviews.apache.org/r/58233/diff/2/
>
>
> Testing
> -------
>
> ant clean test, ant test
>
> ant clean test -Dthirdparty=true -Dsqoop.thirdparty.lib.dir=3rdpartylib -Dsqoop.test.sqlserver.connectstring.host_url=sqlserver -Dsqoop.test.sqlserver.database=databasename -Dms.sqlserver.username=username -Dms.sqlserver.password=password -Dtestcase=SQLServer*
>
> ant clean test -Dthirdparty=true -Dsqoop.thirdparty.lib.dir=3rdpartylib -Dsqoop.test.mysql.connectstring.host_url=mysql -Dsqoop.test.mysql.databasename=databasename -Dsqoop.test.mysql.password=password -Dsqoop.test.mysql.username=username -Dsqoop.test.oracle.connectstring=oracle -Dsqoop.test.postgresql.connectstring.host_url=postgre -Dsqoop.test.cubrid.connectstring.host_url=cubrid -Dsqoop.test.cubrid.connectstring.username=username -Dsqoop.test.cubrid.connectstring.database=database -Dsqoop.test.cubrid.connectstring.password=password -Dmapred.child.java.opts="-Djava.security.egd=file:/dev/../dev/urandom" -Dtest.timeout=1000000 -Dsqoop.test.sqlserver.connectstring.host_url=sqlserver -Dsqoop.test.sqlserver.database=database -Dms.sqlserver.username=username -Dms.sqlserver.password=password
>
>
> Thanks,
>
> Boglarka Egyed
>
>
Re: Review Request 58233: SQOOP-3167: Evalutation and automation of
SQLServer Manual tests
Posted by Abraham Fine <af...@apache.org>.
Just a small comment. Let's make sure that going forward we discuss
ideas based on their merit and not justify them with an appeal to
authority. There is no need to "trust in anyone's experience and wisdom"
when a meritocracy is operating properly.
Thanks,
Abe
On Mon, Apr 10, 2017, at 05:51, Boglarka Egyed wrote:
>
>
> > On April 7, 2017, 5:25 p.m., Attila Szabo wrote:
> > > Hi Bogi, Liz,
> > >
> > > Most probably I did not phrase my thoughts precise enough in the previous round, so please let me express myself in a bit more direct and clear way:
> > > My problem is not with the size of the patch file (although IMHO ~1000+ changed lines is quite a few), but with the fact that this change tries to achieve multiple things, which I think would make more sense as part of multiple issues/commits. Here are the things I've identified as the effects/consequences of this changeset:
> > > - It delivers improvements/fixes around the way connections are handled in the MSSqlServer related tests, which is a great thing, makes the usage much more flexible.
> > > -- Although around the deletes (were those codes really dead?)/fixes I'm not sure if those things would be in sync with the design goals of the SqlServer connector, but I'm not used to consider myself as a MSSqlServer expert.
> > > - Pushes the SqlServer related manual tests into the third party ant test cycle. IMHO this is not the best decision in the current test + CI architecture, as from that moment this patch would have been committed, it would force every contributor to have a working MSSqlServer instance (on the top of the currently existing ones including MySQL, PostgreSQL, Oracle, Cubrid, etc.) on their dev infrastructure, or facing with the fact that some of the third party tests will fail continuously on their side (which does not sound like a best practice). Most probably on your side it didn't appear as a problem as you do have standalone instances for yourself, but we cannot depend on that this is true in case of every contributor. Although the test files themselves contains some very basic instructions how to make the tests work, but still in the current version it would need manual interactions+installs, thus won't work out of the box (needless to say that the instructions are very much not
> detailed enough to someone who is not expert in installing MSSqlServer).
> > > -- As a subpart of this section, I have to highlight that this way of changes alternates the intention of the original devs (and I'm pretty sure they had good reason why these tests were not automated but manual).
> > > -- Introducing just another external dependency won't help to onboard new contributors (just another new -D param for a process which is not quite good documented already)
> > > -- It also won't make the CI integration easier in the future
> > > - The title+description JIRA is missleading as it does not solve the true automation (neither on CI level, nor on the side of the devs).
> > >
> > > So if you trust in my experience and wisdom as a committer you would consider splitting up this changeset into 2-3 parts, and only after pushing the tests into the 3rd party cycle, when the SqlServer deployment is solved out of the box (e.g. with docker or global apache installation or ansible or so).
> > >
> > > Please also consider fixing that part what I've identified as an issue
> > >
> > > My 2 cents,
> > > Attila
> >
> > Attila Szabo wrote:
> > I suggest discussing this on dev@sqoop list.
> > If the PMC agrees on adding SqlServer integration tests to the current test suite, I'd be happy to merge your patch.
>
> Thanks Attila for your verbose answer and suggestions, I discard this
> review and cancel the patch.
> I have created another JIRA with a reduced scope, see SQOOP-3169
>
> Regards,
> Bogi
>
>
> - Boglarka
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58233/#review171363
> -----------------------------------------------------------
>
>
> On April 6, 2017, 5:27 p.m., Boglarka Egyed wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/58233/
> > -----------------------------------------------------------
> >
> > (Updated April 6, 2017, 5:27 p.m.)
> >
> >
> > Review request for Sqoop, Anna Szonyi, Szabolcs Vasas, and Liz Szilagyi.
> >
> >
> > Bugs: SQOOP-3167
> > https://issues.apache.org/jira/browse/SQOOP-3167
> >
> >
> > Repository: sqoop-trunk
> >
> >
> > Description
> > -------
> >
> > Automated SQLServer Manual tests including test case correction and minor rework too:
> > - modified connection string setup to make them able to run automatically
> > - fixed failing test cases
> > - excluded invalid test cases
> > - added database cleanup logic in tearDown part
> > - updated java docs
> > - removed unused imports
> > - changing names to add them to the 3rd party test suite
> >
> > Note: A more extensive refactor of the test classes in org.apache.sqoop.manager.sqlserver could be made, it will be addressed in another JIRA as that is a different scope.
> >
> >
> > Diffs
> > -----
> >
> > build.xml 73db28b272c50b4f76fef8421e6b9dfe5fed40f4
> > src/test/com/cloudera/sqoop/hive/TestHiveImport.java 33e0cc41f6f379bac2085431e0f1adc60bce6bce
> > src/test/com/cloudera/sqoop/manager/SQLServerManagerExportManualTest.java 9a92479245fa35c210d8e49f847292ee53d6f9b1
> > src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java 1f69725da8408853ac55b1f316ce1b9ef015e674
> > src/test/org/apache/sqoop/manager/sqlserver/MSSQLTestUtils.java 851bf49614e829d07de252b83f4ad550d0cb043b
> > src/test/org/apache/sqoop/manager/sqlserver/ManagerCompatExport.java 8c5176ad61aae61b96c7458d3b4b83dc11960268
> > src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeExportDelimitedFileManualTest.java 099d7344beb428c58b32d926af5ea079211da490
> > src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeExportSequenceFileManualTest.java 21676f02510693dcdd856a1d9dfba7d05eace023
> > src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeImportDelimitedFileManualTest.java 519fb525bdbb167520368d404667036669925041
> > src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeImportSequenceFileManualTest.java a0dad8a60b99d522ad3691e15b8b16c56e4b5858
> > src/test/org/apache/sqoop/manager/sqlserver/SQLServerHiveImportManualTest.java 1999272181421a539318ed195ea4257f52b2ed08
> > src/test/org/apache/sqoop/manager/sqlserver/SQLServerManagerManualTest.java 1178e3c79de4d0b5c7a96c6ad7eb316ed15e47c4
> > src/test/org/apache/sqoop/manager/sqlserver/SQLServerMultiColsManualTest.java 6a8ab51967237f471044b868615fdb3e057b1d92
> > src/test/org/apache/sqoop/manager/sqlserver/SQLServerMultiMapsManualTest.java c9a5b5ef596cfc1b28948c2a071935dfb9500cde
> > src/test/org/apache/sqoop/manager/sqlserver/SQLServerParseMethodsManualTest.java cd05aecf1ae5bd79fb485325d58b33a73e9df290
> > src/test/org/apache/sqoop/manager/sqlserver/SQLServerQueryManualTest.java 0057ac9df562c8e92cf7b9014c5e4239886a8104
> > src/test/org/apache/sqoop/manager/sqlserver/SQLServerSplitByManualTest.java f85245ab8cdd66da983ac9017d356f251f22e7db
> > src/test/org/apache/sqoop/manager/sqlserver/SQLServerWhereManualTest.java 10ae03b324b15f5ea0cc3cbbc04d3a5041233dd9
> >
> >
> > Diff: https://reviews.apache.org/r/58233/diff/2/
> >
> >
> > Testing
> > -------
> >
> > ant clean test, ant test
> >
> > ant clean test -Dthirdparty=true -Dsqoop.thirdparty.lib.dir=3rdpartylib -Dsqoop.test.sqlserver.connectstring.host_url=sqlserver -Dsqoop.test.sqlserver.database=databasename -Dms.sqlserver.username=username -Dms.sqlserver.password=password -Dtestcase=SQLServer*
> >
> > ant clean test -Dthirdparty=true -Dsqoop.thirdparty.lib.dir=3rdpartylib -Dsqoop.test.mysql.connectstring.host_url=mysql -Dsqoop.test.mysql.databasename=databasename -Dsqoop.test.mysql.password=password -Dsqoop.test.mysql.username=username -Dsqoop.test.oracle.connectstring=oracle -Dsqoop.test.postgresql.connectstring.host_url=postgre -Dsqoop.test.cubrid.connectstring.host_url=cubrid -Dsqoop.test.cubrid.connectstring.username=username -Dsqoop.test.cubrid.connectstring.database=database -Dsqoop.test.cubrid.connectstring.password=password -Dmapred.child.java.opts="-Djava.security.egd=file:/dev/../dev/urandom" -Dtest.timeout=1000000 -Dsqoop.test.sqlserver.connectstring.host_url=sqlserver -Dsqoop.test.sqlserver.database=database -Dms.sqlserver.username=username -Dms.sqlserver.password=password
> >
> >
> > Thanks,
> >
> > Boglarka Egyed
> >
> >
>
Re: Review Request 58233: SQOOP-3167: Evalutation and automation of
SQLServer Manual tests
Posted by Boglarka Egyed <eg...@gmail.com>.
> On April 7, 2017, 5:25 p.m., Attila Szabo wrote:
> > Hi Bogi, Liz,
> >
> > Most probably I did not phrase my thoughts precise enough in the previous round, so please let me express myself in a bit more direct and clear way:
> > My problem is not with the size of the patch file (although IMHO ~1000+ changed lines is quite a few), but with the fact that this change tries to achieve multiple things, which I think would make more sense as part of multiple issues/commits. Here are the things I've identified as the effects/consequences of this changeset:
> > - It delivers improvements/fixes around the way connections are handled in the MSSqlServer related tests, which is a great thing, makes the usage much more flexible.
> > -- Although around the deletes (were those codes really dead?)/fixes I'm not sure if those things would be in sync with the design goals of the SqlServer connector, but I'm not used to consider myself as a MSSqlServer expert.
> > - Pushes the SqlServer related manual tests into the third party ant test cycle. IMHO this is not the best decision in the current test + CI architecture, as from that moment this patch would have been committed, it would force every contributor to have a working MSSqlServer instance (on the top of the currently existing ones including MySQL, PostgreSQL, Oracle, Cubrid, etc.) on their dev infrastructure, or facing with the fact that some of the third party tests will fail continuously on their side (which does not sound like a best practice). Most probably on your side it didn't appear as a problem as you do have standalone instances for yourself, but we cannot depend on that this is true in case of every contributor. Although the test files themselves contains some very basic instructions how to make the tests work, but still in the current version it would need manual interactions+installs, thus won't work out of the box (needless to say that the instructions are very much not
detailed enough to someone who is not expert in installing MSSqlServer).
> > -- As a subpart of this section, I have to highlight that this way of changes alternates the intention of the original devs (and I'm pretty sure they had good reason why these tests were not automated but manual).
> > -- Introducing just another external dependency won't help to onboard new contributors (just another new -D param for a process which is not quite good documented already)
> > -- It also won't make the CI integration easier in the future
> > - The title+description JIRA is missleading as it does not solve the true automation (neither on CI level, nor on the side of the devs).
> >
> > So if you trust in my experience and wisdom as a committer you would consider splitting up this changeset into 2-3 parts, and only after pushing the tests into the 3rd party cycle, when the SqlServer deployment is solved out of the box (e.g. with docker or global apache installation or ansible or so).
> >
> > Please also consider fixing that part what I've identified as an issue
> >
> > My 2 cents,
> > Attila
>
> Attila Szabo wrote:
> I suggest discussing this on dev@sqoop list.
> If the PMC agrees on adding SqlServer integration tests to the current test suite, I'd be happy to merge your patch.
Thanks Attila for your verbose answer and suggestions, I discard this review and cancel the patch.
I have created another JIRA with a reduced scope, see SQOOP-3169
Regards,
Bogi
- Boglarka
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58233/#review171363
-----------------------------------------------------------
On April 6, 2017, 5:27 p.m., Boglarka Egyed wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58233/
> -----------------------------------------------------------
>
> (Updated April 6, 2017, 5:27 p.m.)
>
>
> Review request for Sqoop, Anna Szonyi, Szabolcs Vasas, and Liz Szilagyi.
>
>
> Bugs: SQOOP-3167
> https://issues.apache.org/jira/browse/SQOOP-3167
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Automated SQLServer Manual tests including test case correction and minor rework too:
> - modified connection string setup to make them able to run automatically
> - fixed failing test cases
> - excluded invalid test cases
> - added database cleanup logic in tearDown part
> - updated java docs
> - removed unused imports
> - changing names to add them to the 3rd party test suite
>
> Note: A more extensive refactor of the test classes in org.apache.sqoop.manager.sqlserver could be made, it will be addressed in another JIRA as that is a different scope.
>
>
> Diffs
> -----
>
> build.xml 73db28b272c50b4f76fef8421e6b9dfe5fed40f4
> src/test/com/cloudera/sqoop/hive/TestHiveImport.java 33e0cc41f6f379bac2085431e0f1adc60bce6bce
> src/test/com/cloudera/sqoop/manager/SQLServerManagerExportManualTest.java 9a92479245fa35c210d8e49f847292ee53d6f9b1
> src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java 1f69725da8408853ac55b1f316ce1b9ef015e674
> src/test/org/apache/sqoop/manager/sqlserver/MSSQLTestUtils.java 851bf49614e829d07de252b83f4ad550d0cb043b
> src/test/org/apache/sqoop/manager/sqlserver/ManagerCompatExport.java 8c5176ad61aae61b96c7458d3b4b83dc11960268
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeExportDelimitedFileManualTest.java 099d7344beb428c58b32d926af5ea079211da490
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeExportSequenceFileManualTest.java 21676f02510693dcdd856a1d9dfba7d05eace023
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeImportDelimitedFileManualTest.java 519fb525bdbb167520368d404667036669925041
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeImportSequenceFileManualTest.java a0dad8a60b99d522ad3691e15b8b16c56e4b5858
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerHiveImportManualTest.java 1999272181421a539318ed195ea4257f52b2ed08
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerManagerManualTest.java 1178e3c79de4d0b5c7a96c6ad7eb316ed15e47c4
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerMultiColsManualTest.java 6a8ab51967237f471044b868615fdb3e057b1d92
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerMultiMapsManualTest.java c9a5b5ef596cfc1b28948c2a071935dfb9500cde
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerParseMethodsManualTest.java cd05aecf1ae5bd79fb485325d58b33a73e9df290
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerQueryManualTest.java 0057ac9df562c8e92cf7b9014c5e4239886a8104
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerSplitByManualTest.java f85245ab8cdd66da983ac9017d356f251f22e7db
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerWhereManualTest.java 10ae03b324b15f5ea0cc3cbbc04d3a5041233dd9
>
>
> Diff: https://reviews.apache.org/r/58233/diff/2/
>
>
> Testing
> -------
>
> ant clean test, ant test
>
> ant clean test -Dthirdparty=true -Dsqoop.thirdparty.lib.dir=3rdpartylib -Dsqoop.test.sqlserver.connectstring.host_url=sqlserver -Dsqoop.test.sqlserver.database=databasename -Dms.sqlserver.username=username -Dms.sqlserver.password=password -Dtestcase=SQLServer*
>
> ant clean test -Dthirdparty=true -Dsqoop.thirdparty.lib.dir=3rdpartylib -Dsqoop.test.mysql.connectstring.host_url=mysql -Dsqoop.test.mysql.databasename=databasename -Dsqoop.test.mysql.password=password -Dsqoop.test.mysql.username=username -Dsqoop.test.oracle.connectstring=oracle -Dsqoop.test.postgresql.connectstring.host_url=postgre -Dsqoop.test.cubrid.connectstring.host_url=cubrid -Dsqoop.test.cubrid.connectstring.username=username -Dsqoop.test.cubrid.connectstring.database=database -Dsqoop.test.cubrid.connectstring.password=password -Dmapred.child.java.opts="-Djava.security.egd=file:/dev/../dev/urandom" -Dtest.timeout=1000000 -Dsqoop.test.sqlserver.connectstring.host_url=sqlserver -Dsqoop.test.sqlserver.database=database -Dms.sqlserver.username=username -Dms.sqlserver.password=password
>
>
> Thanks,
>
> Boglarka Egyed
>
>
Re: Review Request 58233: SQOOP-3167: Evalutation and automation of
SQLServer Manual tests
Posted by Attila Szabo <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58233/#review171363
-----------------------------------------------------------
Hi Bogi, Liz,
Most probably I did not phrase my thoughts precise enough in the previous round, so please let me express myself in a bit more direct and clear way:
My problem is not with the size of the patch file (although IMHO ~1000+ changed lines is quite a few), but with the fact that this change tries to achieve multiple things, which I think would make more sense as part of multiple issues/commits. Here are the things I've identified as the effects/consequences of this changeset:
- It delivers improvements/fixes around the way connections are handled in the MSSqlServer related tests, which is a great thing, makes the usage much more flexible.
-- Although around the deletes (were those codes really dead?)/fixes I'm not sure if those things would be in sync with the design goals of the SqlServer connector, but I'm not used to consider myself as a MSSqlServer expert.
- Pushes the SqlServer related manual tests into the third party ant test cycle. IMHO this is not the best decision in the current test + CI architecture, as from that moment this patch would have been committed, it would force every contributor to have a working MSSqlServer instance (on the top of the currently existing ones including MySQL, PostgreSQL, Oracle, Cubrid, etc.) on their dev infrastructure, or facing with the fact that some of the third party tests will fail continuously on their side (which does not sound like a best practice). Most probably on your side it didn't appear as a problem as you do have standalone instances for yourself, but we cannot depend on that this is true in case of every contributor. Although the test files themselves contains some very basic instructions how to make the tests work, but still in the current version it would need manual interactions+installs, thus won't work out of the box (needless to say that the instructions are very much not deta
iled enough to someone who is not expert in installing MSSqlServer).
-- As a subpart of this section, I have to highlight that this way of changes alternates the intention of the original devs (and I'm pretty sure they had good reason why these tests were not automated but manual).
-- Introducing just another external dependency won't help to onboard new contributors (just another new -D param for a process which is not quite good documented already)
-- It also won't make the CI integration easier in the future
- The title+description JIRA is missleading as it does not solve the true automation (neither on CI level, nor on the side of the devs).
So if you trust in my experience and wisdom as a committer you would consider splitting up this changeset into 2-3 parts, and only after pushing the tests into the 3rd party cycle, when the SqlServer deployment is solved out of the box (e.g. with docker or global apache installation or ansible or so).
Please also consider fixing that part what I've identified as an issue
My 2 cents,
Attila
src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeImportDelimitedFileManualTest.java
Lines 224-242 (original), 249-299 (patched)
<https://reviews.apache.org/r/58233/#comment244257>
Hi,
Could you please give me some explanation around these test cases?
For me it looks strange intentionally, that we're trying to leverage from some base/super test class, but nullifying some of it's test cases with an empty method body.
If these things are not neccessary I would suggest deleting them, as the current solution is very much misleading, b/c these cases won't look as 'invalid' as you marked them, but very much passed green test cases, giving the false intention these cases are very much supported in the delimited case as well.
If you need a common ancestor I would advise using 'extract superclass' refactoring.
- Attila Szabo
On April 6, 2017, 5:27 p.m., Boglarka Egyed wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58233/
> -----------------------------------------------------------
>
> (Updated April 6, 2017, 5:27 p.m.)
>
>
> Review request for Sqoop, Anna Szonyi, Szabolcs Vasas, and Liz Szilagyi.
>
>
> Bugs: SQOOP-3167
> https://issues.apache.org/jira/browse/SQOOP-3167
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Automated SQLServer Manual tests including test case correction and minor rework too:
> - modified connection string setup to make them able to run automatically
> - fixed failing test cases
> - excluded invalid test cases
> - added database cleanup logic in tearDown part
> - updated java docs
> - removed unused imports
> - changing names to add them to the 3rd party test suite
>
> Note: A more extensive refactor of the test classes in org.apache.sqoop.manager.sqlserver could be made, it will be addressed in another JIRA as that is a different scope.
>
>
> Diffs
> -----
>
> build.xml 73db28b272c50b4f76fef8421e6b9dfe5fed40f4
> src/test/com/cloudera/sqoop/hive/TestHiveImport.java 33e0cc41f6f379bac2085431e0f1adc60bce6bce
> src/test/com/cloudera/sqoop/manager/SQLServerManagerExportManualTest.java 9a92479245fa35c210d8e49f847292ee53d6f9b1
> src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java 1f69725da8408853ac55b1f316ce1b9ef015e674
> src/test/org/apache/sqoop/manager/sqlserver/MSSQLTestUtils.java 851bf49614e829d07de252b83f4ad550d0cb043b
> src/test/org/apache/sqoop/manager/sqlserver/ManagerCompatExport.java 8c5176ad61aae61b96c7458d3b4b83dc11960268
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeExportDelimitedFileManualTest.java 099d7344beb428c58b32d926af5ea079211da490
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeExportSequenceFileManualTest.java 21676f02510693dcdd856a1d9dfba7d05eace023
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeImportDelimitedFileManualTest.java 519fb525bdbb167520368d404667036669925041
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeImportSequenceFileManualTest.java a0dad8a60b99d522ad3691e15b8b16c56e4b5858
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerHiveImportManualTest.java 1999272181421a539318ed195ea4257f52b2ed08
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerManagerManualTest.java 1178e3c79de4d0b5c7a96c6ad7eb316ed15e47c4
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerMultiColsManualTest.java 6a8ab51967237f471044b868615fdb3e057b1d92
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerMultiMapsManualTest.java c9a5b5ef596cfc1b28948c2a071935dfb9500cde
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerParseMethodsManualTest.java cd05aecf1ae5bd79fb485325d58b33a73e9df290
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerQueryManualTest.java 0057ac9df562c8e92cf7b9014c5e4239886a8104
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerSplitByManualTest.java f85245ab8cdd66da983ac9017d356f251f22e7db
> src/test/org/apache/sqoop/manager/sqlserver/SQLServerWhereManualTest.java 10ae03b324b15f5ea0cc3cbbc04d3a5041233dd9
>
>
> Diff: https://reviews.apache.org/r/58233/diff/2/
>
>
> Testing
> -------
>
> ant clean test, ant test
>
> ant clean test -Dthirdparty=true -Dsqoop.thirdparty.lib.dir=3rdpartylib -Dsqoop.test.sqlserver.connectstring.host_url=sqlserver -Dsqoop.test.sqlserver.database=databasename -Dms.sqlserver.username=username -Dms.sqlserver.password=password -Dtestcase=SQLServer*
>
> ant clean test -Dthirdparty=true -Dsqoop.thirdparty.lib.dir=3rdpartylib -Dsqoop.test.mysql.connectstring.host_url=mysql -Dsqoop.test.mysql.databasename=databasename -Dsqoop.test.mysql.password=password -Dsqoop.test.mysql.username=username -Dsqoop.test.oracle.connectstring=oracle -Dsqoop.test.postgresql.connectstring.host_url=postgre -Dsqoop.test.cubrid.connectstring.host_url=cubrid -Dsqoop.test.cubrid.connectstring.username=username -Dsqoop.test.cubrid.connectstring.database=database -Dsqoop.test.cubrid.connectstring.password=password -Dmapred.child.java.opts="-Djava.security.egd=file:/dev/../dev/urandom" -Dtest.timeout=1000000 -Dsqoop.test.sqlserver.connectstring.host_url=sqlserver -Dsqoop.test.sqlserver.database=database -Dms.sqlserver.username=username -Dms.sqlserver.password=password
>
>
> Thanks,
>
> Boglarka Egyed
>
>
Re: Review Request 58233: SQOOP-3167: Evalutation and automation of
SQLServer Manual tests
Posted by Boglarka Egyed <eg...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58233/
-----------------------------------------------------------
(Updated April 6, 2017, 5:27 p.m.)
Review request for Sqoop, Anna Szonyi, Szabolcs Vasas, and Liz Szilagyi.
Changes
-------
Review finding corrections
Bugs: SQOOP-3167
https://issues.apache.org/jira/browse/SQOOP-3167
Repository: sqoop-trunk
Description
-------
Automated SQLServer Manual tests including test case correction and minor rework too:
- modified connection string setup to make them able to run automatically
- fixed failing test cases
- excluded invalid test cases
- added database cleanup logic in tearDown part
- updated java docs
- removed unused imports
- changing names to add them to the 3rd party test suite
Note: A more extensive refactor of the test classes in org.apache.sqoop.manager.sqlserver could be made, it will be addressed in another JIRA as that is a different scope.
Diffs (updated)
-----
build.xml 73db28b272c50b4f76fef8421e6b9dfe5fed40f4
src/test/com/cloudera/sqoop/hive/TestHiveImport.java 33e0cc41f6f379bac2085431e0f1adc60bce6bce
src/test/com/cloudera/sqoop/manager/SQLServerManagerExportManualTest.java 9a92479245fa35c210d8e49f847292ee53d6f9b1
src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java 1f69725da8408853ac55b1f316ce1b9ef015e674
src/test/org/apache/sqoop/manager/sqlserver/MSSQLTestUtils.java 851bf49614e829d07de252b83f4ad550d0cb043b
src/test/org/apache/sqoop/manager/sqlserver/ManagerCompatExport.java 8c5176ad61aae61b96c7458d3b4b83dc11960268
src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeExportDelimitedFileManualTest.java 099d7344beb428c58b32d926af5ea079211da490
src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeExportSequenceFileManualTest.java 21676f02510693dcdd856a1d9dfba7d05eace023
src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeImportDelimitedFileManualTest.java 519fb525bdbb167520368d404667036669925041
src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeImportSequenceFileManualTest.java a0dad8a60b99d522ad3691e15b8b16c56e4b5858
src/test/org/apache/sqoop/manager/sqlserver/SQLServerHiveImportManualTest.java 1999272181421a539318ed195ea4257f52b2ed08
src/test/org/apache/sqoop/manager/sqlserver/SQLServerManagerManualTest.java 1178e3c79de4d0b5c7a96c6ad7eb316ed15e47c4
src/test/org/apache/sqoop/manager/sqlserver/SQLServerMultiColsManualTest.java 6a8ab51967237f471044b868615fdb3e057b1d92
src/test/org/apache/sqoop/manager/sqlserver/SQLServerMultiMapsManualTest.java c9a5b5ef596cfc1b28948c2a071935dfb9500cde
src/test/org/apache/sqoop/manager/sqlserver/SQLServerParseMethodsManualTest.java cd05aecf1ae5bd79fb485325d58b33a73e9df290
src/test/org/apache/sqoop/manager/sqlserver/SQLServerQueryManualTest.java 0057ac9df562c8e92cf7b9014c5e4239886a8104
src/test/org/apache/sqoop/manager/sqlserver/SQLServerSplitByManualTest.java f85245ab8cdd66da983ac9017d356f251f22e7db
src/test/org/apache/sqoop/manager/sqlserver/SQLServerWhereManualTest.java 10ae03b324b15f5ea0cc3cbbc04d3a5041233dd9
Diff: https://reviews.apache.org/r/58233/diff/2/
Changes: https://reviews.apache.org/r/58233/diff/1-2/
Testing
-------
ant clean test, ant test
ant clean test -Dthirdparty=true -Dsqoop.thirdparty.lib.dir=3rdpartylib -Dsqoop.test.sqlserver.connectstring.host_url=sqlserver -Dsqoop.test.sqlserver.database=databasename -Dms.sqlserver.username=username -Dms.sqlserver.password=password -Dtestcase=SQLServer*
ant clean test -Dthirdparty=true -Dsqoop.thirdparty.lib.dir=3rdpartylib -Dsqoop.test.mysql.connectstring.host_url=mysql -Dsqoop.test.mysql.databasename=databasename -Dsqoop.test.mysql.password=password -Dsqoop.test.mysql.username=username -Dsqoop.test.oracle.connectstring=oracle -Dsqoop.test.postgresql.connectstring.host_url=postgre -Dsqoop.test.cubrid.connectstring.host_url=cubrid -Dsqoop.test.cubrid.connectstring.username=username -Dsqoop.test.cubrid.connectstring.database=database -Dsqoop.test.cubrid.connectstring.password=password -Dmapred.child.java.opts="-Djava.security.egd=file:/dev/../dev/urandom" -Dtest.timeout=1000000 -Dsqoop.test.sqlserver.connectstring.host_url=sqlserver -Dsqoop.test.sqlserver.database=database -Dms.sqlserver.username=username -Dms.sqlserver.password=password
Thanks,
Boglarka Egyed