Skip to content

Separate generate/trigger child jobs with parameters defined by Configrations and Params - #7536

Merged
smlambert merged 6 commits into
adoptium:masterfrom
sophia-guo:aqa-tck
Sep 3, 2026
Merged

Separate generate/trigger child jobs with parameters defined by Configrations and Params#7536
smlambert merged 6 commits into
adoptium:masterfrom
sophia-guo:aqa-tck

Conversation

@sophia-guo

Copy link
Copy Markdown
Contributor

Close #7535

AqaTestPipeline triggers donwstream test jobs either by predefined configuration parameters or parameters forwarded from pipeline job itself. However several parameters (PARALLEL, NUM_MACHINES,KEEP_REPORTDIR, DYNAMIC_COMPILE) read from buildConfig only and silently discarded the incoming pipeline params, which arehardcoded as special cases. This is hard to reason about and error-prone.

Refactor generateJobs() into three focused functions:

  • generateJobsWithConfig(): JSON buildConfig is authoritative for all config values
  • generateJobsFromParams(): manual/Grinder/noconfigurations,pipeline params are authoritative, hard-coded DEFAULTS as last resort
  • triggerChildJob(): shared lower half — fires the downstream job and collects TAP artifacts

Extract two additional shared helpers:

  • buildTestFlagSuffix(): computes job name suffix from test flag string
  • resolvePlatformVars(): derives os, arch, jdk_impl, download_url from platform string

Each function now has a single clear source of truth for parameter values, eliminating the mixed buildConfig/?:/params fallback chains that made the previous code error-prone and hard to reason about.

Assisted by IBM Bob

@sophia-guo
sophia-guo marked this pull request as draft August 18, 2026 04:05
@sophia-guo
sophia-guo requested a lite review from Copilot August 18, 2026 04:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors aqaTestPipeline.groovy to clearly separate downstream job generation into config-driven vs params-driven paths, addressing cases where pipeline parameters were previously ignored or inconsistently overridden (notably for private relay/manual runs).

Changes:

  • Split job generation into generateJobsWithConfig() (JSON config authoritative) and generateJobsFromParams() (pipeline params authoritative).
  • Extract shared helpers for job naming/platform-derived variables and downstream triggering/artifact collection (buildTestFlagSuffix, resolvePlatformVars, triggerChildJob).
  • Simplify/centralize parameter assembly logic for downstream job triggers.
Suppressed comments (1)

