RFR: 8294989: ResourceBundle naming convention issue in JdbcRowSetResourceBundle.java [v4]

[Justin Lu]

On Wed, 19 Oct 2022 22:21:04 GMT, Naoto Sato <naoto at 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