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

Naoto Sato naoto at openjdk.org
Fri Oct 21 01:23:55 UTC 2022


On Thu, 20 Oct 2022 23:34:05 GMT, Justin Lu <duke at openjdk.org> wrote:

>> Issue: Resource bundle name does not follow proper naming conventions according to [getBundle method](https://docs.oracle.com/en/java/javase/18/docs/api/java.base/java/util/ResourceBundle.html#getBundle(java.lang.String,java.util.Locale,java.lang.Module)) for base name parameter
>> 
>> Fix: Modified bundle name to be a fully qualified class and added regression tests.
>
> Justin Lu has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Run Validate_.java in othervm mode

Much better now. BTW, there seems to be a stray `.class` binary file included in this PR. Please remove it.

test/jdk/javax/sql/resourceBundleTests/ValidateGetBundle.java line 61:

> 59:                     Locale.US, JdbcRowSetResourceBundle.class.getModule());
> 60:             if (!expectBundle) {
> 61:                 String message = "$$$ '%s' shouldn't have loaded!%n";

variable `message` is not needed

test/jdk/javax/sql/resourceBundleTests/ValidateGetBundle.java line 67:

> 65:         } catch (MissingResourceException mr) {
> 66:             if (expectBundle) {
> 67:                 throw new RuntimeException(String.format("Error:%s%n", mr.getMessage()));

Probably the message could be more descriptive than a simple "Error". Also, instead of `mr.getMessage()`, use the 2-arg constructor that takes `mr` as the "cause". That would be straightforward.

test/jdk/javax/sql/testng/test/rowset/ValidateResourceBundleAccess.java line 49:

> 47:     public void testResourceBundleAccess() throws SQLException {
> 48:         // Checking against English messages, set to US Locale
> 49:         Locale.setDefault(Locale.US);

Could be placed in a separate static method with `@BeforeAll` annotation.

test/jdk/javax/sql/testng/test/rowset/ValidateResourceBundleAccess.java line 60:

> 58:             jrs.getMetaData();
> 59:             // Unexpected case where exception is not forced
> 60:             var msg = "$$$ Error: SQLException was not caught!%n";

The literal can directly be used as the argument to the constructor.

test/jdk/javax/sql/testng/test/rowset/ValidateResourceBundleAccess.java line 69:

> 67:             crs.execute();
> 68:             // Unexpected case where exception is not forced
> 69:             var msg = "$$$ Error: SQLException was not caught!%n";

Same as above

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

Changes requested by naoto (Reviewer).

PR: https://git.openjdk.org/jdk/pull/10612


More information about the core-libs-dev mailing list