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

Update PARALLEL parameter in testJobTemplate #2973

Closed
wants to merge 46 commits into from

Conversation

abigailsleek
Copy link

I just updated the PARALLEL description in the testjobTemplate.

updated the PARALLEL description
@abigailsleek abigailsleek changed the title Update on https://github.com/adoptium/aqa-tests/issues/2943 testJobTemplate Update on testJobTemplate Oct 10, 2021
@smlambert
Copy link
Contributor

Testing triggered via https://ci.adoptopenjdk.net/view/work-in-progress/job/WIP_Test_Job_Auto_Gen/61 (awaiting executor)

Thanks for your PR @abigailsleek - please ensure you have signed the ECA agreement (details at: https://www.eclipse.org/legal/ECA.php), noting the email used in the Eclipse profile needs to match that used for Github ID.

@abigailsleek
Copy link
Author

abigailsleek commented Oct 10, 2021 via email

@smlambert
Copy link
Contributor

smlambert commented Oct 10, 2021

This change has updated the NUM_MACHINES parameter, instead of the PARALLEL parameter, so it needs to be updated.

Output from test job generated via WIP_Test_Job_Auto_Gen/61

Screen Shot 2021-10-10 at 12 07 22 PM

Please restore the NUM_MACHINES description to what it was, and move the PARALLEL description up to the PARALLEL parameter.

@smlambert smlambert changed the title Update on testJobTemplate Update PARALLEL parameter in testJobTemplate Oct 10, 2021
@abigailsleek
Copy link
Author

abigailsleek commented Oct 10, 2021 via email

stringParam('NUM_MACHINES', NUM_MACHINES, '''Optional. It has to be used with PARALLEL=Dynamic or PARALLEL=NodesByIterations<br/>
Use with PARALLEL=Dynamic, NUM_MACHINES cannot be greater than the number of matched nodes on Jenkins<br/>
Use with PARALLEL=NodesByIterations, NUM_MACHINES cannot be greater than 20<br/>''')
stringParam('NUM_MACHINES', NUM_MACHINES, '''Optional. Parallel mode: <br/>None // run test(s) in serial <br/>
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the NUM_MACHINES parameter, keep the description for this as it was.

Suggested change
stringParam('NUM_MACHINES', NUM_MACHINES, '''Optional. Parallel mode: <br/>None // run test(s) in serial <br/>
stringParam('NUM_MACHINES', NUM_MACHINES, '''Optional. It has to be used with PARALLEL=Dynamic or PARALLEL=NodesByIterations<br/>
Use with PARALLEL=Dynamic, NUM_MACHINES cannot be greater than the number of matched nodes on Jenkins<br/>
Use with PARALLEL=NodesByIterations, NUM_MACHINES cannot be greater than 20<br/>''')

buildenv/jenkins/testJobTemplate Outdated Show resolved Hide resolved
@abigailsleek
Copy link
Author

abigailsleek commented Oct 10, 2021 via email

Recently corrected changes made to the PARALLEL description.
Copy link
Author

@abigailsleek abigailsleek left a comment

Choose a reason for hiding this comment

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

I just updated changes to the PARALLEL description. My previous commit changed the NUM_MACHINES description. This, I have just fixed.

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.

Need to use 3 single quotes ''' when surrounding a multi-line block (rather than double-quote "). Please see suggested changes inline.

@@ -375,7 +375,10 @@ ARCH_OS_LIST.each { ARCH_OS ->
sectionHeader('Additional test options if you wish to run in various parallel modes')
sectionHeaderStyle(sectionHeaderHelpTextStyleCss)
}
choiceParam('PARALLEL', PARALLEL_LIST, "Optional. Parallel mode")
choiceParam('PARALLEL', PARALLEL_LIST, "Optional. Parallel mode: <br/>None // run test(s) in serial <br/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
choiceParam('PARALLEL', PARALLEL_LIST, "Optional. Parallel mode: <br/>None // run test(s) in serial <br/>
choiceParam('PARALLEL', PARALLEL_LIST, '''Optional. Parallel mode: <br/>None // run test(s) in serial <br/>

choiceParam('PARALLEL', PARALLEL_LIST, "Optional. Parallel mode: <br/>None // run test(s) in serial <br/>
Dynamic // dynamically divides tests according to NUM_MACHINES and runs them in parallel. Please also provide NUM_MACHINES. <br/>
NodesByIterations // run the same test(s) in parallel according to NUM_MACHINES. You can also provide ITERATIONS. This is mainly for debugging intermittent issues. <br/>
Subdir // run tests in parallel according to subdir. This is mainly for external tests.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Subdir // run tests in parallel according to subdir. This is mainly for external tests.")
Subdir // run tests in parallel according to subdir. This is mainly for external tests.<br/>''')

@smlambert
Copy link
Contributor

smlambert commented Oct 11, 2021

New test triggered via: https://ci.adoptopenjdk.net/view/work-in-progress/job/WIP_Test_Job_Auto_Gen/62

indicates an issue:

Processing DSL script buildenv/jenkins/testJobTemplate
ERROR: startup failed:
testJobTemplate: 378: expecting anything but ''\n''; got it anyway @ line 378, column 113.
   // run test(s) in serial <br/>
                                 ^

1 error

Suggesting to start the block with ''' and end the block with <br/>'''

abigailsleek and others added 4 commits October 11, 2021 15:05
* finish adding GIT_ISSUE_STATUS to the json

* add error handling

* delete output file

* change the name of input file

* change the common.xml

* fix the then

* change the to outputproperty in common.xm;

* add the error msg

* handle the case if branch not exit go to origin/branch

* solve the nested echo problem

* remove the change relaed to adoptium#2882

* move the if inside

* accept sha for openjdk_systemtest

* accept sha for openj9-systemtest

* get rid of repetetive code for cloneing stf

* fix the outputproperty unable to overwrite

* revert the change

* fixfix the outputproperty unable to overwrite for systemtest

* fix formatting

* change the logic for the openjdk system test

* change logic for openj9-systemtest

* fix no ending if

* fix failonerror

* fix the return

* change the name of sha
Signed-off-by: Jason Feng <fengj@ca.ibm.com>
Signed-off-by: Mesbah-Alam <Mesbah_Alam@ca.ibm.com>
@smlambert
Copy link
Contributor

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.

The resulting generated test job looks good. Would prefer not to pull in all the whitespace changes come in with this PR.

@abigailsleek
Copy link
Author

abigailsleek commented Oct 13, 2021 via email

psaunderualberta and others added 10 commits October 13, 2021 09:57
…doptium#2894)


Signed-off-by: Paul Saunders <44712655+psaunderualberta@users.noreply.github.com>
Signed-off-by: renfeiw <renfeiw@ca.ibm.com>
…dows-x86 (adoptium#2998)

* Update ProblemList_openjdk17.txt

* Update ProblemList_openjdk17.txt
* add sainamercy to conributing list

* Exclude 2 jfr testcases on aarch64 platform

* Revert "Revert "add sainamercy to conributing list""

This reverts commit ade06be.
…doptium#2981)

* Switch api query back to AdoptOpenjdk for openj9 jdk
sainamercy and others added 8 commits October 20, 2021 08:01
I need to delete this file, then commit using a separate pull request
* Exclude 2 tests on window32 with jdk17

* Exclude 2 tests on window32 with jdk17

* Update ProblemList_openjdk17-openj9.txt

* Update ProblemList_openjdk17.txt
…ws-x86" (adoptium#3006)

* Update ProblemList_openjdk17.txt

* Update ProblemList_openjdk17.txt

Fix adoptium#3004

* Update ProblemList_openjdk17.txt

for windows-x86 platform

* Update ProblemList_openjdk17.txt

* Update ProblemList_openjdk17.txt

* Update ProblemList_openjdk17.txt
@abigailsleek
Copy link
Author

abigailsleek commented Oct 23, 2021 via email

sophia-guo and others added 17 commits October 25, 2021 10:33
Signed-off-by: Sophia Guo <sophia.gwf@gmail.com>
Signed-off-by: Mesbah-Alam <Mesbah_Alam@ca.ibm.com>
Signed-off-by: Mesbah-Alam <Mesbah_Alam@ca.ibm.com>
…3008)

* enhance the function getProperty() to deal with quotations

reword removed the echo  in getProperty()

reword removed the logic of parsing and removing the quotes in set_test_info()

reword updated set_test_info()

updated set_test_info() to resolve conflict

* modified set_test_info()

* modified line 226
Signed-off-by: Keith W. Campbell <keithc@ca.ibm.com>
- Add jdk_lang_VarHandleTest_j9 test with specific jit option

Signed-off-by: Longyu Zhang <longyu.zhang@ibm.com>
- VarHandleTestAccess tests fails for jdk17, so exclude them temporarily
- These tests will be moved to a new test with specific jit options

Signed-off-by: Longyu Zhang <longyu.zhang@ibm.com>
* Removed JRE_IMAGE in openjdk.mk

* Added DYNAMIC_COMPILE in testJobTemplate adoptium#3031

* Update testJobTemplate

Removed the DYNAMIC_COMPILE and its description after BUILD_LIST in testJobTemplate
…m#3105)

Signed-off-by: Mesbah-Alam <Mesbah_Alam@ca.ibm.com>
@smlambert
Copy link
Contributor

Guidance for ECA since this PR still fails the ECA check:
https://github.com/adoptium/documentation/blob/main/documentation-documentation/eca-sign-off.adoc

@smlambert
Copy link
Contributor

I will close this PR as stale. It may be best to start with a fresh branch after signing the ECA should you want to return to this @abigailsleek . Thanks.

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.