sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Boglarka Egyed <b...@apache.org>
Subject Re: Review Request 68541: SQOOP-3104: Create test categories instead of test suites and naming conventions
Date Fri, 07 Sep 2018 10:03:05 GMT


> On Aug. 31, 2018, 1:24 p.m., Boglarka Egyed wrote:
> > Hi Nguyen,
> > 
> > Many thanks for taking care of this huge effort! Having well categorized tests is
a long-awaited improvement Sqoop, it will ease the life of every developer.
> > 
> > In general your change looks good to me however I have three findings:
> > 
> > 1.) New test classes have been introduced recently in SQOOP-3348, SQOOP-3363 and
SQOOP-3368 testing the Sqoop-S3 integration. I think it would make sense to have a new test
category for the S3 related tests similarly to the KerberizedTest category.
> > 
> > 2.) There should be well defined, thorough definitions of each test categories and
these should be well documented somewhere. This is something that could be a result of a discussion
amongst the contributors. After creating such definitions they could be added as javadocs
to the test category interfaces as well as included in the Sqoop Developer’s Guide (changes
would be needed under sqoop/src/docs/dev).
> > 
> > 3.) It would be also very useful to have exact commands described to run specific
categories in the COMPILING.txt.
> > 
> > Thank you very much,
> > Bogi
> 
> Nguyen Truong wrote:
>     Hi Boglarka,
>     
>     Thanks for your review. I will go on and add categories for the new tests.
>     
>     I don't know about the S3 tests. Could you please let me know more why they should
be under a separate category as KerberizedTest? Thank you very much.
>     
>     I will update the COMPILING.txt after we come up with the final decision on the categories.
>     
>     Thanks,
>     Nguyen
> 
> Boglarka Egyed wrote:
>     S3 tests should be in a separate test category as they test the integration with
the Amazon S3 cloud service that is a third party side just like an RDBMS. These tests also
require AWS credentials to access S3 and they run only if these credentials are provided via
the -Ds3.generator.command=<credential-generator-command> property as well as the target
S3 location via the -Ds3.bucket.url=<bucket-url> property.
>     The S3 tests that should be in a separate category are under sqoop/src/test/org/apache/sqoop/s3/
>     The S3 related test case added in SQOOP-3368 is a simple unit test case though, it
doesn't test the integration with S3 just the sqoop option validation mechanism.
>     
>     I will also think about the definitions of the test categories and will share my
thoughts later.

Hey Natalie,

I think I ran a bit forward with my previous comments. Let's skip the S3 category now, it
can be added in a later patch as well. Please ignore my words above.

We had a discussion with Szabolcs and Fero and here is our recommendation for the test categorie
definitions:

SqoopTest
---------
UnitTest:
A unit test shall test one class at a time having it's dependencies mocked.
A unit test shall not start a mini cluster nor an embedded database and it shall not use a
JDBC driver.

IntegrationTest:
An integration test shall test if independently developed classes work together correctly.
An integration test checks a whole scenario and thus may start mini clusters or embedded databases
and may connect to external resources like RDBMS instances.

ManualTest:
Deprecated category, shall not be used nor extended.

ThirdPartyTest
--------------
A third party test shall test a scenario where a third party side is required such as a JDBC
driver or an external RDBMS instance.

CubridTest:
A CubridTest shall test scenarios where a Cubrid driver and/or external instance is required.

Db2Test:
A DB2 test shall test scenarios where a DB2 driver and/or external instance is required.

MainFrameTest:
A MainFrame test shall test scenarios where a MainFrame external instance is required.

MysqlTest:
A MySql test shall test scenarios where a MySql driver and/or external instance is required.

NetezzaTest:
A Netezza test shall test scenarios where a Netezza driver and/or external instance is required.

OracleTest:
An Oracle test shall test scenarios where a Oracle driver and/or external instance is required.

PostgresqlTest:
A PostgreSql test shall test scenarios where a PostgreSql driver and/or external instance
is required.

SqlServerTest:
An SqlServer test shall test scenarios where a SqlServer driver and/or external instance is
required.

