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.

Sign up for GitHub

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

Jump to bottom

Update modularity tests based on Java 11 changes in SDK #196

Merged
merged 1 commit into from
Nov 27, 2018
Merged

Update modularity tests based on Java 11 changes in SDK #196

merged 1 commit into from
Nov 27, 2018

Conversation

Mesbah-Alam
Copy link
Contributor

Resolves #150
Signed-off-by: Mesbah Alam [email protected]

@Mesbah-Alam Mesbah-Alam self-assigned this Nov 27, 2018
@Mesbah-Alam Mesbah-Alam mentioned this pull request Nov 27, 2018
Closed
@Mesbah-Alam
Copy link
Contributor Author

Mesbah-Alam commented Nov 27, 2018 •
edited
Loading

This PR contains the following changes:

  1. Removed reference to java.transaction module from all tests (as this module no longer exists in JDK11) and replaced them with java.xml.
  2. Upgrade module path related tests have been updated to exclude tests for "upgrading an upgradale system/platform module -- as the only "upradale" platform modules were ee modules (java.transaction) which have been removed in jdk11.
  3. Updated CpMpJink test and removed use of --class-for-name (this option is no longer supported in jdk11).
  4. Updated TestJDKInternalAPI test and removed reference to sun.reflect.Reflection as it no longer exists in JDK11.
  5. Removed exclusion of modularity project from system test project build list.

@Mesbah-Alam
Copy link
Contributor Author

Mesbah-Alam commented Nov 27, 2018 •
edited
Loading

The PR has been tested here: https://ci.adoptopenjdk.net/view/Test_grinder/job/Grinder/465/tapResults/

Note: the two failing layers stress tests could e real issues. I've opened #197 to investigate them, they will be excluded from the playlist.

@Mesbah-Alam
Copy link
Contributor Author

Mesbah-Alam commented Nov 27, 2018 •
edited
Loading

Playlist changes to include modularity tests to jdk11 are contained here: adoptium/aqa-tests#703

sophia-guo
sophia-guo reviewed Nov 27, 2018
View reviewed changes
openjdk.build/build.xml Outdated
@@ -55,19 +51,13 @@ limitations under the License.
<ant antfile="${source_root}/openjdk.test.serialization/build.xml" dir="${source_root}/openjdk.test.serialization" inheritAll="false"/>
<ant antfile="${source_root}/openjdk.test.util/build.xml" dir="${source_root}/openjdk.test.util" inheritAll="false"/>
<ant antfile="${source_root}/openjdk.test.jlm/build.xml" dir="${source_root}/openjdk.test.jlm" inheritAll="false"/>
<ant antfile="${source_root}/openjdk.test.modularity/build.xml" dir="${source_root}/openjdk.test.modularity" inheritAll="false"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Will the source be built by any jdk version or build.xml has flag to do condition compiling?

Copy link
Contributor Author

@Mesbah-Alam Mesbah-Alam Nov 27, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Good catch! It should not be built for Java 8, it can be built for Java 9 and up. Need to add a check here. WIll update the PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sophia-guo - a check has been added.

@sophia-guo
Copy link
Contributor

Just to confirm that if excluded tests "upgrading an upgradale system/platform module" and CpMpJink test "use of --class-for-name" are excluded for all jdk versions?

@Mesbah-Alam
Copy link
Contributor Author

Mesbah-Alam commented Nov 27, 2018 •
edited
Loading

Modularity doesn't exist in Java 8. And I believe it is safe to not write and maintain complicated tests that are based on modules that do not exit in java 11 but only in Java 9 as Modularity had still been evolving during Java 9-- lots of things have been changed since (e.g. --class-for-name and some more of such options have been redesigned).

@sophia-guo
Copy link
Contributor

Better to run a grinder with jdk8 before merging.

@Mesbah-Alam
Copy link
Contributor Author

@sophia-guo - agreed. Here's the grinder with JDK 8 - with a modularity test target - it builds successfully and runs nothing : https://ci.adoptopenjdk.net/view/Test_grinder/job/Grinder/469/console

@Mesbah-Alam Mesbah-Alam merged commit 7c36d5b into adoptium:master Nov 27, 2018
@Mesbah-Alam Mesbah-Alam deleted the enableModularityTestsOnJDK11 branch November 27, 2018 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sophia-guo sophia-guo sophia-guo approved these changes

Assignees

@Mesbah-Alam Mesbah-Alam

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Mesbah-Alam @sophia-guo