RFR: JDK-8292067 Convert test/sun/management/jmxremote/bootstrap shell tests to java version

Leonid Mesnik lmesnik at openjdk.org
Mon Aug 29 23:15:11 UTC 2022


On Mon, 22 Aug 2022 22:51:50 GMT, Bill Huang <duke at openjdk.org> wrote:

> This task convert 3 shell tests below to java version. 
> test/sun/management/jmxremote/bootstrap/RmiBootstrapTest.sh test/sun/management/jmxremote/bootstrap/RmiSslBootstrapTest.sh 
> test/sun/management/jmxremote/bootstrap/RmiSslNoKeyStoreTest.sh

test/jdk/sun/management/jmxremote/bootstrap/RmiBootstrapTest.java line 33:

> 31: import java.rmi.server.ExportException;
> 32: 
> 33: import java.util.*;

wildcards in import are not welcome, please avoid them
not needed to replace existing, just don't introduce them

test/jdk/sun/management/jmxremote/bootstrap/RmiBootstrapTest.java line 115:

> 113:             throws IOException {
> 114: 
> 115:         final Set names = server.queryNames(pattern, query);

Since you change these lines, might add template parameters here?
Set<ObjectName> names = ... 
and simplify if it is possible.
Not a request, just a proposal.

test/jdk/sun/management/jmxremote/bootstrap/RmiBootstrapTest.java line 166:

> 164:         if (args.length == 0) {
> 165:             throw new IllegalArgumentException("Argument is required for this" +
> 166:                     " test");

not needed to split lines

test/jdk/sun/management/jmxremote/bootstrap/RmiBootstrapTest.java line 700:

> 698:             }
> 699:         }
> 700:         System.err.println(

I think that the previous indentation was good enough. No need to change it.

test/jdk/sun/management/jmxremote/bootstrap/RmiBootstrapTest.java line 716:

> 714:             throws InterruptedException, IOException {
> 715:         String errStr = null;
> 716:         errStr = testConfiguration(conf);

Merge with errStr = null ?

test/jdk/sun/management/jmxremote/bootstrap/RmiTestBase.java line 47:

> 45:     static final String TEST_SRC = "@TEST-SRC@";
> 46: 
> 47:     static final boolean isWindows = System.getProperty("os.name").contains(

Check Platform.java.

test/jdk/sun/management/jmxremote/bootstrap/RmiTestBase.java line 112:

> 110: 
> 111:     static String getDefaultFileName(String basename) {
> 112:         final StringBuffer defaultFileName =

Why don't just create 
final String = System.getProperty(.._) + SEP + .... + basename; 
or even make static final variable like defaultFileNamePrefix = ... and 
return defaultFileNamePrefix + basename;

test/jdk/sun/management/jmxremote/bootstrap/RmiTestBase.java line 130:

> 128:      **/
> 129:     static String getDefaultStoreName(String basename) {
> 130:         final StringBuffer defaultFileName =

the same as previous

test/jdk/sun/management/jmxremote/bootstrap/RmiTestBase.java line 162:

> 160:             Utils.replaceFilesString(propertyFiles,
> 161:                     (s) -> s.replace(TEST_SRC, DEST)
> 162:                             .replaceAll("[/\\\\]", "\\\\\\\\"));

Could you please comment this replacement.

-------------

PR: https://git.openjdk.org/jdk/pull/9973


More information about the serviceability-dev mailing list