On Wed, 6 Jul 2022 06:40:32 GMT, KIRIYAMA Takuya <d...@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?
-------------
PR: https://git.openjdk.org/jdk/pull/9389
