Skip to content

adding more jck test targets into jck playlist #235

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Feb 2, 2018

Conversation

terryzuoty
Copy link

  • update jck build.xml to ensure it uses the same javac in JAVA_BIN
  • add JCK_VERSION to use same test target on all JCK versions
  • and JCK custom test targets and JCK_TEST_TARGET variable
  • user can run JCK test subset with new added JCK custom test targets
  • export JCK_TEST_TARGET to define which JCK test subset to run
  • add daily and weekly JCK test tags and test targets

Signed-off-by: Tianyu Zuo tianyu@ca.ibm.com

@terryzuoty terryzuoty self-assigned this Jan 30, 2018
@terryzuoty terryzuoty force-pushed the jck branch 2 times, most recently from 0656578 to 0116752 Compare January 30, 2018 22:12
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.

Before we commit to adding the weekly tags, I think we should consider what we plan to run 'weekly' or per release. We want to run the entire set of JCKs, so we do not need the tag (we would just run 'jck' target).

For the daily, by which we likely mean running against nightly builds at Adopt, we are actually setting a level rather than group (much like sanity).

So maybe as we wait for the update to testkitgen, we hold off on those two tags.

jck/README.md Outdated

# How-to Run customized JCK test targets

There are three custom JCK test targets `jck-runtime-custom`, `jck-runtime-custom` and `jck-runtime-custom`. With these three test targets, you can run custom JCK subsets.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a cut'n'paste error, the 3 targets have identical names? do you mean to include jck-devtools-custom and jck-compiler-custom in that list.

Copy link
Author

Choose a reason for hiding this comment

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

ah, thanks Shelley, this is a cut'n error. updated with newer changeset.

@terryzuoty
Copy link
Author

For the weekly builds (a.k.a. whole set of JCK tests), I have some thoughts but it may be wrong. Please correct me if any part is inappropriate.

I think the definition of daily and weekly would be different between OpenJ9 and AdoptOpenJDK.
AdoptOpenJDK is providing a whole JDK so it needs to run the 'whole' set of JCK tests, which means it need to run runtime, compiler and devtools.
However for OpenJ9, it provides the VM part of JDK, I assume the whole set of JCK tests for OpenJ9 is the JCK runtime suites. With the same reason, the requests of 'daily' tests may be different as well. Do we want to use the same playlist for both OpenJ9 and OpenJDK?

Another issue is about how the whole set of JCK tests will be executed: serial or parallel? Serial execution would be easy to implement as just one target can do the task. If it is parallel, do we want to parallel weekly targets which have been defined in the jck playlist, or run jck.xxx.custom target several times with different passed in JCK test subset?

@terryzuoty
Copy link
Author

I have deleted those weekly targets since the weekly run are not clear yet.

@smlambert
Copy link
Contributor

@TianyuZuo - it is true that different projects will have different preferences for what JCKs they want to run against different builds, which is a reason to be careful not to over-label.

At AdoptOpenJDK there is no weekly build. There are nightly builds and release builds.

There is a JCK test build that is run against the release build that should include ALL of the JCK tests, therefore can use "make jck" to accomplish it.

For nightly builds at AdoptOpenJDK, we may choose to run a subset of the JCKs (topic under discussion in issue #186 ). That JCK test build is limited by time, and I would imagine would be a subset from the vm and api tests.

For other projects like that wish to use this test material from AdoptOpenJDK (assuming they have their own JCK test materials), they can choose the full test suite and run "make jck" or once we decide on the nightly subset, use the tag name that we agree upon for the make target, or run a set of individual make targets of their choosing. I am saying hold off on this tag for now, as you know we are about to update testkitgen, and we'd either add a level called 'nightly' or leverage the existing 'sanity' or 'extended' levels for that purpose. At present, I want to try and use the same playlist for both AdoptOpenJDK and OpenJ9 testing.

For now, I do not care to optimize the running of these tests in parallel, as we do not really have enough machines to support this. If we eventually do support parallel testing, we would be doing that in a Jenkinsfile and likely would do that using the jck.xxx.custom targets as you suggest. Note we will always still need to support a serial/manual run outside of Jenkins.

@smlambert
Copy link
Contributor

If you have tried running this from your branch, and are satisfied that it works as expected, then I will merge this for now. We will have to update the playlist when we bring in the updates to testkitgen, at which point we can finalize what level we use for which builds.

* update jck build.xml to ensure it uses the same javac in JAVA_BIN 
* add JCK_VERSION to use same test target on all JCK versions
* and JCK custom test targets and JCK_TEST_TARGET variable
* user can run JCK test subset with new added JCK custom test targets
* export JCK_TEST_TARGET to define which JCK test subset to run 
* add daily and weekly JCK test tags and test targets
* add copyright

Signed-off-by: Tianyu Zuo <tianyu@ca.ibm.com>
@terryzuoty
Copy link
Author

Hi @smlambert
Thanks for the info, which helps a lot. So I will keep the plyalist.xml as is (without weekly test targets, only contains daily targets). And once the tkg is synced from OpenJ9, they will also be updated.

Tests have been tested locally, worked and passed as expected.

jck/README.md Outdated

3. Export `JAVA_HOME=<your_JDK_root>` as an environment variable
3. Export `JCK_VERSION=<your_jck_version>` as an environment variable or pass it in makefile when run make commands. For example `export JCK_VERSION=jck8b`
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo?.. Could be phrased as "when run as a make command".

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for pointing it out Mesbah, updated with newer chageset

@terryzuoty terryzuoty changed the title WIP: adding more jck test targets into jck playlist adding more jck test targets into jck playlist Feb 1, 2018
jck/README.md Outdated

1. Follow the Steps 1 - 4 mentioned above.

2. Export `JCK_TEST_TARGET=<jck_test_subset>` as an environment variable or pass it in makefile when run make commands. For example `export JCK_TEST_TARGET=api/java_math`
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo?.. Could be phrased as "when run as a make command".

Copy link
Author

Choose a reason for hiding this comment

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

updated

jck/README.md Outdated

3. Make sure the JCK test subset is available in JCK test material folder, a.k.a. `$(JCK_ROOT)/$(JCK_VERSION)/`.

4. If you need to add extra Java options to JCK tests, you could export `JCK_JAVA_ARGS="<java_options>"`. Then extra added Java options would bee added to JCk test during execution.
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: be, JCK

Copy link
Author

Choose a reason for hiding this comment

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

updated

* added JCK_JAVA_ARGS which accept extra Java options
* extra Java options will be added to JCK test during execution.
* updated README to show how to use this use variable

Signed-off-by: Tianyu Zuo <tianyu@ca.ibm.com>
Copy link
Contributor

@Mesbah-Alam Mesbah-Alam left a comment

Choose a reason for hiding this comment

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

LGTM.

@terryzuoty terryzuoty merged commit e85594b into adoptium:master Feb 2, 2018
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants