Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SET-155 Create Windows EAP test job on CCI #100

Merged
merged 1 commit into from
Sep 13, 2023

Conversation

RanabirChakraborty
Copy link
Contributor

Copy link
Collaborator

@spyrkob spyrkob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good just a couple nitpicks

@echo off

REM Set the environment variables for Maven and Java
set "JAVA_HOME=%JDK_PATH%"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we simplify this and just use JAVA_HOME and PATH set by Jenkins job definitions in env variables?

cd eap\eap-sources

REM Set the testsuite command
set "COMMAND=mvn clean install -Djboss.dist=%WORKSPACE%\eap\jboss-eap-%EAP_VERSION% -DallTests -D%ip%"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick - it would be helpful if the script could check whether the EAP_VERSION is set before executing the command and fail if not

Copy link
Collaborator

@spyrkob spyrkob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @RanabirChakraborty

@rpelisse FYI we're adding a Windows script to harmonia


REM Set the testsuite command
set "COMMAND=mvn clean install -Djboss.dist=%WORKSPACE%\eap\jboss-eap-%EAP_VERSION% -DallTests -D%ip%"

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to dump all variables being set before the building and testing processes so we can easily identify what is going on in case there's a need to debug or replicate a build environment locally

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Printed all the required variables and the command too.

@spyrkob spyrkob merged commit 96db613 into jboss-set:main Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants