RFR: JDK-8294994: Update Jarsigner and Keytool i18n tests to validate i18n compliance
Naoto Sato
naoto at openjdk.org
Mon Oct 10 19:54:10 UTC 2022
On Mon, 10 Oct 2022 18:18:55 GMT, Bill Huang <bhuang at openjdk.org> wrote:
> The jarsigner and keytool are localized into English, German, Japanese and Simplified Chinese. This task is to modify the existing i18n tests to validate i18n compliance in these tools.
>
> In addition, this task also contains changes for manual test enhancement and simplification which originated from [JDK-8292663](https://bugs.openjdk.org/browse/JDK-8292663.
Looks good overall. Some minor suggestions.
test/jdk/sun/security/tools/keytool/i18n.java line 62:
> 60: * @library /test/lib
> 61: * @run main/manual/othervm i18n zh CN
> 62: */
Do you need to triplicate these `@test` tags? Would 3-lines of `@run` suffice?
Also setting the locale by `-Duser.language/country` and `getProperty` them in the main would be preferable to passing them as the test case arguments.
test/jdk/sun/security/tools/keytool/i18n.java line 250:
> 248: private Thread currentThread = null;
> 249:
> 250: public static class DialogBuilder {
This seems to be a boilerplate for displaying the panel. Could this be separated from the test and converted into some library?
test/jdk/sun/security/tools/keytool/i18n.java line 334:
> 332: } else if (args.length == 2) {
> 333: Locale.setDefault(Locale.of(args[0], args[1]));
> 334: }
Can be eliminated with the suggestion above.
test/jdk/sun/security/tools/keytool/i18n.java line 335:
> 333: Locale.setDefault(Locale.of(args[0], args[1]));
> 334: }
> 335: final String LANG = Locale.getDefault().getDisplayLanguage();
Instead of `getDisplayLanguage()`, it should issue `getDisplayName()`, as for `zh-CN` case, it simply displays `Chinese` in the current impl. It's ambiguous whether it is simplified or traditional.
Also, `LANG` should be lowercase, as it is not a constant.
-------------
PR: https://git.openjdk.org/jdk/pull/10635
More information about the security-dev
mailing list