JDK-8295756 Improve NonLocalRegistry Manual Test Process #10825

bwhuang-us · 2022-10-21T21:45:30Z

The current non local registry tests require a manual process that runs rmiregitrty on a different machine and changes the -Dregistry.host property in the source before running the tests on the local machine. This task is created to improve this manual process and provide a clearer instruction to the test engineer about the test requirement.

Tests include:
java/rmi/registry/nonLocalRegistry/NonLocalSkeletonTest.java
java/rmi/registry/nonLocalRegistry/NonLocalRegistryTest.java
javax/management/remote/nonLocalAccess/NonLocalJMXRemoteTest.java

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8295756: Improve NonLocalRegistry Manual Test Process

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/10825/head:pull/10825
$ git checkout pull/10825

Update a local copy of the PR:
$ git checkout pull/10825
$ git pull https://git.openjdk.org/jdk pull/10825/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 10825

View PR using the GUI difftool:
$ git pr show -t 10825

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/10825.diff

bridgekeeper · 2022-10-21T21:46:47Z

👋 Welcome back bhuang! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2022-10-21T21:49:46Z

@bwhuang-us The following labels will be automatically applied to this pull request:

core-libs
jmx
serviceability

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

mlbridge · 2022-10-21T21:52:47Z

Webrevs

AlanBateman · 2022-10-27T18:58:41Z

test/jdk/TEST.groups

@@ -200,6 +200,8 @@ jdk_time = \

 jdk_rmi = \
    java/rmi \
+    -java/rmi/registry/nonLocalRegistry/NonLocalRegistryTest.java \
+    -java/rmi/registry/nonLocalRegistry/NonLocalSkeletonTest.java \


RunTests.gmk runs jtreg with -automatic so I don't think you need to exclude the manual tests from these test groups.

mahendrachhipa · 2022-11-04T14:23:35Z

test/jdk/java/rmi/registry/nonLocalRegistry/NonLocalRegistryBase.java

+public class NonLocalRegistryBase {
+    static final String instructions =
+            "This is a manual test that requires rmiregistry run on a different host"
+                    + ". Login or ssh to a different host, install the latest JDK "


replace "latest JDK" with "JDK under test"

mahendrachhipa · 2022-11-04T14:30:03Z

test/jdk/javax/management/remote/nonLocalAccess/NonLocalJMXRemoteTest.java

@@ -154,4 +222,43 @@ private static void assertIsAccessException(Throwable ex) {
            throw new RuntimeException("AccessException did not occur when expected", ex);
        }
    }
