On Thu, 6 Feb 2025 12:12:35 GMT, Mikhail Yankelevich <d...@openjdk.org> wrote:
> * ./util/Pem/encoding.sh is now a java test
> * also added the validations for the test results, not only an error check
test/jdk/sun/security/util/Pem/PemEncoding.java line 47:
> 45: File.separator,
> 46: File.separator,
> 47: File.separator);
nit: the following may be easier to read
Suggestion:
final var certPath = Path.of(System.getProperty("test.src", "."))
.getParent()
.resolve("HostnameChecker")
.resolve("cert5.crt");
test/jdk/sun/security/util/Pem/PemEncoding.java line 58:
> 56: final var subProcessOutput = result.getStdout();
> 57: final var subProcessErr = result.getStderr();
> 58: System.out.printf("Output:\n%s%n", subProcessOutput);
No need to print the output here, JTreg automatically creates a .log file in
the scratch directory with the subprocess stderr and stdout for debugging
test/jdk/sun/security/util/Pem/PemEncoding.java line 72:
> 70: // Some systems may automatically convert non-ASCII to ?
> 71: // so this will detect the signature in all possible scenarios
> 72: final var isUTF16Used = matcher.find() ||
> subProcessOutput.contains("??: ?");
IMO, this is confusing. **isUTF16Used** seems misleading, as it actually checks
for non-ASCII characters in the output, and it does not confirm that UTF-16 was
used.
If the goal is to double-check the default file encoding in the subprocess,
instead I would simply add a verification in PemEncodingTest:
if (!"UTF-16".equals(Charset.defaultCharset().displayName())) {
throw new RuntimeException("File encoding is not UTF-16");
}
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23492#discussion_r1944946184
PR Review Comment: https://git.openjdk.org/jdk/pull/23492#discussion_r1944964050
PR Review Comment: https://git.openjdk.org/jdk/pull/23492#discussion_r1945089066