KerberizedTest
--------------
A kerberized test shall run in kerberized environment thus it starts mini KDC server.


These also can be refined and/or extended later but could be a good start for now.

What do you think?

Thanks,
Bogi


- Boglarka


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


On Aug. 28, 2018, 3:52 p.m., Nguyen Truong wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68541/
> -----------------------------------------------------------
> 
> (Updated Aug. 28, 2018, 3:52 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3104
>     https://issues.apache.org/jira/browse/SQOOP-3104
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> We are currently unsing test naming conventions to differentiate between ManualTests,
Unit tests and 3rd party tests. Instead of that, I implemented junit categories which will
allow us to have more categories in the future. This would also remove the reliance on the
test class name.
> 
> Test categories skeleton:
>       SqoopTest _____ UnitTest
>                   |__ IntegrationTest
>                   |__ ManualTest
> 
>       ThirdPartyTest _____ CubridTest
>                        |__ Db2Test
>                        |__ MainFrameTest
>                        |__ MysqlTest
>                        |__ NetezzaTest
>                        |__ OracleTest
>                        |__ PostgresqlTest
>                        |__ SqlServerTest
> 
>       KerberizedTest
> 
> Categories explanation:
>     * SqoopTest: Group of the big categories, including:
>         - UnitTest: It tests one class only with its dependencies mocked or if the dependency
>         is lightweight we can keep it. It must not start a minicluster or an hsqldb database.
>         It does not need JCDB drivers.
>         - IntegrationTest: It usually tests a whole scenario. It may start up miniclusters,
>         hsqldb and connect to external resources like RDBMSs.
>         - ManualTest: This should be a deprecated category which should not be used in
the future.
>         It only exists to mark the currently existing manual tests.
>     * ThirdPartyTest: An orthogonal hierarchy for tests that need a JDBC driver and/or
a docker
>     container/external RDBMS instance to run. Subcategories express what kind of external
>     resource the test needs. E.g: OracleTest needs an Oracle RDBMS and Oracle driver
on the classpath
>     * KerberizedTest: Test that needs Kerberos, which needs to be run on a separate JVM.
> 
> Opinions are very welcomed. Thanks!
> 
> 
> Diffs
> -----
> 
>   build.gradle fc7fc0c4c 
>   src/test/org/apache/sqoop/TestConnFactory.java fb6c94059 
>   src/test/org/apache/sqoop/TestIncrementalImport.java 29c477954 
>   src/test/org/apache/sqoop/TestSqoopOptions.java e55682edf 
>   src/test/org/apache/sqoop/accumulo/TestAccumuloUtil.java 631eeff5e 
>   src/test/org/apache/sqoop/authentication/TestKerberosAuthenticator.java f5700ce65 
>   src/test/org/apache/sqoop/db/TestDriverManagerJdbcConnectionFactory.java 244831672

>   src/test/org/apache/sqoop/db/decorator/TestKerberizedConnectionFactoryDecorator.java
d3e3fb23e 
>   src/test/org/apache/sqoop/hbase/HBaseKerberizedConnectivityTest.java 3bfb39178 
>   src/test/org/apache/sqoop/hbase/TestHBasePutProcessor.java e78a535f4 
>   src/test/org/apache/sqoop/hcat/TestHCatalogBasic.java ba05cabbb 
>   src/test/org/apache/sqoop/hive/HiveServer2ConnectionFactoryInitializerTest.java 4d2cb2f88

>   src/test/org/apache/sqoop/hive/TestHiveClientFactory.java a3c2dc939 
>   src/test/org/apache/sqoop/hive/TestHiveMiniCluster.java 419f888c0 
>   src/test/org/apache/sqoop/hive/TestHiveServer2Client.java 02617295e 
>   src/test/org/apache/sqoop/hive/TestHiveServer2ParquetImport.java b55179a4f 
>   src/test/org/apache/sqoop/hive/TestHiveServer2TextImport.java 410724f37 
>   src/test/org/apache/sqoop/hive/TestHiveTypesForAvroTypeMapping.java 276e9eaa4 
>   src/test/org/apache/sqoop/hive/TestTableDefWriter.java 626ad22f6 
>   src/test/org/apache/sqoop/hive/TestTableDefWriterForExternalTable.java f1768ee76 
>   src/test/org/apache/sqoop/io/TestCodecMap.java e71921823 
>   src/test/org/apache/sqoop/io/TestLobFile.java 2bc95f283 
>   src/test/org/apache/sqoop/io/TestNamedFifo.java a93784e08 
>   src/test/org/apache/sqoop/io/TestSplittableBufferedWriter.java c59aa26ad 
>   src/test/org/apache/sqoop/lib/TestBlobRef.java b271d3c7b 
>   src/test/org/apache/sqoop/lib/TestBooleanParser.java 914ab37e4 
>   src/test/org/apache/sqoop/lib/TestClobRef.java f94d1a8af 
>   src/test/org/apache/sqoop/lib/TestFieldFormatter.java 9ac55e703 
>   src/test/org/apache/sqoop/lib/TestLargeObjectLoader.java 1e07d7174 
>   src/test/org/apache/sqoop/lib/TestRecordParser.java d6844c1cf 
>   src/test/org/apache/sqoop/manager/TestDefaultManagerFactory.java 8e1632430 
>   src/test/org/apache/sqoop/manager/TestMainframeManager.java c84f05f66 
>   src/test/org/apache/sqoop/manager/TestSqlManager.java 185f5a7a1 
>   src/test/org/apache/sqoop/manager/cubrid/CubridAuthTest.java 82fac12e3 
>   src/test/org/apache/sqoop/manager/cubrid/CubridCompatTest.java 8a075e87d 
>   src/test/org/apache/sqoop/manager/cubrid/CubridManagerExportTest.java 4de8e40fd 
>   src/test/org/apache/sqoop/manager/cubrid/CubridManagerImportTest.java addf1aeec 
>   src/test/org/apache/sqoop/manager/db2/DB2ImportAllTableWithSchemaManualTest.java d1a6d6926

>   src/test/org/apache/sqoop/manager/db2/DB2ManagerImportManualTest.java b5d47f2ed 
>   src/test/org/apache/sqoop/manager/db2/DB2XmlTypeImportManualTest.java 393a110fb 
>   src/test/org/apache/sqoop/manager/hsqldb/TestHsqldbManager.java 745a8125d 
>   src/test/org/apache/sqoop/manager/mainframe/MainframeManagerImportTest.java 3b8ed2361

>   src/test/org/apache/sqoop/manager/mysql/DirectMySQLExportTest.java b3570ff1f 
>   src/test/org/apache/sqoop/manager/mysql/DirectMySQLTest.java 89a7fec6e 
>   src/test/org/apache/sqoop/manager/mysql/JdbcMySQLExportTest.java f655bcc8a 
>   src/test/org/apache/sqoop/manager/mysql/MySQLAllTablesTest.java baf0e2a71 
>   src/test/org/apache/sqoop/manager/mysql/MySQLAuthTest.java 1e2f70d23 
>   src/test/org/apache/sqoop/manager/mysql/MySQLCompatTest.java 7e822e66f 
>   src/test/org/apache/sqoop/manager/mysql/MySQLFreeFormQueryTest.java f4f0b7415 
>   src/test/org/apache/sqoop/manager/mysql/MySQLLobAvroImportTest.java 6208975fc 
>   src/test/org/apache/sqoop/manager/mysql/MySqlCallExportTest.java 22a66761c 
>   src/test/org/apache/sqoop/manager/mysql/MySqlColumnEscapeImportTest.java eaab8c578

>   src/test/org/apache/sqoop/manager/netezza/DirectNetezzaExportManualTest.java 0a6997fa3

>   src/test/org/apache/sqoop/manager/netezza/DirectNetezzaHCatExportManualTest.java 9365ba0f3

>   src/test/org/apache/sqoop/manager/netezza/DirectNetezzaHCatImportManualTest.java c05b73332

>   src/test/org/apache/sqoop/manager/netezza/NetezzaExportManualTest.java 95abe7a6e 
>   src/test/org/apache/sqoop/manager/netezza/NetezzaImportManualTest.java 4002c647a 
>   src/test/org/apache/sqoop/manager/oracle/OraOopDataDrivenDBInputFormatConnectionCloseTest.java
bb33c3547 
>   src/test/org/apache/sqoop/manager/oracle/OraOopTestCase.java 1bae71cb0 
>   src/test/org/apache/sqoop/manager/oracle/OracleCallExportTest.java 6d6602a7b 
>   src/test/org/apache/sqoop/manager/oracle/OracleColumnEscapeImportTest.java 684586c8d

>   src/test/org/apache/sqoop/manager/oracle/OracleCompatTest.java 553096a87 
>   src/test/org/apache/sqoop/manager/oracle/OracleExportTest.java a880af36d 
>   src/test/org/apache/sqoop/manager/oracle/OracleFreeFormQueryTest.java bb3e7c460 
>   src/test/org/apache/sqoop/manager/oracle/OracleIncrementalImportTest.java 8e6ccc96a

>   src/test/org/apache/sqoop/manager/oracle/OracleLobAvroImportTest.java 525ccf457 
>   src/test/org/apache/sqoop/manager/oracle/OracleManagerTest.java 9251f0266 
>   src/test/org/apache/sqoop/manager/oracle/OracleSpecialCharacterTableImportTest.java
6539e5a9b 
>   src/test/org/apache/sqoop/manager/oracle/OracleSplitterTest.java c2f9532c1 
>   src/test/org/apache/sqoop/manager/postgresql/DirectPostgreSQLExportManualTest.java
22b202a20 
>   src/test/org/apache/sqoop/manager/postgresql/PGBulkloadManagerManualTest.java 8855316c8

>   src/test/org/apache/sqoop/manager/postgresql/PostgresqlExportTest.java f86b1192c 
>   src/test/org/apache/sqoop/manager/postgresql/PostgresqlExternalTableImportTest.java
dd4cfb48b 
>   src/test/org/apache/sqoop/manager/postgresql/PostgresqlImportTest.java b8aa17b03 
>   src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeExportDelimitedFileTest.java
9b70af181 
>   src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeExportSequenceFileTest.java
293da0002 
>   src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeImportDelimitedFileTest.java
520c4ac8b 
>   src/test/org/apache/sqoop/manager/sqlserver/SQLServerDatatypeImportSequenceFileTest.java
592a78f22 
>   src/test/org/apache/sqoop/manager/sqlserver/SQLServerHiveImportTest.java e6b086550

>   src/test/org/apache/sqoop/manager/sqlserver/SQLServerManagerExportTest.java b7c2b75d6

>   src/test/org/apache/sqoop/manager/sqlserver/SQLServerManagerImportTest.java 79e37f08f

>   src/test/org/apache/sqoop/manager/sqlserver/SQLServerManagerTest.java fdf856be1 
>   src/test/org/apache/sqoop/manager/sqlserver/SQLServerMultiColsTest.java fb765fb83 
>   src/test/org/apache/sqoop/manager/sqlserver/SQLServerMultiMapsTest.java 5e89cc9a9 
>   src/test/org/apache/sqoop/manager/sqlserver/SQLServerParseMethodsTest.java fbd8d9633

>   src/test/org/apache/sqoop/manager/sqlserver/SQLServerQueryTest.java e0c8d6724 
>   src/test/org/apache/sqoop/manager/sqlserver/SQLServerSplitByTest.java a1c220192 
>   src/test/org/apache/sqoop/manager/sqlserver/SQLServerWhereTest.java 11d0963f2 
>   src/test/org/apache/sqoop/manager/sqlserver/TestSqlServerManagerContextConfigurator.java
c0d0a248f 
>   src/test/org/apache/sqoop/mapreduce/TestJdbcExportJob.java 81ab6772d 
>   src/test/org/apache/sqoop/mapreduce/TestJobBase.java e1781bb63 
>   src/test/org/apache/sqoop/mapreduce/db/TestBigDecimalSplitter.java 951a3dc55 
>   src/test/org/apache/sqoop/mapreduce/db/TestDBConfiguration.java 3160db956 
>   src/test/org/apache/sqoop/mapreduce/db/TestDataDrivenDBInputFormat.java 9e538fd91 
>   src/test/org/apache/sqoop/mapreduce/db/TestIntegerSplitter.java b43fc41ff 
>   src/test/org/apache/sqoop/mapreduce/db/TestSQLServerDBRecordReader.java 70187b17e 
>   src/test/org/apache/sqoop/mapreduce/db/TestTextSplitter.java 5d9cdf05b 
>   src/test/org/apache/sqoop/mapreduce/db/TextSplitterHadoopConfIntegrationTest.java 9eb8922d5

>   src/test/org/apache/sqoop/mapreduce/hcat/TestSqoopHCatImportHelper.java 3f734ea34 
>   src/test/org/apache/sqoop/mapreduce/hcat/TestSqoopHCatUtilities.java dff11f1e5 
>   src/test/org/apache/sqoop/mapreduce/mainframe/TestMainframeDatasetFTPRecordReader.java
3547294fc 
>   src/test/org/apache/sqoop/mapreduce/mainframe/TestMainframeDatasetInputFormat.java
efef056bc 
>   src/test/org/apache/sqoop/mapreduce/mainframe/TestMainframeDatasetInputSplit.java 5d92f6d51

>   src/test/org/apache/sqoop/mapreduce/mainframe/TestMainframeDatasetPath.java 9b277b2ac

>   src/test/org/apache/sqoop/mapreduce/mainframe/TestMainframeFTPFileEntryParser.java
eb0f8c009 
>   src/test/org/apache/sqoop/mapreduce/mainframe/TestMainframeImportJob.java be62efd04

>   src/test/org/apache/sqoop/mapreduce/sqlserver/SqlServerUpsertOutputFormatTest.java
a89e8005e 
>   src/test/org/apache/sqoop/metastore/PasswordRedactorTest.java a2dbc7185 
>   src/test/org/apache/sqoop/metastore/SavedJobsTestBase.java 9c9b2f441 
>   src/test/org/apache/sqoop/metastore/TestAutoGenericJobStorage.java d5424c6fc 
>   src/test/org/apache/sqoop/metastore/TestGenericJobStorage.java 026fbdee4 
>   src/test/org/apache/sqoop/metastore/TestGenericJobStorageValidate.java 9995a425e 
>   src/test/org/apache/sqoop/metastore/TestMetastoreConfigurationParameters.java 5a6fac569

>   src/test/org/apache/sqoop/metastore/db2/DB2JobToolTest.java b2b1fb62a 
>   src/test/org/apache/sqoop/metastore/db2/DB2MetaConnectIncrementalImportTest.java e7969faaa

>   src/test/org/apache/sqoop/metastore/db2/DB2SavedJobsTest.java caf753c81 
>   src/test/org/apache/sqoop/metastore/mysql/MySqlJobToolTest.java 2ec9648fc 
>   src/test/org/apache/sqoop/metastore/mysql/MySqlMetaConnectIncrementalImportTest.java
e19bbc831 
>   src/test/org/apache/sqoop/metastore/mysql/MySqlSavedJobsTest.java e15c322d5 
>   src/test/org/apache/sqoop/metastore/oracle/OracleJobToolTest.java a3e61e98b 
>   src/test/org/apache/sqoop/metastore/oracle/OracleMetaConnectIncrementalImportTest.java
37beaa44c 
>   src/test/org/apache/sqoop/metastore/oracle/OracleSavedJobsTest.java 4691530ff 
>   src/test/org/apache/sqoop/metastore/postgres/PostgresJobToolTest.java 065e1bbdb 
>   src/test/org/apache/sqoop/metastore/postgres/PostgresMetaConnectIncrementalImportTest.java
0ffbf5ad7 
>   src/test/org/apache/sqoop/metastore/postgres/PostgresSavedJobsTest.java ee3f00564 
>   src/test/org/apache/sqoop/metastore/sqlserver/SqlServerJobToolTest.java 87d7b3433 
>   src/test/org/apache/sqoop/metastore/sqlserver/SqlServerMetaConnectIncrementalImportTest.java
f1a2a662a 
>   src/test/org/apache/sqoop/metastore/sqlserver/SqlServerSavedJobsTest.java b37623b82

>   src/test/org/apache/sqoop/orm/TestClassWriter.java 0cc07cf88 
>   src/test/org/apache/sqoop/orm/TestCompilationManager.java abd72d850 
>   src/test/org/apache/sqoop/testcategories/KerberizedTest.java PRE-CREATION 
>   src/test/org/apache/sqoop/testcategories/sqooptest/IntegrationTest.java PRE-CREATION

>   src/test/org/apache/sqoop/testcategories/sqooptest/ManualTest.java PRE-CREATION 
>   src/test/org/apache/sqoop/testcategories/sqooptest/SqoopTest.java PRE-CREATION 
>   src/test/org/apache/sqoop/testcategories/sqooptest/UnitTest.java PRE-CREATION 
>   src/test/org/apache/sqoop/testcategories/thirdpartytest/CubridTest.java PRE-CREATION

>   src/test/org/apache/sqoop/testcategories/thirdpartytest/Db2Test.java PRE-CREATION 
>   src/test/org/apache/sqoop/testcategories/thirdpartytest/MainFrameTest.java PRE-CREATION

>   src/test/org/apache/sqoop/testcategories/thirdpartytest/MysqlTest.java PRE-CREATION

>   src/test/org/apache/sqoop/testcategories/thirdpartytest/NetezzaTest.java PRE-CREATION

>   src/test/org/apache/sqoop/testcategories/thirdpartytest/OracleTest.java PRE-CREATION

>   src/test/org/apache/sqoop/testcategories/thirdpartytest/PostgresqlTest.java PRE-CREATION

>   src/test/org/apache/sqoop/testcategories/thirdpartytest/SqlServerTest.java PRE-CREATION

>   src/test/org/apache/sqoop/testcategories/thirdpartytest/ThirdPartyTest.java PRE-CREATION

>   src/test/org/apache/sqoop/testutil/BaseSqoopTestCase.java fe6ba8311 
>   src/test/org/apache/sqoop/testutil/TestArgumentArrayBuilder.java 6d701ab94 
>   src/test/org/apache/sqoop/tool/ImportToolValidateOptionsTest.java bdac437f1 
>   src/test/org/apache/sqoop/tool/TestBaseSqoopTool.java 5571b25a1 
>   src/test/org/apache/sqoop/tool/TestExportToolValidateOptions.java f16d1873e 
>   src/test/org/apache/sqoop/tool/TestHiveServer2OptionValidations.java ed4b5a498 
>   src/test/org/apache/sqoop/tool/TestImportTool.java 8c2be3bab 
>   src/test/org/apache/sqoop/tool/TestToolPlugin.java 19dea221a 
>   src/test/org/apache/sqoop/tool/TestValidateImportOptions.java 9b61bd5d9 
>   src/test/org/apache/sqoop/validation/AbortOnFailureHandlerTest.java ee04563c4 
>   src/test/org/apache/sqoop/validation/AbsoluteValidationThresholdTest.java 86a99c445

> 
> 
> Diff: https://reviews.apache.org/r/68541/diff/1/
> 
> 
> Testing
> -------
> 
> Ran unit tests, integration plain tests, and third party tests.
> You can run unit tests and integration plain tests together with ./gradlew test
> or separately with ./gradlew unitTest and ./gradlew integrationPlainTest
> 
> 
> File Attachments
> ----------------
> 
> SQOOP-3104.patch
>   https://reviews.apache.org/media/uploaded/files/2018/08/28/7058a562-ccef-4ca3-8b58-bd6a6e6ec377__SQOOP-3104.patch
> 
> 
> Thanks,
> 
> Nguyen Truong
> 
>


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