> On Jan. 15, 2015, 11:03 p.m., Qian Xu wrote:
> > core/src/test/java/org/apache/sqoop/connector/TestConnectorManagerUtils.java,
> > line 59
> > <https://reviews.apache.org/r/29773/diff/8/?file=821969#file821969line59>
> >
> > Are you expecting GenericJdbcConnector class can be instantiated? If it
> > is true, better add a assertNotNull to test.
> >
> > NIT: Maybe a docstring before @Test will be descriptive for this test
> > case.
>
> Veena Basavaraj wrote:
> the ClassUtils throws an exception if class nt found so that will fail
> the test. DOnt think NotNull test is a must.
What is the expectation of the test then??? Usually the following code without
checking its return value means you expect an exception!
ClassUtils.loadClass("org.apache.sqoop.connector.jdbc.GenericJdbcConnector");
- Qian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29773/#review68249
-----------------------------------------------------------
On Jan. 16, 2015, 3:02 a.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29773/
> -----------------------------------------------------------
>
> (Updated Jan. 16, 2015, 3:02 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1821
> https://issues.apache.org/jira/browse/SQOOP-1821
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see jira for details
>
> For committing/ testing:
> Please upload the test jars separately while applying patch and committing,
> since QA bot does not like binary files.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/utils/ClassUtils.java 0be4d41
> common/src/test/java/org/apache/sqoop/utils/TestClassUtils.java 161a1fa
> core/src/main/java/org/apache/sqoop/connector/ConnectorHandler.java 1919b4b
> core/src/main/java/org/apache/sqoop/connector/ConnectorManager.java b9d4d60
> core/src/main/java/org/apache/sqoop/connector/ConnectorManagerUtils.java
> c7193ee
> core/src/main/java/org/apache/sqoop/core/ConfigurationConstants.java
> f341108
>
> core/src/test/java/org/apache/sqoop/connector/TestConnectorManagerUtils.java
> PRE-CREATION
> core/src/test/resources/test-connector.jar PRE-CREATION
> core/src/test/resources/test-non-connector.jar PRE-CREATION
> dist/src/main/server/conf/sqoop.properties e22e8b0
>
> Diff: https://reviews.apache.org/r/29773/diff/
>
>
> Testing
> -------
>
> added unit tests as well
>
>
> Thanks,
>
> Veena Basavaraj
>
>
