[Fix keycloak#12385] Update option to run kc.bat on windows instead of kc.sh #12386
Conversation
|
@nehachopra27 Thanks. |
There was a problem hiding this comment.
Could you please elaborate on why we need this dependency? Looks like you added after the first round of reviews?
testsuite/model/pom.xml
Outdated
There was a problem hiding this comment.
Could you please elaborate on why we need this dependency? Looks like you added after the first round of reviews?
There was a problem hiding this comment.
@pedroigor : the earlier wrapper which was used was failing the testsuite compilation and tests. So with current scenario I was trying to fix additional dependancies that were required. I realised that these should come in a separate PR. This approach created additional failures, so I am now using environment variable.
01f863e to
27148b3
Compare
There was a problem hiding this comment.
Works for me to not add the SystemUtils library (or the Environment wrapper we use and you used in your previous version), but then please check if "os.name" is null before lowercasing, bc. even when true for 99% of the systems, it may be (and I experienced this in the past 😅 ) that os.name is not set at all for some reason. Apache commons checks for that, and so should we :)
There was a problem hiding this comment.
Thanks @DGuhr, I agree it can happen. I have added the suggested constraint.
fb5809e to
ec96dfa
Compare
|
Thanks, @nchopra27. |
|
Thanks @pedroigor and @DGuhr ! |
Closes #12385
The Current Quarkus container runs kc.sh in the windows machine as seen in the QE test pipeline.
Adding support for kc.bat files to be run when the environment is windows.