adoptium / aqa-tests

Home of test infrastructure for Adoptium builds
https://adoptium.net/aqavit
Apache License 2.0
129 stars 308 forks source link

Proposal: Reduce Jenkinsfilebase params by combining repo/branch names into 1 string #1873

Closed smlambert closed 3 years ago

smlambert commented 4 years ago

For each of the REPO and BRANCH parameter pairs we have, we can reduce the 2 parameters to a single string, this would not only reduce the number of parameters, but also allow for cut'n'paste when launching Grinders during review.

(repo:branch is typically listed at the top right in Github PR, which could then be cut'n'pasted into the Grinder fields when running Grinders during review)

In Jenkinsfilebase (or TKG if we want to do this at the lower layer), logic to parse the single string into REPO and BRANCH. One caveat is if we proceed with https://github.com/AdoptOpenJDK/openjdk-tests/issues/1872 then parsing logic would also have to look at other information, example vendor info, to avoid making assumptions about repo location

Reduces 14 params down to 7 (if you count in JDK_REPO/BRANCH from proposal #1872):

Screen Shot 2020-07-08 at 2 32 47 PM Screen Shot 2020-07-08 at 2 33 07 PM Screen Shot 2020-07-08 at 2 33 35 PM
smlambert commented 4 years ago

I like the explicit nature of the 2 separate params, but definitely think the Grinder job config would be nicer to use by collapsing the params. Let's use this issue to discuss pros/cons.

llxia commented 4 years ago

I really like the idea. This will make Grinder so much easy to use. No more typing! I can see this been used for most of our repo and branch parameters, but I am not sure about ADOPTOPENJDK_REPO and ADOPTOPENJDK_BRANCH.

Currently, we use them in job configure, so the job can check out specific ADOPTOPENJDK_REPO and ADOPTOPENJDK_BRANCH.

If we change ADOPTOPENJDK_REPO and ADOPTOPENJDK_BRANCH into one parameter, I am not sure how to use it in the job configure.

smlambert commented 4 years ago

Ah good call, so maybe ADOPTOPENJDK_REPO and ADOPTOPENJDK_BRANCH keep their current status of unique individual parameters, since they represent the seed/base/starter repo, and other tangential repos shift to the collapsed single parameter, not quite as clean, but it still reduction.

If ADOPTOPENJDK_REPO_BRANCH = "AdoptOpenJDK:master" Doubt its possible to change the Repository URL and Branch specifier to contain groovy code or if they only allow for variables but if it were possible, perhaps could parse and fill with some string methods... where ADOPTOPENJDK_BRANCH is substring(${ADOPTOPENJDK_REPO_BRANCH}.indexOf(‘:’))

llxia commented 4 years ago

Thanks Shelley. I will give it a try.

llxia commented 4 years ago

PR #1940 and PR #1941 combined repo/branch names for TKG, stf, openjdk-systemtest and openj9-systemtest.

Remaining repos:

smlambert commented 3 years ago

I think the remaining 3 repos are not easily collapsed into a single param (as there are variants and/or reasons that make it impossible to assume defaults), given this, we can likely mark this issue as closed. Do you agree @llxia ?

llxia commented 3 years ago

yes, close this issue.