Rework camel-jbang-it to support camel-launcher #21362
Conversation
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🤖 CI automation will test this PR automatically. 🐫 Apache Camel Committers, please review the following items:
|
jvrubel
force-pushed
the
implement-camel-launcher
branch
from
ad00ff2 to
d476966
Compare
...ang/camel-jbang-it/src/test/java/org/apache/camel/dsl/jbang/it/support/JBangTestSupport.java
Outdated
Show resolved
Hide resolved
|
|
||
| private static final String DATA_FOLDER = System.getProperty(CliProperties.DATA_FOLDER); | ||
|
|
||
| private static final String MAIN_COMMAND = System.getProperty("cli.service.command"); |
There was a problem hiding this comment.
what do you think to implement a getMainCommand() method in the CliService class so that we have just one place to retrieve the property? you can use so containerService.getMainCommand() instead of MAIN_COMMAND in all the places
There was a problem hiding this comment.
Removed the property access from JBangTestSupport.java and replaced it with a method to get the property from CliService
jvrubel
force-pushed
the
implement-camel-launcher
branch
from
d476966 to
3d3f296
Compare
| protected void checkCommandOutputs(String command, String contains) { | ||
| Assertions.assertThat(execute(command)) | ||
| .as("command camel " + command + " should output " + contains) | ||
| .as("command " + MAIN_COMMAND + " " + command + "should output " + contains) |
There was a problem hiding this comment.
does it compile? I cannot find the MAIN_COMMAND variable, you should use now getMainCommand(), am I wrong?
|
|
||
| protected String executeBackground(final String command) { | ||
| return containerService.executeBackground(command); | ||
| return containerService.execute(command + " --background"); |
There was a problem hiding this comment.
I'm expecting this to be restored on the current version containerService.executeBackground(command) or there are specific reasons? thanks
4 participants
These changes allow customization of the
camelcommand in order to supportcamel-launcher.There are also differences from the closed PR with the same title. This one only contains the rework itself alongside some minor fixes. I also greatly simplified it by removing the property out of the container service and keeping it as a simple system property as @mcarlett suggested.