> On Jan. 5, 2015, 9:44 a.m., Jarek Cecho wrote: > > 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 > > Abraham Elmahrek wrote: > 1. Pre-commit build should be fine. > 2. I'll update the patch and send a note to dev@. Let's sync. so that we > can minimize the window. I'll send a note to the dev@ mailing list as > suggested. Let's chat there.
Works with me, thank you Abe! I'm more then happy to help in the window, so let's start the discussion! - Jarek ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29499/#review66608 ----------------------------------------------------------- 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 > >
