----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29499/#review66608 -----------------------------------------------------------
Overall the patch looks good, thank you for taking a stab at this big change Abe! In addition to Veena's comments I have few high level notes on my own: 1) Do we need to change pre-commit build? Based on the changes it seems to me that the answer should be "no" as by default we will run "all" the tests, right? For reference, here are the commands that we're using in pre-commit build - "mvn test" for unit tests [1] and "integration-test -pl test" for integration tests [2]. 2) The patch no longer applies cleanly as I've just committed bunch of patches (PostgreSQL repo is the biggest one). I think that it will be hard to find a window where this patch will apply cleanly and at the same time no patches in the review queue gets affected. I was thinking how to address this and here is one idea that I came up with. What about pinging dev@ that we're doing JUnit -> TestNG migration on given day and within given 2-3 hours (pretty much a "maitenance window"). All patches before this window are expected to have JUnit tests. On the day&hour we will migrate the remaining committed changes (e.g. finish this patch) and commit it (e.g. do the migration). All patches after this window would be expected to use TestNG instead of JUnit. I'm fully expecting that couple of patches will try to get into the window and will miss it, so I would expect that there will be still some distruption, but hopefully it will be limited and contained. What do you think? Links: 1: https://github.com/apache/sqoop/blob/sqoop2/dev-support/test-patch.py#L259 2: https://github.com/apache/sqoop/blob/sqoop2/dev-support/test-patch.py#L262 - Jarek Cecho On Dec. 31, 2014, 1:52 a.m., Abraham Elmahrek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29499/ > ----------------------------------------------------------- > > (Updated Dec. 31, 2014, 1:52 a.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-1950 > https://issues.apache.org/jira/browse/SQOOP-1950 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > Use TestNG instead of JUnit. Major changes include: > # Imports were changed from org.junit.* to org.testng.* > # Parameterized tests use DataProvider now > # Change PowerMock methods and annotations to TestNG specific methods and > annotations > > > Diffs > ----- > > client/pom.xml d424e8d > client/src/test/java/org/apache/sqoop/client/TestSqoopClient.java 18b6132 > common-test/pom.xml 609a875 > > common-test/src/main/java/org/apache/sqoop/common/test/asserts/ProviderAsserts.java > 8196fe2 > > common-test/src/main/java/org/apache/sqoop/common/test/categories/SlowTests.java > f8ce474 > common/pom.xml e8a8010 > common/src/test/java/org/apache/sqoop/common/TestMapContext.java 7ce1ccd > common/src/test/java/org/apache/sqoop/common/TestSqoopResponseCode.java > f556c1c > common/src/test/java/org/apache/sqoop/common/TestSupportedDirections.java > 4f0cdd6 > common/src/test/java/org/apache/sqoop/common/TestVersionInfo.java 43575da > common/src/test/java/org/apache/sqoop/json/TestConnectorBean.java 44d7dd9 > common/src/test/java/org/apache/sqoop/json/TestConnectorsBean.java b4370e6 > common/src/test/java/org/apache/sqoop/json/TestDriverBean.java b72c432 > common/src/test/java/org/apache/sqoop/json/TestJSONUtils.java 7efee21 > common/src/test/java/org/apache/sqoop/json/TestJobBean.java 98cfee8 > common/src/test/java/org/apache/sqoop/json/TestJobsBean.java 42fbf9d > common/src/test/java/org/apache/sqoop/json/TestLinkBean.java 704e4da > common/src/test/java/org/apache/sqoop/json/TestLinksBean.java 9633c5e > common/src/test/java/org/apache/sqoop/json/TestSubmissionBean.java 24c0b34 > common/src/test/java/org/apache/sqoop/json/TestThrowableBean.java a67283c > common/src/test/java/org/apache/sqoop/json/TestValidationResultBean.java > c119e02 > > common/src/test/java/org/apache/sqoop/json/util/TestConfigSerialization.java > 9ae764a > > common/src/test/java/org/apache/sqoop/json/util/TestSchemaSerialization.java > 9031c99 > common/src/test/java/org/apache/sqoop/model/TestMAccountableEntity.java > ba53739 > common/src/test/java/org/apache/sqoop/model/TestMBooleanInput.java 433c160 > common/src/test/java/org/apache/sqoop/model/TestMConfig.java c5a07a0 > common/src/test/java/org/apache/sqoop/model/TestMConfigList.java 46f4f81 > common/src/test/java/org/apache/sqoop/model/TestMConnector.java 89ae440 > common/src/test/java/org/apache/sqoop/model/TestMDriver.java aa1ee34 > common/src/test/java/org/apache/sqoop/model/TestMEnumInput.java c76d031 > common/src/test/java/org/apache/sqoop/model/TestMIntegerInput.java dd9227e > common/src/test/java/org/apache/sqoop/model/TestMJob.java 848c2cc > common/src/test/java/org/apache/sqoop/model/TestMJobConfig.java 7d0641e > common/src/test/java/org/apache/sqoop/model/TestMLink.java 9ad8954 > common/src/test/java/org/apache/sqoop/model/TestMLinkConfig.java 62f61a6 > common/src/test/java/org/apache/sqoop/model/TestMMapInput.java fbc08c7 > common/src/test/java/org/apache/sqoop/model/TestMNamedElement.java 6610784 > common/src/test/java/org/apache/sqoop/model/TestMPersistableEntity.java > 8e1e5bd > common/src/test/java/org/apache/sqoop/model/TestMStringInput.java 76e625e > common/src/test/java/org/apache/sqoop/model/TestMValidatedElement.java > 3db261a > common/src/test/java/org/apache/sqoop/schema/type/TestArray.java 82a7d8e > common/src/test/java/org/apache/sqoop/schema/type/TestEnum.java f97ebf4 > common/src/test/java/org/apache/sqoop/schema/type/TestMap.java 1f3abfb > common/src/test/java/org/apache/sqoop/schema/type/TestSet.java daca037 > common/src/test/java/org/apache/sqoop/submission/TestSubmissionStatus.java > 948c0a6 > common/src/test/java/org/apache/sqoop/submission/counter/TestCounter.java > 0cf5d2b > > common/src/test/java/org/apache/sqoop/submission/counter/TestCounterGroup.java > ae7aaf0 > common/src/test/java/org/apache/sqoop/submission/counter/TestCounters.java > 90a35c3 > common/src/test/java/org/apache/sqoop/utils/TestClassUtils.java 53ece87 > common/src/test/java/org/apache/sqoop/utils/TestMapResourceBundle.java > 1edc404 > common/src/test/java/org/apache/sqoop/validation/TestStatus.java 813969e > common/src/test/java/org/apache/sqoop/validation/TestValidationRunner.java > 6825435 > > common/src/test/java/org/apache/sqoop/validation/validators/TestClassAvailable.java > cdfa2a9 > > common/src/test/java/org/apache/sqoop/validation/validators/TestContains.java > 485efdd > > common/src/test/java/org/apache/sqoop/validation/validators/TestNotEmpty.java > 1e53e0f > > common/src/test/java/org/apache/sqoop/validation/validators/TestNotNull.java > 0a0b5dc > > common/src/test/java/org/apache/sqoop/validation/validators/TestNullOrContains.java > 60eafff > > common/src/test/java/org/apache/sqoop/validation/validators/TestStartsWith.java > 0fd464c > > common/src/test/java/org/apache/sqoop/validation/validators/TestValidator.java > 49b7609 > connector/connector-generic-jdbc/pom.xml fc6cab4 > > connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutorTest.java > 61846b7 > > connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExtractor.java > 8e1ce5b > > connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestFromInitializer.java > 345fe9b > > connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestGenericJdbcConnectorUpgrader.java > d6fe504 > > connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestLoader.java > 538b033 > > connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestPartitioner.java > d62e494 > > connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestToInitializer.java > 2d7ec3a > connector/connector-hdfs/pom.xml 8b1e11f > > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java > e5b7b2a > > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestHdfsUtils.java > bba6502 > > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLinkConfig.java > 176d0df > > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java > be57fa0 > > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestPartitioner.java > 04e09cd > connector/connector-kafka/pom.xml e8fea9b > > connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/TestConfigValidator.java > b61d979 > > connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/TestKafkaLoader.java > 4ed027f > connector/connector-kite/pom.xml 10ed099 > > connector/connector-kite/src/test/java/org/apache/sqoop/connector/kite/TestKiteExecutor.java > 5e4edc5 > > connector/connector-kite/src/test/java/org/apache/sqoop/connector/kite/TestKiteLoader.java > a1016a0 > > connector/connector-kite/src/test/java/org/apache/sqoop/connector/kite/TestKiteToDestroyer.java > 4051fda > > connector/connector-kite/src/test/java/org/apache/sqoop/connector/kite/TestKiteToInitializer.java > 5f0525d > connector/connector-sdk/pom.xml 38c217a > > connector/connector-sdk/src/test/java/org/apache/sqoop/connector/common/TestSqoopIDFUtils.java > e9108b0 > > connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestCSVIntermediateDataFormat.java > f6852a0 > core/pom.xml 2b6e436 > core/src/test/java/org/apache/sqoop/core/TestSqoopConfiguration.java > 4d58bd1 > core/src/test/java/org/apache/sqoop/driver/TestDriverConfigUpgrader.java > e5201fc > core/src/test/java/org/apache/sqoop/driver/TestJobManager.java 5bc1b03 > core/src/test/java/org/apache/sqoop/driver/TestJobRequest.java 3f36030 > core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java > 56add07 > core/src/test/java/org/apache/sqoop/repository/TestRepositoryManager.java > 7cec536 > docs/pom.xml f8ccc34 > execution/mapreduce/pom.xml ad7f489 > execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java > cc0a3cc > execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java > 1b791e3 > > execution/mapreduce/src/test/java/org/apache/sqoop/job/io/TestSqoopWritable.java > 79d6a8f > > execution/mapreduce/src/test/java/org/apache/sqoop/job/mr/TestMRConfigurationUtils.java > 70ea6d4 > > execution/mapreduce/src/test/java/org/apache/sqoop/job/mr/TestSqoopOutputFormatLoadExecutor.java > f5f627d > pom.xml ea157f7 > repository/repository-common/pom.xml 37378c6 > repository/repository-derby/pom.xml 9be96db > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java > 6cb3eb0 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestConnectorHandling.java > efc4418 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestDriverHandling.java > 25a0093 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestInputTypes.java > 71cc763 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java > 309e6b2 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java > 6274d11 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestRepositoryUpgrade.java > 31154af > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestSubmissionHandling.java > b479266 > submission/mapreduce/pom.xml 7b45492 > test/pom.xml f74ee3c > test/src/main/java/org/apache/sqoop/test/asserts/HdfsAsserts.java d8f2b8d > test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java > 9a76c4b > > test/src/main/java/org/apache/sqoop/test/testcases/KafkaConnectorTestCase.java > 804516b > test/src/main/java/org/apache/sqoop/test/testcases/TomcatTestCase.java > 0b0a0a2 > > test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/FromHDFSToRDBMSTest.java > b1b3b16 > > test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/FromRDBMSToHDFSTest.java > e482ac5 > > test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/PartitionerTest.java > bd34911 > > test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/TableStagedRDBMSTest.java > f42fa32 > > test/src/test/java/org/apache/sqoop/integration/connector/kafka/FromHDFSToKafkaTest.java > dabb69d > > test/src/test/java/org/apache/sqoop/integration/connector/kafka/FromRDBMSToKafkaTest.java > 93d657c > > test/src/test/java/org/apache/sqoop/integration/repository/derby/upgrade/DerbyRepositoryUpgradeTest.java > 0c25f18 > > test/src/test/java/org/apache/sqoop/integration/server/SubmissionWithDisabledModelObjectsTest.java > 055bc3d > test/src/test/java/org/apache/sqoop/integration/server/VersionTest.java > b3254bd > tomcat/pom.xml 91616bb > tools/pom.xml 01e1a5f > > Diff: https://reviews.apache.org/r/29499/diff/ > > > Testing > ------- > > mvn clean verify > mvn clean test -Phadoop200,fast > mvn clean test -Phadoop200,slow > > > Thanks, > > Abraham Elmahrek > >
