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



Hi Nguyen,

Many thanks for updating the patch, I have one last command regarding the 
documentation part, please find it below.

Thank you,
Bogi


src/docs/dev/test-categories-definitions.txt
Lines 1 (patched)
<https://reviews.apache.org/r/68541/#comment292416>

    This file should have been included into 
sqoop/src/docs/dev/SqoopDevGuide.txt so that ant docs and gradle docs generate 
it to the the SqoopDevGuide html too.
    However, after thinking it through, I think it makes no sense to have these 
definitions written here and in the interface's javadocs too as it would mean 
duplicated documentation that will be failed to maintain parallel at some point 
in the future.
    So I would rather suggest to:
    - keep the definitions in the javadocs, as you have already added them in 
your new patch,
    - remove this (test-categories-definitions.txt) file.


- Boglarka Egyed


On Sept. 8, 2018, 2:57 p.m., Nguyen Truong wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68541/
> -----------------------------------------------------------
> 
> (Updated Sept. 8, 2018, 2:57 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/docs/dev/test-categories-definitions.txt PRE-CREATION 
>   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/HBaseImportAddRowKeyTest.java c4caafba5 
>   src/test/org/apache/sqoop/hbase/HBaseKerberizedConnectivityTest.java 
> 3bfb39178 
>   src/test/org/apache/sqoop/hbase/HBaseUtilTest.java c6a808c33 
>   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/importjob/avro/AvroImportForNumericTypesTest.java 
> ff13dc3bc 
>   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/util/BlockJUnit4ClassRunnerWithParameters.java 
> PRE-CREATION 
>   
> src/test/org/apache/sqoop/util/BlockJUnit4ClassRunnerWithParametersFactory.java
>  PRE-CREATION 
>   src/test/org/apache/sqoop/util/TestDirCleanupHook.java 41146fd17 
>   src/test/org/apache/sqoop/util/TestFileSystemUtil.java 28fb58ceb 
>   src/test/org/apache/sqoop/util/TestMainframeFTPClientUtils.java 90a85194c 
>   src/test/org/apache/sqoop/util/TestOptionsFileExpansion.java 3fc9e6005 
>   src/test/org/apache/sqoop/util/TestSqoopJsonUtil.java fdf972c11 
>   src/test/org/apache/sqoop/util/TestSubstitutionUtils.java a2ac3410a 
>   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/2/
> 
> 
> 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
> Test order in my machine
>   
> https://reviews.apache.org/media/uploaded/files/2018/09/08/4af4030f-1230-402b-99c7-51d2d37da524__SqoopTestOrder.txt
> 
> 
> Thanks,
> 
> Nguyen Truong
> 
>

Reply via email to