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