Fix bug in loadJars() #131
Conversation
… not absolute Add slf4j-simple logger in test scope for some logging from tests Convert some File usage to Path
Also adds a nice exception in case we try to load a plugin without a classes folder
| @Test | ||
| public void testPluginLoader() throws ClassNotFoundException { | ||
| PluginManager manager = new DefaultPluginManager(); | ||
| assertNotNull(manager.loadPlugin(Paths.get("src/test/resources/plugin-with-jar"))); |
There was a problem hiding this comment.
Is this file available only on your machine?
| @@ -96,27 +92,27 @@ public static boolean delete(File fileOrFolder) { | |||
| return success; | |||
| } | |||
|
|
|||
| public static List<File> getJars(File folder) { | |||
| public static List<File> getJars(Path folder) { | |||
There was a problem hiding this comment.
I am OK with the modifications from this class. My intention is to move this code to Java 7 NIO API, and to use Files.newDirectoryStream (less code lines and probably faster).
| @@ -33,6 +33,12 @@ | |||
| <version>1.7.5</version> | |||
| </dependency> | |||
| <dependency> | |||
| <groupId>org.slf4j</groupId> | |||
There was a problem hiding this comment.
Do we need this? In all my libraries projects I use slf4j-api as dependency and I use what slf4j I wish in application.
There was a problem hiding this comment.
This added dependency is an slf4j logger implementation with a test scope so it will allow tests to output logs. The runtime jars will NOT contain this class - as you say, the user app decides what to use.
|
Wow, test coverage from 17% to 32% :-) |
|
Please remove |
|
Thanks! I prefer small PR that solves only one problem. |
DefaultPluginLoader loads my plugin just fine, it manages to add
classes/to the PluginClassLoader, but the jars inlib/are not added.Turn out that
loadJars()refuses to add jars if the folder File object is not an absolute path.Also adds
slf4j-simplelogger in test scope and convert some File usage to nio Path.