ivankelly commented on a change in pull request #1135: Specify repo in 
MavenClassLoader
URL: https://github.com/apache/bookkeeper/pull/1135#discussion_r167520709
 
 

 ##########
 File path: 
tests/integration-tests-utils/src/test/java/org/apache/bookkeeper/tests/MavenClassLoaderTest.java
 ##########
 @@ -39,4 +39,9 @@ public void testNoZooKeeperInterference() throws Exception {
                                    
org.apache.zookeeper.KeeperException.NoNodeException.class));
     }
 
+    @Test
+    public void testLoadSpecifyingMavenRepo() throws Exception {
 
 Review comment:
   @eolivelli actually I misunderstood your original point. Yes, unit tests 
shouldn't have remote resource dependencies. I thought you meant we shouldn't 
use this in the integration tests.
   @sijie I wrote the test case before the code to validate that what I assumed 
the library would do is actually what the library did.
   
   I think the test has value, but given the hassle of having to boot a http 
server and mock up a maven repo, just to test a test utility, I'm just going to 
remove the test.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to