> 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.
>
> Qian Xu wrote:
> 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");
>
> Veena Basavaraj wrote:
> expectation is just if it found the class, if it did not find it then it
> will throw Exception,
The expected behavior of your test case is to ensure class can be loaded. And
your code relies on not seeing an implicit exception. Wouldn't the following
code be more straightfoward?
Class<AbstractConnector> actual =
ClassUtils.loadClass("org.apache.sqoop.connector.jdbc.GenericJdbcConnector");
assertNotNull(actual);
- 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
>
>
