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

Adoptopenjdk_repo can only be changed in grinders #5135

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

adamfarley
Copy link
Contributor

This change is to alert jenkins users that non-grinder test jobs will ignore any changes to these two variables.

This change is to alert jenkins users that non-grinder test jobs will
ignore any changes to these two variables.

Signed-off-by: Adam Farley <adfarley@redhat.com>
Copy link
Contributor

@sendaoYan sendaoYan left a comment

Choose a reason for hiding this comment

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

/LGTM

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.

I do not believe this is technically true (and want to check how that is enforced if is true). Also why drop the information about zos, used by one of our members. ignore that, see that its retained.

You may be referring to when Test jobs triggered by build pipelines override the values of these, or when USE_TESTENV_PROPERTIES=true, and not the case when running a Test job manually with the appropriate parameters set.

@adamfarley
Copy link
Contributor Author

Hi Shelley,

I think this enforced by the "Pipeline script from SCM" bit in the job config. If you're rerunning a test job manually, the Git repository URL is hard-coded in a normal test job, but set dynamically to the value of the ADOPTOPENJDK_REPO variable in a grinder job.

I recall the team had a discussion about this during one of our calls (bit vague, sorry), and the conclusion was that this was intentional. I proposed adding a comment to clarify the functionality.

@adamfarley
Copy link
Contributor Author

@adamfarley
Copy link
Contributor Author

adamfarley commented Mar 12, 2024

I think what tripped me up at the time was that the aqa-test branch is set dynamically, but the repo is hard-coded. So I guess the comment could more accurately be: Repo cannot be changed, and branch shouldn't be changed.

In normal test jobs, I mean. Like this one: https://ci.adoptium.net/view/Failing%20Temurin%20Test%20Jobs/job/Test_openjdk23_hs_extended.perf_aarch64_windows/

@karianna karianna requested a review from smlambert March 13, 2024 01:16
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.

This statement is incorrect. This PR should be closed without merging.

Signed-off-by: Adam Farley <adfarley@redhat.com>
@adamfarley
Copy link
Contributor Author

This statement is incorrect. This PR should be closed without merging.

Hi Lan. I agree that the comment could be more accurate, so I've updated it.

Let me know if this works for you.

Signed-off-by: Adam Farley <adfarley@redhat.com>
Signed-off-by: Adam Farley <adfarley@redhat.com>
Signed-off-by: Adam Farley <adfarley@redhat.com>
Signed-off-by: Adam Farley <adfarley@redhat.com>
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.

None yet

5 participants