On Fri, 7 Oct 2022 18:24:02 GMT, Justin Lu <d...@openjdk.org> wrote:
> Issue: Resource bundle name does not follow proper naming conventions
> according to [getBundle
> method](https://docs.oracle.com/javase/8/docs/api/java/util/ResourceBundle.html#getBundle-java.lang.String-java.util.Locale-)
> for base name parameter
>
> Fix: Modified bundle name to be a fully qualified class and added regression
> tests.
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?
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.
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.
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.
test/jdk/javax/sql/testng/test/rowset/ValidateResourceBundleAccess.java line 53:
> 51: // is found from the resource bundle
> 52: try {
> 53: jrs.getMetaData();
The test should fail if the execution of `getMetaData()` succeeds.
test/jdk/javax/sql/testng/test/rowset/ValidateResourceBundleAccess.java line 59:
> 57: // Now tests via CachedRowSet
> 58: try {
> 59: crs.execute();
Ditto, as above.
-------------
PR: https://git.openjdk.org/jdk/pull/10612
