RFR: 8289797: tools/launcher/I18NArgTest.java fails on Japanese Windows enviromnent

[KIRIYAMA Takuya]

On Fri, 8 Jul 2022 12:10:29 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> I removed a section of via JDK_JAVA_OPTIONS because including main class is not allowed in the specification.
>> This behavior is added in JDK-8170832, which add JAVA_OPTIONS environment variable. At this time, this test is mismatch with the specification.
>> I tried to test and get Passed on Japanese Windows environment.
>> Could you review this fix, please?
>
> The change looks fine to me since in its current form the usage of `JDK_JAVA_OPTIONS` is incorrect.
> 
> Having said that and looking at the history of this file, I think, this part of the test was added to check if the `java` launcher would be able to read an environment variable (specifically the `JDK_JAVA_OPTIONS`) whose value was in `MS932` encoding, and pass it along as a correctly encoded String, all the way to the main method of the application. If we remove this section of the test, then we would be skipping that code path completely.
> 
> Perhaps this part of the test could be modified a bit to let it pass the `JDK_JAVA_OPTIONS` environment variable with a `MS932` encoded value that acts some option to the java launcher, instead of being the argument to the main method? That way, you won't have to specify the main class name in the value of the environment variable. Specifically, something like:
> 
> 
> diff --git a/test/jdk/tools/launcher/I18NArgTest.java b/test/jdk/tools/launcher/I18NArgTest.java
> index fa09736da2f..2ba724d6f1d 100644
> --- a/test/jdk/tools/launcher/I18NArgTest.java
> +++ b/test/jdk/tools/launcher/I18NArgTest.java
> @@ -97,12 +97,17 @@ public class I18NArgTest extends TestHelper {
>  
>          // Test via JDK_JAVA_OPTIONS
>          Map<String, String> env = new HashMap<>();
> -        String cmd = "-Dtest.src=" + TEST_SOURCES_DIR.getAbsolutePath() +
> -                " -Dtest.classes=" + TEST_CLASSES_DIR.getAbsolutePath() +
> -                " -cp " + TEST_CLASSES_DIR.getAbsolutePath() +
> -                " I18NArgTest " + unicodeStr + " " + hexValue;
> -        env.put("JDK_JAVA_OPTIONS", cmd);
> -        tr = doExec(env, javaCmd);
> +        String sysPropName = "foo.bar";
> +        // pass "-Dfoo.bar=<unicodestr>" through the JDK_JAVA_OPTIONS environment variable.
> +        // we expect that system property value to be passed along to the main method with the
> +        // correct encoding
> +        String jdkJavaOpts = "-D" + sysPropName + "=" + unicodeStr;
> +        env.put("JDK_JAVA_OPTIONS", jdkJavaOpts);
> +        tr = doExec(env,javaCmd,
> +                "-Dtest.src=" + TEST_SOURCES_DIR.getAbsolutePath(),
> +                "-Dtest.classes=" + TEST_CLASSES_DIR.getAbsolutePath(),
> +                "-cp", TEST_CLASSES_DIR.getAbsolutePath(),
> +                "I18NArgTest", unicodeStr, hexValue, sysPropName);
>          System.out.println(tr.testOutput);
>          if (!tr.isOK()) {
>              System.err.println(tr);
> @@ -125,5 +130,23 @@ public class I18NArgTest extends TestHelper {
>                  "expected:" + expected + " obtained:" + hexValue;
>              throw new RuntimeException(message);
>          }
> +        if (args.length == 3) {
> +            // verify the value of the system property matches the expected value
> +            String sysPropName = args[2];
> +            String sysPropVal = System.getProperty(sysPropName);
> +            if (sysPropVal == null) {
> +                throw new RuntimeException("Missing system property " + sysPropName);
> +            }
> +            String sysPropHexVal = "";
> +            for (int i = 0; i < sysPropVal.length(); i++) {
> +                sysPropHexVal = sysPropHexVal.concat(Integer.toHexString(sysPropVal.charAt(i)));
> +            }
> +            System.out.println("System property " + sysPropName + " computed hex value: "
> +                    + sysPropHexVal);
> +            if (!sysPropHexVal.equals(expected)) {
> +                throw new RuntimeException("Unexpected value in system property, expected "
> +                        + expected + ", but got " + sysPropHexVal);
> +            }
> +        }
>      }
>  }
> 
> I haven't tested this change, so you might have to experiment with it a bit. What do you think?

@jaikiran 
Thank you for your comment. I agree to the additional check. 
I run the test you proposed, but it failed. I'm surveying that reason.

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

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