buildenv/jenkins/aqaTestPipeline.groovy:593

  • generateJobsWithConfig() later forces config-derived values for DYNAMIC_COMPILE/KEEP_REPORTDIR/RERUN_ITERATIONS and VENDOR_TEST_* via explicit childParams appends, but these keys are not excluded from the params.each loop. If Jenkins honors the first occurrence of a duplicate parameter, the pipeline param value could override the config value, defeating the "config is authoritative" goal. Exclude the keys that are set explicitly later from the params pass-through loop.
                if (param.key in ["PLATFORMS", "TARGETS", "TOP_LEVEL_SDK_URL", "AUTO_AQA_GEN",
                                  "JDK_VERSIONS", "VARIANT", "PIPELINE_DISPLAY_NAME"]) {
                    // do not pass to child jobs

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread buildenv/jenkins/aqaTestPipeline.groovy Outdated
Comment on lines +415 to +417
if (param.key in ["PLATFORMS", "TARGETS", "TOP_LEVEL_SDK_URL", "AUTO_AQA_GEN",
"JDK_VERSIONS", "VARIANT", "PIPELINE_DISPLAY_NAME"]) {
// do not pass to child jobs
Comment thread buildenv/jenkins/aqaTestPipeline.groovy Outdated
Comment on lines 557 to 560
def VENDOR_TEST_REPOS = params.VENDOR_TEST_REPOS ?: (buildConfig.VENDOR_TEST_REPOS ?: '')
def VENDOR_TEST_BRANCHES = params.VENDOR_TEST_BRANCHES ?: (buildConfig.VENDOR_TEST_BRANCHES ?: '')
def VENDOR_TEST_DIRS = params.VENDOR_TEST_DIRS ?: (buildConfig.VENDOR_TEST_DIRS ?: '')
def VENDOR_TEST_DIRS = params.VENDOR_TEST_DIRS ?: (buildConfig.VENDOR_TEST_DIRS ?: '')
int rerunIterations = (params.RERUN_ITERATIONS ?: (buildConfig.RERUN_ITERATIONS ?: '0')).toString().toInteger()
@sophia-guo sophia-guo changed the title Fix parameter pass-through for private relay scenario in aqaTestPipeline Separate generate/trigger child jobs with parameters defined by Configrations and Params Aug 24, 2026
@sophia-guo
sophia-guo force-pushed the aqa-tck branch 4 times, most recently from fe02c27 to 74e030c Compare August 26, 2026 22:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment thread buildenv/jenkins/aqaTestPipeline.groovy
Comment thread buildenv/jenkins/aqaTestPipeline.groovy
@sophia-guo

Copy link
Copy Markdown
Contributor Author

Depends on #7632

@sophia-guo

Copy link
Copy Markdown
Contributor Author

Test with AUTO_AQA_GEN=true and AUTO_AQA_GEN=false, job parameters generated by configuration are same as before and parameters are passed to child jobs correctly for example https://ci.adoptium.net/job/Test_openjdk26_hs_sanity.functional_x86-64_linux/62/parameters/ - time_limit is 25 instead of job template default 10.

https://ci.adoptium.net/job/AQA_Test_Pipeline_TESTING/24/
https://ci.adoptium.net/job/AQA_Test_Pipeline_TESTING/23/

jck rerun release build https://ci.adoptium.net/job/AQA_Test_Pipeline_JCK/444/
--> https://ci.adoptium.net/view/Test_grinder/job/AQA_Test_Pipeline_JCK/496/ - has same job parameters.

@sophia-guo sophia-guo self-assigned this Aug 31, 2026
@sophia-guo
sophia-guo marked this pull request as ready for review August 31, 2026 20:50

@andrew-m-leonard andrew-m-leonard left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks good I think

@sophia-guo

Copy link
Copy Markdown
Contributor Author

@annaibm could you help to test if there is any issue with openj9 side? To test I've created aqa-tckT branch. The only difference is aqa-tckT branch set aqa repo and branch as adoptium:master, which is on purpose as test jobs are light_weight_checkout by default. Check if test jobs are triggered with correct parameters are enough. Thanks.

@annaibm

annaibm commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

@sophia-guo
Tested on internal Jenkins (hyc-runtimes-jenkins) against sophia-guo/openjdk-tests:aqa-tckT.

AQA_Test_Pipeline_Release #573 correctly checked out the personal repo/branch and computed the merged buildConfig — confirmed in the log:

Cloning repository https://github.com/sophia-guo/openjdk-tests.git
Checking out Revision ea682c8acafccc18007001d8bdc22221bda48ee8 (origin/aqa-tckT)
...
Applied buildConfig: [..., VENDOR_TEST_REPOS:git@github.ibm.com:runtimes/test.git, VENDOR_TEST_BRANCHES:master, VENDOR_TEST_DIRS:functional, KEEP_REPORTDIR:true, PARALLEL:Dynamic, NUM_MACHINES:3, DYNAMIC_COMPILE:true, RERUN_ITERATIONS:3, ...]

This build then triggered Test_openjdk26_j9_sanity.functional_x86-64_linux #22. Checking that child job's printenv, most params landed correctly and matched the upstream buildConfig — KEEP_REPORTDIR=true, DYNAMIC_COMPILE=true, RERUN_ITERATIONS=3, VENDOR_TEST_REPOS/BRANCHES/DIRS all correct, no duplicates observed.

However:

ADOPTOPENJDK_REPO=https://github.com/adoptium/aqa-tests.git
ADOPTOPENJDK_BRANCH=master

The child job's own checkout still used the default adoptium/aqa-tests:master rather than sophia-guo/openjdk-tests:aqa-tckT.

Note: I don't have access to https://openj9-jenkins.osuosl.org/ so this was run entirely on our internal hyc-runtimes-jenkins instance instead

@sophia-guo

sophia-guo commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

The child job's own checkout still used the default adoptium/aqa-tests:master rather than sophia-guo/openjdk-tests:aqa-tckT.

@annaibm this is expected. The only difference between aqa-tckT( the branch for testing) and aqa-tck( this PR branch) https://github.com/sophia-guo/openjdk-tests/compare/aqa-tck...sophia-guo:openjdk-tests:aqa-tckT?expand=1 is the test job's aqa setting is default adoptium/aqa-tests:master rather than sophia-guo/openjdk-tests:aqa-tckT. If you check the job configuration in openj9 side it will be similar as
Screenshot 2026-09-01 at 2 05 06 PM

SCM repository URL is hardcoded as https://github.com/adoptium/aqa-tests.git, which will fail the job if using personal repo and branch - sophia-guo/openjdk-tests:aqa-tck.

Downstream test jobs uses openjdk_tests groovy, which this PR doesn't change it. Testing parameters can be passed in correctly is enough. Thanks @annaibm .

@sophia-guo

Copy link
Copy Markdown
Contributor Author

@smlambert updated, main changes

  • generateJobsWithConfig(): JSON buildConfig is authoritative for all config values. Instead of looping through pipeline job parameters, looping through generated buildConfig and job params not in configurations.

  • generateJobsFromParams(): manual/Grinder/noconfigurations,pipeline params are authoritative, looping through pipeline job parameters and computed values.

@smlambert smlambert left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

My one comment is out of curiosity, but does not block this PR.

Comment on lines +303 to +304
if (arch.contains("x86-64")) arch = "x64"
else if (arch.contains("x86-32")) arch = "x86-32"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure I understand why L303 resolves to x64 and L304 resolves to x86-64, as in resolves in a non-symmetrical way. Is it that this only mattered when we were building Win32 builds to build the download_url for those artifacts?

@smlambert
smlambert merged commit 7651ffa into adoptium:master Sep 3, 2026
2 checks passed
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.

Downstream child jobs triggered by aqaTestPipeline may not propagate parameters correctly

5 participants