sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Boglarka Egyed <egyedb...@gmail.com>
Subject Re: Review Request 58233: SQOOP-3167: Evalutation and automation of SQLServer Manual tests
Date Fri, 07 Apr 2017 10:03:24 GMT


> 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
> 
>


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