'bin/server' shellscript execution instead of 'bin/tools/ws-server.jar' #42
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 7 commits
February 1, 2019 16:46
- Mapping server.env file to liberty-maven-plugin
…h server.bat) - Changed runnable to bin/tools/ws-server.jar (in accordance with server.bat) - Attempt to set ProcessBuilder environment before server-start - Parsing server.env files from both runtime-level (.../etc/server.env) and server-level (.../<server-name>/server.env) - Populating ProcessBuilder environment with the fetched values
…injections (might as well be a DBConnection) and System.getenv... - Adding jndi-1.0 feature to server.xml, for obvious reasons - Defining <jndiEntry .../> for reasons defined above
- Adding Runtime.getRuntime().exec() runnable for os specific shell script execution - Server is being run
HasseNasse
changed the title
aguibert
reviewed
Feb 8, 2019
There was a problem hiding this comment.
Thanks @HasseNasse! Looks like a great start, just a few review comments.
liberty-managed/src/test/java/io/openliberty/arquillian/managed/WLPResourceTestCase.java
Outdated
Show resolved
Hide resolved
liberty-managed/src/main/java/io/openliberty/arquillian/managed/WLPManagedContainer.java
Outdated
Show resolved
Hide resolved
liberty-managed/src/main/java/io/openliberty/arquillian/managed/WLPManagedContainer.java
Outdated
Show resolved
Hide resolved
liberty-managed/src/main/java/io/openliberty/arquillian/managed/WLPManagedContainer.java
Outdated
Show resolved
Hide resolved
liberty-managed/src/main/java/io/openliberty/arquillian/managed/WLPManagedContainer.java
Outdated
Show resolved
Hide resolved
added 7 commits
February 9, 2019 00:54
- Refactor method name to suit test-case with expected turn-out
- Implement os specific shell-script execution behaviour - Add some trace-logging - Merge errorstream with stdout - create (initial setup) for JAVA_TOOL_OPTIONS environment variable.
…ssible - Created functionality to parse and add JAVA_TOOL_OPTIONS env-var as a white space seperated list of arguments. - Trace logging added - Setting default '-Dcom.ibm.ws.logging.console.log.level=INFO' argument.
- Mapping server.env file to liberty-maven-plugin - Changed -javaagent to bin/tools/ws-javaagent.jar (in accordance with server.bat) - Changed runnable to bin/tools/ws-server.jar (in accordance with server.bat) - Attempt to set ProcessBuilder environment before server-start - Parsing server.env files from both runtime-level (.../etc/server.env) and server-level (.../<server-name>/server.env) - Populating ProcessBuilder environment with the fetched values - ResourceInjection test created, testing to fetch both through JNDI injections (might as well be a DBConnection) and System.getenv... - Adding jndi-1.0 feature to server.xml, for obvious reasons - Defining <jndiEntry .../> for reasons defined above - Small javadoc changes - Commenting out old ProcessBuilder - Adding Runtime.getRuntime().exec() runnable for os specific shell script execution - Server is being run - Use /bin/server shell script to initiate managed server connection - code being re-added for future reference - Amend test to check resource injection value rather than nullity - Refactor method name to suit test-case with expected turn-out - Remove uncommented code - Implement os specific shell-script execution behaviour - Add some trace-logging - Merge errorstream with stdout - create (initial setup) for JAVA_TOOL_OPTIONS environment variable. - Add wlp and ol 19.0.0.1 server versions to travis build - Add missing jvm-arguments documentation to liberty-managed - Removed some uneeded type specifications where type-inference is possible - Created functionality to parse and add JAVA_TOOL_OPTIONS env-var as a white space seperated list of arguments. - Trace logging added - Setting default '-Dcom.ibm.ws.logging.console.log.level=INFO' argument.
…b.com/HasseNasse/liberty-arquillian into feature/server-shellscript-conformity
HasseNasse
changed the title
chyt
reviewed
Feb 11, 2019
There was a problem hiding this comment.
I'm thinking that it's most likely preferable to call bin/server stop in stop() rather than just killing the server process? Other than that the changes look good to me. Also I would just point out that we should probably refactor to use the Liberty Ant Plugin at some point in the future.
aguibert
approved these changes
Feb 11, 2019
There was a problem hiding this comment.
looks good, thanks @HasseNasse!
I'll defer the actual merge to @chyt though
I`ll give the gracefull shutdown comment a look whenever possible. |
|
The shut down hook has now been updated, would be nice to merge this PR as we are wanting to run Integration Tests on our application using the liberty-managed adapter with the added functionalities. |
chyt
reviewed
Feb 19, 2019
liberty-managed/src/main/java/io/openliberty/arquillian/managed/WLPManagedContainer.java
Outdated
Show resolved
Hide resolved
…of using list.replaceAll(...)
chyt
approved these changes
Feb 20, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Short description of what this resolves:
Summary of discussion #40:
bin/servershellscript already handles jvm.options + server.env pre-processing before server start-up, we would wantliberty-managedto re-use this script rather than programmatically tailoring its own.19.0.0.1?will support passing generic JVM arguments to thebin/serverChanges proposed in this pull request:
bin/server run <servername>used to start server with ConsoleConsumer.Comment: Tests passing.
-Dcom.ibm.ws.logging.console.log.level=INFOComment:
19.0.0.2would have allowed the passing of JVM args directly to the shell script. However, this would cause issues with older versions of liberty. The proposed solution (@aguibert ) is to useJAVA_TOOL_OPTIONSonProcessBuilderwhich should work for all JDKs.bin/server stopList will be updated as work goes on..
Fixes: #40