On Thu, 20 Oct 2022 23:34:05 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/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
