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

Trigger tests with standard set #2024

Merged
merged 1 commit into from
Sep 24, 2020
Merged

Conversation

sophia-guo
Copy link
Contributor

@sophia-guo sophia-guo commented Aug 6, 2020

Set standard set of top-level test targets run nightly, weekly. Per release combined nightly and weekly targets will run

  1. Keep all pipeline jobs runs the same targets as before.
  2. Keep job configuration testList as simple as a placeholder
  3. Possible key value for testlist

no test enabled: test: false, or no test as key of configuration.
standard set --> test: "default"
others -->

                test                : [
                        nightly: [customized target lists, could be empty too],
                        weekly : [customized target lists, could be empty too]
                ],

Close #1116

Signed-off-by: Sophia Guo sophia.gwf@gmail.com

@sophia-guo sophia-guo requested a review from smlambert August 6, 2020 20:55
@sophia-guo
Copy link
Contributor Author

For now test for x64Linux in jdk11u_pipeline_config.groovy is updated to "default", which means it will run the full standard daily for nightly build, weekly( Cron triggered) and nightly+weekly for release builds.

Keep others as before, we can go through to see if we need to update for others.

@karianna karianna added the enhancement Issues that enhance the code or documentation of the repo in any way label Aug 7, 2020
@karianna karianna added this to the August 2020 milestone Aug 7, 2020
@@ -124,12 +124,25 @@ class Builder implements Serializable {
}

List<String> getTestList(Map<String, ?> configuration) {
if (configuration.containsKey("test")) {
List<String> nightly = ['sanity.openjdk', 'sanity.system', 'extended.system', 'sanity.perf', 'sanity.external']
Copy link
Contributor

Choose a reason for hiding this comment

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

These look like constants to me, should we hoist up to the class level?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, updated.

if ( testJobType == "nightly" ) {
return (configuration.test as Map).get("nightly") as List<String>
} else {
return ((configuration.test as Map).get("nightly") as List<String>) + (configuration.test as Map).get("weekly") as List<String>
Copy link
Contributor

Choose a reason for hiding this comment

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

Long return statement, prefer splitting it out into individual statements with a simpler return at the end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, updated.

pipelines/build/common/build_base_file.groovy Show resolved Hide resolved
}
return ['sanity.openjdk', 'sanity.system', 'extended.system', 'sanity.perf', 'sanity.external']
Copy link
Contributor

Choose a reason for hiding this comment

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

this looks like a constant, should it be extracted and named?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.

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.

Let's walk through this draft together next week @sophia-guo, thanks for getting it started!

@sophia-guo sophia-guo marked this pull request as draft August 11, 2020 18:06
@sophia-guo sophia-guo force-pushed the testTrigger2 branch 2 times, most recently from d1a0956 to 663998f Compare August 14, 2020 18:44
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.

Thanks for stepping me through this @sophia-guo and for updating as per @karianna's review notes!

One more request, as you have done for jdk11/jdk15, can you update the PR to use the default set for jdk8 as it is like jdk11, an LTS release. That way LTS releases (8/11) and current (at this point in time 15) and next (a.k.a. tip where it appears build team copy/pastes pipelines from current) will all use the default set.

@sophia-guo sophia-guo force-pushed the testTrigger2 branch 2 times, most recently from 852df00 to bd4a69f Compare August 17, 2020 15:03
@sophia-guo
Copy link
Contributor Author

@smlambert Thanks for reminding. jdk8u updated.

@sophia-guo sophia-guo marked this pull request as ready for review August 18, 2020 13:57
@sophia-guo sophia-guo changed the title WIP: trigger with standard set trigger with standard set Aug 18, 2020
@smlambert
Copy link
Contributor

run tests

@sophia-guo
Copy link
Contributor Author

Could someone run a PR build? Thanks.

@smlambert smlambert changed the title trigger with standard set Trigger tests with standard set Aug 18, 2020
@smlambert
Copy link
Contributor

@karianna - if you are good with @sophia-guo's updates to your review comments, could you trigger pr tests or let me know how I might do so (my run tests comment above did not seem to have an effect).

@aahlenst
Copy link
Contributor

run tests

@sophia-guo
Copy link
Contributor Author

The user should be added to White list. Either directly update PR jobs configuration or Admin need to comment 'add to whitelist'.

@gdams
Copy link
Member

gdams commented Aug 18, 2020

Run tests

@sophia-guo sophia-guo force-pushed the testTrigger2 branch 2 times, most recently from 1aa1cb2 to ceaf851 Compare August 18, 2020 17:28
@sophia-guo
Copy link
Contributor Author

Run tests

@karianna
Copy link
Contributor

run tests

@sxa
Copy link
Member

sxa commented Sep 14, 2020

@sophia-guo where are we with this? Is it ready to go in once build lockdown is complete (and conflicts resolved)?

@sophia-guo
Copy link
Contributor Author

The former PR tests ran good. I haven't got the chance to check the new pipeline-build-check failure. Will check it and resolve the conflicts.

@sophia-guo
Copy link
Contributor Author

sophia-guo commented Sep 14, 2020

run tests

@adoptopenjdk-github-bot
Copy link
Contributor

🟢 PR TESTER RESULT 🟢

✅ All pipelines passed! ✅

@sophia-guo
Copy link
Contributor Author

It is ready to go in once build lockdown is complete.

@adoptopenjdk-github-bot
Copy link
Contributor

🟢 PR TESTER RESULT 🟢

✅ All pipelines passed! ✅

1 similar comment
@adoptopenjdk-github-bot
Copy link
Contributor

🟢 PR TESTER RESULT 🟢

✅ All pipelines passed! ✅

@adoptopenjdk-github-bot
Copy link
Contributor

🟠 PR TESTER RESULT 🟠

❎ Some pipelines failed or the job was aborted! ❎
See the pipeline-build-check below for more information...

@sophia-guo
Copy link
Contributor Author

run tests

Signed-off-by: Sophia Guo <sophia.gwf@gmail.com>
@karianna
Copy link
Contributor

All good but waiting for OpenJ9 15 release before landing this

@adoptopenjdk-github-bot
Copy link
Contributor

🟠 PR TESTER RESULT 🟠

❎ Some pipelines failed or the job was aborted! ❎
See the pipeline-build-check below for more information...

@sophia-guo
Copy link
Contributor Author

Can anyone help this? If the PR test build was gone and I have no idea what is the failure. Thanks.

@karianna
Copy link
Contributor

run tests

@adoptopenjdk-github-bot
Copy link
Contributor

🟢 PR TESTER RESULT 🟢

✅ All pipelines passed! ✅

@karianna karianna merged commit 773c2eb into adoptium:master Sep 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues that enhance the code or documentation of the repo in any way
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update how tests are launched via Build Pipelines
7 participants