On Wed, 19 Oct 2022 22:21:04 GMT, Naoto Sato <na...@openjdk.org> wrote:
>> Justin Lu has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Run Validate_.java in othervm mode
>
> test/jdk/javax/sql/resourceBundleTests/ValidateGetBundle.java line 45:
>
>> 43: private static final String PATH_TO_BUNDLE =
>> "com/sun/rowset/RowSetResourceBundle";
>> 44: // Default Locale
>> 45: private static final Locale DEFAULT_LOCALE = Locale.getDefault();
>
> Is using the default locale OK? What if the test is run under some locale
> where JDBC does not provide the localized bundle?
Switched default to constant US locale
> test/jdk/javax/sql/resourceBundleTests/ValidateGetBundle.java line 62:
>
>> 60: private static void testResourceBundleAccess(String bundleName,
>> boolean expectBundle) {
>> 61: try {
>> 62: var bundle = (PropertyResourceBundle)
>> ResourceBundle.getBundle(
>
> Is casting to `PropertyResourceBundle` needed? If you intend to check the
> bundle type, I'd explicitly check the type.
Thank you. Not needed, removed
> test/jdk/javax/sql/resourceBundleTests/ValidateGetBundle.java line 64:
>
>> 62: var bundle = (PropertyResourceBundle)
>> ResourceBundle.getBundle(
>> 63: bundleName, DEFAULT_LOCALE,
>> JdbcRowSetResourceBundle.class.getModule());
>> 64: System.out.printf("$$$ %s found as expected!%n", bundleName);
>
> I think `expectBundle` needs to be examined here. If it is `false`, it should
> throw an exception.
Added a check there for case when expectBundle is false
> test/jdk/javax/sql/testng/test/rowset/ValidateResourceBundleAccess.java line
> 42:
>
>> 40: private static final String invalidState = "Invalid state";
>> 41: // Expected JDBCResourceBundle message, crsreader.connecterr
>> 42: private static final String rsReaderError = "Internal Error in
>> RowSetReader: no connection or command";
>
> Should be tested in English environment, if the test is going to compare the
> result with English text.
Set default locale as US, running in othervm mode now as well
-------------
PR: https://git.openjdk.org/jdk/pull/10612