+
+    private String readHostInput(int index) {


Duplicate code, this code is also in NonLocalRegistryBase.java, Please move this code in a common class under lib/jdk/test/lib. Or move this NonLocalRegistryBase.java under lib/jdk/test/lib and provide some generic name.

mahendrachhipa · 2022-11-04T15:11:27Z

test/jdk/java/rmi/registry/nonLocalRegistry/NonLocalRegistryTest.java

 */
-public class NonLocalRegistryTest {
+public class NonLocalRegistryTest extends NonLocalRegistryBase {


Move private static void assertIsAccessException(Throwable ex) method to NonLocalRegistryBase class.

mahendrachhipa · 2022-11-04T15:12:09Z

test/jdk/java/rmi/registry/nonLocalRegistry/NonLocalSkeletonTest.java

@@ -76,12 +78,17 @@
 * On the first host modify the @run command above to replace "rmi-registry-host"
 * with the hostname or IP address of the different host and run the test with jtreg.
 */
-public class NonLocalSkeletonTest {
+public class NonLocalSkeletonTest extends NonLocalRegistryBase {


Move private static void assertIsAccessException(Throwable ex) method to NonLocalRegistryBase class.

mahendrachhipa · 2022-11-04T15:17:43Z

test/jdk/javax/management/remote/nonLocalAccess/NonLocalJMXRemoteTest.java

@@ -154,4 +222,43 @@ private static void assertIsAccessException(Throwable ex) {
            throw new RuntimeException("AccessException did not occur when expected", ex);


private static void assertIsAccessException(Throwable ex) is define in all these three test classes, move this method in lib/jdk/test/lib

This method is test dependent that checks for specific keywords in the AccessException. It seems better remain inside the test classes.

mahendrachhipa · 2022-11-25T17:01:19Z

LGTM

AlanBateman · 2022-11-28T13:56:29Z

test/jdk/javax/management/remote/nonLocalAccess/NonLocalJMXRemoteTest.java

+        Set<InetAddress> myAddrs =
+                Set.copyOf(Arrays.asList(InetAddress.getAllByName(myHostName)));
+        Set<InetAddress> hostAddrs =
+                Set.copyOf(Arrays.asList(InetAddress.getAllByName(host)));


Is there a reason to change this from Set.of?

This is a workaround for duplicate elements detected by Set.of(). On my localhost, InetAddress.getAllByName(localhost returns duplicate IPv6 addresses with different scoped id which is not counted during comparison. This change shouldn't impact the goal of these tests which is to verify that registry modification request from client side is prohibited.

Thanks. You might want to add a comment to avoid drive-by suggestions to simplify it.

openjdk · 2022-11-29T00:52:45Z

@bwhuang-us this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout JDK-8295756
git fetch https://git.openjdk.org/jdk master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

bridgekeeper · 2023-01-04T21:51:01Z

@bwhuang-us This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper · 2023-03-02T03:31:44Z

@bwhuang-us This pull request has been inactive for more than 16 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

bwhuang-us added 2 commits October 21, 2022 14:28

JDK-8295756 Improve NonLocalRegistry Manual Test Process

98a2bdf

Updated TEST.groups

a190fc7

openjdk bot added the rfr Pull request is ready for review label Oct 21, 2022

openjdk bot added serviceability serviceability-dev@openjdk.org jmx jmx-dev@openjdk.org core-libs core-libs-dev@openjdk.org labels Oct 21, 2022

Updated test instruction messages.

a61d56e

AlanBateman reviewed Oct 27, 2022

View reviewed changes

Reverted exclusion of nonlocal registry tests.

5ee556a

mahendrachhipa reviewed Nov 4, 2022

View reviewed changes

Implemented review comments.

278564b

AlanBateman reviewed Nov 28, 2022

View reviewed changes

Implemented review comments.

cff3773

openjdk bot added the merge-conflict Pull request has merge conflict with target branch label Nov 29, 2022

Merge master

3ae1f79

openjdk bot removed the merge-conflict Pull request has merge conflict with target branch label Nov 29, 2022

bwhuang-us added 2 commits December 5, 2022 12:37

Reduced host input timeout to 20 minutes.

c564816

Reduced host input timeout to 5 minutes.

761cf90

bwhuang-us marked this pull request as draft January 17, 2023 18:13

openjdk bot removed the rfr Pull request is ready for review label Jan 17, 2023

bridgekeeper bot closed this Mar 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JDK-8295756 Improve NonLocalRegistry Manual Test Process #10825

JDK-8295756 Improve NonLocalRegistry Manual Test Process #10825

bwhuang-us commented Oct 21, 2022 •

edited by openjdk bot

Loading

bridgekeeper bot commented Oct 21, 2022

openjdk bot commented Oct 21, 2022

mlbridge bot commented Oct 21, 2022 •

edited

Loading

AlanBateman Oct 27, 2022

mahendrachhipa Nov 4, 2022

mahendrachhipa Nov 4, 2022

mahendrachhipa Nov 4, 2022

mahendrachhipa Nov 4, 2022

mahendrachhipa Nov 4, 2022

bwhuang-us Nov 4, 2022

mahendrachhipa commented Nov 25, 2022

AlanBateman Nov 28, 2022

bwhuang-us Nov 28, 2022

AlanBateman Nov 28, 2022

openjdk bot commented Nov 29, 2022

bridgekeeper bot commented Jan 4, 2023

bridgekeeper bot commented Mar 2, 2023

		@@ -154,4 +222,43 @@ private static void assertIsAccessException(Throwable ex) {
		throw new RuntimeException("AccessException did not occur when expected", ex);

JDK-8295756 Improve NonLocalRegistry Manual Test Process #10825

JDK-8295756 Improve NonLocalRegistry Manual Test Process #10825

Conversation

bwhuang-us commented Oct 21, 2022 • edited by openjdk bot Loading

Progress

Issue

Reviewing

bridgekeeper bot commented Oct 21, 2022

openjdk bot commented Oct 21, 2022

mlbridge bot commented Oct 21, 2022 • edited Loading

Webrevs

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mahendrachhipa commented Nov 25, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openjdk bot commented Nov 29, 2022

bridgekeeper bot commented Jan 4, 2023

bridgekeeper bot commented Mar 2, 2023

bwhuang-us commented Oct 21, 2022 •

edited by openjdk bot

Loading

mlbridge bot commented Oct 21, 2022 •

edited

Loading