Ref #596: adapt library 'camel-test-blueprint-junit5' to camel-karaf #651
Conversation
essobedo
changed the title
essobedo
left a comment
essobedo
left a comment
There was a problem hiding this comment.
Thx for the PR, please avoid huge PR like this one next time as it too long to review. Regarding the PR, could you please remove all tests that seem to test Camel instead of camel-blueprint?
| * | ||
| * @return the name of the path for the .cfg file to load, and the persistence-id of the property placeholder. | ||
| */ | ||
| protected String[] loadConfigAdminConfigurationFile() { |
There was a problem hiding this comment.
Maybe you could create a dedicated class to hold the path of the cfg and the persistence id. Using an array of strings is too error-prone
| FileWriter writer = new FileWriter(cfg); | ||
| try { | ||
| initialConfiguration.store(writer, null); | ||
| } finally { | ||
| IOHelper.close(writer); | ||
| } |
There was a problem hiding this comment.
You could use try-with-resources statement instead
| camelCore.stop(); | ||
| test.stop(); | ||
|
|
||
| Thread.sleep(500); |
There was a problem hiding this comment.
I'm afraid it will make this test flaky, so either we find a way to avoid this sleep or I rather prefer not having the test at all
| if (mJar instanceof JarFile) { | ||
| JarFile jf = (JarFile) mJar; |
There was a problem hiding this comment.
Can be inlined using pattern matching https://docs.oracle.com/en/java/javase/17/language/pattern-matching-instanceof.html
| + "It's likely that the test is misconfigured!"); | ||
| } | ||
|
|
||
| // doPostTearDown(); |
| if (node instanceof Element) { | ||
| Element pp = (Element) node; |
There was a problem hiding this comment.
Can be inlined using pattern matching
| String[][] configAdminPidFiles = new String[0][0]; | ||
| if (file != null) { | ||
| if (file.length % 2 != 0) { // This needs to return pairs of filename and pid | ||
| throw new IllegalArgumentException("The length of the String[] returned from loadConfigAdminConfigurationFile must divisible by 2, was " + file.length); | ||
| } | ||
| configAdminPidFiles = new String[file.length / 2][2]; | ||
|
|
||
| int pair = 0; | ||
| for (int i = 0; i < file.length; i += 2) { | ||
| String fileName = file[i]; | ||
| String pid = file[i + 1]; | ||
| if (!new File(fileName).exists()) { | ||
| throw new IllegalArgumentException("The provided file \"" + fileName + "\" from loadConfigAdminConfigurationFile doesn't exist"); | ||
| } | ||
| configAdminPidFiles[pair][0] = fileName; | ||
| configAdminPidFiles[pair][1] = pid; | ||
| pair++; | ||
| } | ||
| } |
There was a problem hiding this comment.
I hope it can be simplified or at least it should be moved to a dedicated method as it is very hard to read
| @Test | ||
| public void testOverrideNormal() { | ||
| final Long someValue = 60000L; | ||
| System.clearProperty(CamelBlueprintTestSupport.SPROP_CAMEL_CONTEXT_CREATION_TIMEOUT); |
There was a problem hiding this comment.
Should not be needed as it is already done by the cleanup method
|
|
||
| @Test | ||
| public void testDefault() { | ||
| System.clearProperty(CamelBlueprintTestSupport.SPROP_CAMEL_CONTEXT_CREATION_TIMEOUT); |
| <artifactId>camel-blueprint-main</artifactId> | ||
| <packaging>jar</packaging> | ||
|
|
||
| <name>Apache Camel :: Karaf :: Blueprint Main</name> |
There was a problem hiding this comment.
| <name>Apache Camel :: Karaf :: Blueprint Main</name> | |
| <name>Apache Camel :: Karaf :: Components :: Blueprint Main</name> |
| </parent> | ||
|
|
||
| <artifactId>camel-blueprint-main</artifactId> | ||
| <packaging>jar</packaging> |
| <artifactId>camel-test-blueprint-junit5</artifactId> | ||
| <packaging>jar</packaging> | ||
|
|
||
| <name>Apache Camel :: Karaf :: Test :: Blueprint :: Junit 5 </name> |
There was a problem hiding this comment.
| <name>Apache Camel :: Karaf :: Test :: Blueprint :: Junit 5 </name> | |
| <name>Apache Camel :: Karaf :: Test :: Blueprint :: JUnit 5 </name> |
| </parent> | ||
|
|
||
| <artifactId>camel-test-blueprint-junit5</artifactId> | ||
| <packaging>jar</packaging> |
2 participants
fixes #596
Here’s my first Pull Request on GitHub.
I’m open to any feedback that could help improve the code and the way we collaborate on these projects.
All the unit tests should normally be working.
I reintroduced the code and unit tests that were present in version 3.22.4 of camel-karaf, and then took inspiration from the changes made to the Spring tests in the Apache Camel project.