----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29749/#review67614 -----------------------------------------------------------
Nice, thank you for the patch Abe. Since the tests are requiring external resources, I think that they are more integration tests then a unit tests and hence I'm wondering if it would make sense to put them into "integration" folder and execute them only as part of integration-test maven phase as we are doing in the "test" module? Here is the associated surefire configuration: https://github.com/apache/sqoop/blob/sqoop2/test/pom.xml#L148 common-test/src/main/java/org/apache/sqoop/common/test/asserts/ProviderAsserts.java <https://reviews.apache.org/r/29749/#comment111678> I'm wondering why this change, what is different in calling fail() and throwing AssertionError? common-test/src/main/java/org/apache/sqoop/common/test/asserts/ProviderAsserts.java <https://reviews.apache.org/r/29749/#comment111677> We should call provider.escapeTableName(table) common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java <https://reviews.apache.org/r/29749/#comment111679> Could you please add finally clause and properly close the ResultSet and PreparedStatement? Jarcec - Jarek Cecho On Jan. 9, 2015, 6:13 a.m., Abraham Elmahrek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29749/ > ----------------------------------------------------------- > > (Updated Jan. 9, 2015, 6:13 a.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-1591 > https://issues.apache.org/jira/browse/SQOOP-1591 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > commit 560387f1d3e6869ada914fa6ae5ada7d8e210721 > Author: Abraham Elmahrek <[email protected]> > Date: Wed Jan 7 20:44:44 2015 -0800 > > SQOOP-1591: Sqoop2: PostgreSQL integration tests > > :100644 100644 8196fe2... cb987f6... M > common-test/src/main/java/org/apache/sqoop/common/test/asserts/ProviderAsserts.java > :100644 100644 82289e8... 3a6205e... M > common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java > :100644 100644 5d80dce... 7248d7c... M > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java > :000000 100644 0000000... 85f895d... A > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java > :000000 100644 0000000... 6022bb3... A > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestDriverHandling.java > :000000 100644 0000000... 1dfacb5... A > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestJobHandling.java > :000000 100644 0000000... 54c598e... A > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestLinkHandling.java > :100644 100644 2da19bc... 4735240... M > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java > :000000 100644 0000000... 9db7940... A > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestSubmissionHandling.java > > > Diffs > ----- > > > common-test/src/main/java/org/apache/sqoop/common/test/asserts/ProviderAsserts.java > fb4e7af > > common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java > 82289e8 > > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java > ea199d4 > > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java > PRE-CREATION > > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestDriverHandling.java > PRE-CREATION > > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestJobHandling.java > PRE-CREATION > > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestLinkHandling.java > PRE-CREATION > > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java > 6075de4 > > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestSubmissionHandling.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/29749/diff/ > > > Testing > ------- > > mvn clean test -pl repository/repository-postgresql > -Dsqoop.provider.class=org.apache.sqoop.common.test.db.PostgreSQLProvider > -Dsqoop.provider.postgresql.jdbc=jdbc:postgresql://.../test > -Dsqoop.provider.postgresql.username=test > -Dsqoop.provider.postgresql.password=test > > > Thanks, > > Abraham Elmahrek > >
