RFR: 8349533: Refactor validator tests shell files to java [v2]

[Weijun Wang]

On Fri, 21 Feb 2025 16:57:32 GMT, Mikhail Yankelevich <duke at openjdk.org> wrote:

>> Changed shell files to be java tests:
>> * ./validator/certreplace.sh
>> * ./validator/samedn.sh
>
> Mikhail Yankelevich has updated the pull request incrementally with one additional commit since the last revision:
> 
>   keyStore is not used to delete, cleanup of the calls, minor refactoring

test/jdk/sun/security/validator/CertReplace.java line 28:

> 26:  */
> 27: 
> 28: import java.io.FileInputStream;

Remove the "This test is called by certreplace.sh" line above.

test/jdk/sun/security/validator/CertReplace.java line 64:

> 62:  *
> 63:  * @run main/othervm CertReplace samedn.jks samedn1.certs
> 64:  * @run main/othervm CertReplace samedn.jks samedn2.certs

Must these tests run in `othervm`? They haven't changed any static VM properties.

test/jdk/sun/security/validator/CertReplace.java line 82:

> 80:         final String intAliase = "int";
> 81:         final String userAlias = "user";
> 82:         final String caAlias = "ca";

Do you really believe creating these variables help the program looking better? There are so many string concatenations in the keytool commands. If it were me, I would remove every variable for a string literal except for `ktBaseParameters`.

On the other hand, it's OK to put `"changeit".toCharArray()` into a variable.

test/jdk/sun/security/validator/CertReplace.java line 120:

> 118:                         new FileInputStream(intCertFileName)
> 119:                 )
> 120:         };

Use `CertUtils.getCertFromStream`. Also, it's better to close the file. Maybe Windows dislikes open files.

test/jdk/sun/security/validator/CertReplace.java line 147:

> 145:                               "-selfcert -alias " + caAlias);
> 146:         keyStore.deleteEntry(intAliase);
> 147:         keyStore.deleteEntry(userAlias);

Call `keyStore.store` to actually remove the 2 entries inside the keystore file. But this needs to be done before the `-selfcert` call, otherwise, the old cert would overwrite the new one.

test/jdk/sun/security/validator/CertReplace.java line 197:

> 195: 
> 196:         // 3. Remove user for cacerts
> 197:         keyStore.deleteEntry(userAlias);

Call `keyStore.store` to write to the keystore.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23727#discussion_r1965994249
PR Review Comment: https://git.openjdk.org/jdk/pull/23727#discussion_r1966022164
PR Review Comment: https://git.openjdk.org/jdk/pull/23727#discussion_r1965999262
PR Review Comment: https://git.openjdk.org/jdk/pull/23727#discussion_r1966008483
PR Review Comment: https://git.openjdk.org/jdk/pull/23727#discussion_r1966005851
PR Review Comment: https://git.openjdk.org/jdk/pull/23727#discussion_r1966016971