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

Test Autogen: Reset JCK_GIT_REPO every loop #2425

Merged

Conversation

AdamBrousseau
Copy link
Contributor

Similar to what is done for ACTUAL_TEST_JOB_NAME,
use and reset the ACTUAL_JCK_GIT_REPO every loop in
order to hit the condition everytime. Otherwise
JCK_GIT_REPO would be set on the first loop for the
duration of the build.

Signed-off-by: Adam Brousseau [email protected]

@llxia for review
Tested internal Test_Job_Auto_Gen_Sandbox/104

@llxia
Copy link
Contributor

llxia commented Mar 30, 2021

Could you resolve the conflicts? Thanks

@AdamBrousseau
Copy link
Contributor Author

Rebased, and cleaned up some whitespace.

if (GROUP == "jck" && JCK_GIT_REPO_PREFIX && !JCK_GIT_REPO) {
JCK_GIT_REPO = "${JCK_GIT_REPO_PREFIX}/JCK${JDK_VERSION}-unzipped.git"
ACTUAL_JCK_GIT_REPO = "${JCK_GIT_REPO_PREFIX}/JCK${JDK_VERSION}-unzipped.git"
Copy link
Contributor

@llxia llxia Mar 30, 2021

Choose a reason for hiding this comment

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

I think the original code supports two cases:

  • the user provides JCK_GIT_REPO repo. The script uses it to generate test jobs
  • the user does not provide JCK_GIT_REPO repo. The script sets JCK_GIT_REPO to ${JCK_GIT_REPO_PREFIX}/JCK${JDK_VERSION}-unzipped.git // this is ideal for running in a loop

Also, the change in this PR will cause user-provided JCK_GIT_REPO to be ignored.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. I think I've fixed it now.

Copy link
Contributor

@llxia llxia left a comment

Choose a reason for hiding this comment

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

LGTM

@llxia llxia requested a review from smlambert March 30, 2021 17:32
@karianna karianna added the bug label Mar 31, 2021
@karianna karianna added this to the March 2021 milestone Mar 31, 2021
@karianna
Copy link
Contributor

rebase required.

Similar to what is done for ACTUAL_TEST_JOB_NAME,
use and reset the ACTUAL_JCK_GIT_REPO every loop in
order to hit the condition everytime. Otherwise
JCK_GIT_REPO would be set on the first loop for the
duration of the build.

Signed-off-by: Adam Brousseau <[email protected]>
@AdamBrousseau
Copy link
Contributor Author

rebased

Copy link
Contributor

@smlambert smlambert left a comment

Choose a reason for hiding this comment

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

LGTM

@smlambert smlambert merged commit 1afa93f into adoptium:master Mar 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants