Extra doc for EXTRA_OPTIONS #5770
Conversation
Signed-off-by: Stewart X Addison <[email protected]>
buildenv/jenkins/testJobTemplate
Outdated
| @@ -385,7 +385,7 @@ ARCH_OS_LIST.each { ARCH_OS -> | |||
| sectionHeaderStyle(sectionHeaderHelpTextStyleCss) | |||
| } | |||
| choiceParam('TEST_FLAG', ['', 'JITAAS', 'AOT', 'FIPS', 'FIPS140_2', 'FIPS140_3_OpenJCEPlusFIPS', 'FIPS140_3_OpenJCEPlusFIPS.FIPS140-3'], "Optional. Only set for feature testing. (i.e., JITAAS, AOT, FIPS140_2, etc)") | |||
| stringParam('EXTRA_OPTIONS', "", "Use this to append options to the test command") | |||
| stringParam('EXTRA_OPTIONS', "", "Use this to append options to the test command e.g. to add extra JVM flags") | |||
| stringParam('JVM_OPTIONS', "", "Use this to replace the test original command line options") | |||
There was a problem hiding this comment.
While you are here, consider improving L389, in keeping with your suggestion for EXTRA_OPTIONS
"Replaces the test's original JVM options" to make it clear versus options that would be passed into some other application called in the test command (which for awareness is set via APPLICATION_OPTIONS).
There was a problem hiding this comment.
Done - LMK what you think of the wording. I've also done a little adjustment to CUSTOM_TARGET based on a discussion about .sh files in slack this week.
Also I note that the full description for CUSTOM_TARGET isn't showing up so I've moved the location of ''' to make it consistent with the the TARGET parameter and this is the obvious difference. At the moment only the first line is visible for some reason:
Maybe if we can display the full thing the CUSTOM_TARGET could have an example of a .java and a .sh file?
(Also noting that I'm not sure if there's an easy way to test this - maybe regen a test job somehow using this fork? But a revert will be easy if it's a problem)
Signed-off-by: Stewart X Addison <[email protected]>
| stringParam('CUSTOM_TARGET', "", ''' | ||
| Only used when the custom target is specified in TARGET , e.g. jdk_custom, langtools_custom, system_custom, etc., CUSTOM_TARGET=path to the test class to execute<br> | ||
| stringParam('CUSTOM_TARGET', "", | ||
| '''Only used when the custom target is specified in TARGET , e.g. jdk_custom, langtools_custom, system_custom, etc., CUSTOM_TARGET=path to the test class to execute (typically ending in .java or .sh)<br> |
There was a problem hiding this comment.
This is statement is not incorrect, but doesn't cover several other cases, so may mislead people into thinking CUSTOM_TARGET can only be used for a single test class (when it can also be used to point to a directory or one can also pass a list of test classes separated by spaces).
There was a problem hiding this comment.
Noting that more exhaustive documentation for these parameters is captured here: https://github.com/adoptium/aqa-tests/blob/master/doc/pages/JenkinFeatures.md
There was a problem hiding this comment.
It would definitely be great to have some sort of reference to the directory usage too somehow for the casual user! (Either via a link to another doc, or including another example in the list that's already in the parameter description) I'm thinking that including a .sh example in the space-separated parameter might be preferable to this extra bit (the java/sh bit was something I added first before putting in the fix to show the full description)
Noting that more exhaustive documentation for these parameters is captured here: https://github.com/adoptium/aqa-tests/blob/master/doc/pages/JenkinFeatures.md
I'm having a look at that page but CUSTOM_TARGET is only referenced in a table that only says Required when TARGET=jdk_custom and I can't see a reference to "directory" or "folder" in that doc. If there is a good list of those things it would be great to reference it in the description.
There was a problem hiding this comment.
Maybe this should be a separate issue/PR now :-)
There was a problem hiding this comment.
Yes, let's get the EXTRA_OPTIONS and JVM_OPTIONS changes in (and make other improvements as part of the documentation EPIC for aqa-tests)
| stringParam('EXTRA_OPTIONS', "", "Use this to append options to the test command") | ||
| stringParam('JVM_OPTIONS', "", "Use this to replace the test original command line options") | ||
| stringParam('EXTRA_OPTIONS', "", "Use this to append options to the test command e.g. to add extra JVM flags") | ||
| stringParam('JVM_OPTIONS', "", "Use this to replace the original options to the test command e.g. to override JVM flags") | ||
| stringParam('APPLICATION_OPTIONS', "", "Use this to append options to the test application") |
Adding an example of what you might use this for. With the replace parameter being
JVM_OPTIONSit's not entirely clear thatEXTRA_OPTIONS(withoutJVMin the name) can be used for extra VM options as opposed to just parameters for the test harness - this should remove any ambiguity from me or others in the future.Ref adoptium/infrastructure#3065 (comment)