-
Notifications
You must be signed in to change notification settings - Fork 5.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
8294989: ResourceBundle naming convention issue in JdbcRowSetResourceBundle.java #10612
Changes from 1 commit
c35f1af
f346917
609a6d0
646ee97
1ce2671
3e49163
c86950b
c67d789
0aa978c
1aa881b
2ea3e42
2e39c41
8f0b6ac
0f50339
da3aeac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,10 +26,9 @@ | |
import java.sql.SQLException; | ||
import javax.sql.rowset.RowSetProvider; | ||
import org.testng.annotations.Test; | ||
import util.BaseTest; | ||
import org.testng.annotations.BeforeClass; | ||
import static org.testng.Assert.*; | ||
|
||
|
||
/** | ||
* @test | ||
* @bug 8294989 | ||
|
@@ -43,11 +42,14 @@ public class ValidateResourceBundleAccess{ | |
// Expected JDBCResourceBundle message, crsreader.connecterr | ||
private static final String rsReaderError = "Internal Error in RowSetReader: no connection or command"; | ||
|
||
@Test | ||
public void testResourceBundleAccess() throws SQLException { | ||
// Checking against English messages, set to US Locale | ||
// Checking against English messages, set to US Locale | ||
@BeforeClass | ||
public void setEnglishEnvironment() { | ||
Locale.setDefault(Locale.US); | ||
} | ||
|
||
@Test | ||
public void testResourceBundleAccess() throws SQLException { | ||
var rsr = RowSetProvider.newFactory(); | ||
var crs =rsr.createCachedRowSet(); | ||
var jrs = rsr.createJdbcRowSet(); | ||
|
@@ -57,17 +59,17 @@ public void testResourceBundleAccess() throws SQLException { | |
try { | ||
jrs.getMetaData(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test should fail if the execution of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, will make that change |
||
// Unexpected case where exception is not forced | ||
var msg = "$$$ Error: SQLException was not caught!%n"; | ||
throw new RuntimeException(String.format(msg)); | ||
throw new RuntimeException( | ||
String.format("$$$ Error: SQLException was not caught!%n")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As you are not passing any parameters, this can just be a String omitting the "%n" Also, it would tweak the String to be "Expected SQLException.... thrown" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will adjust the exception message |
||
} catch (SQLException sqe) { | ||
assertTrue(sqe.getMessage().equals(invalidState)); | ||
} | ||
// Now tests via CachedRowSet | ||
try { | ||
crs.execute(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto, as above. |
||
// Unexpected case where exception is not forced | ||
var msg = "$$$ Error: SQLException was not caught!%n"; | ||
throw new RuntimeException(String.format(msg)); | ||
throw new RuntimeException( | ||
String.format("$$$ Error: SQLException was not caught!%n")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment about not needing to use String.format. Also, it would tweak the String to be "Expected SQLException.... thrown" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, Naoto actually recommended the same with the .format(), but I thought that I should preserve the newline. Like you said %n should not be needed, will make the fix. |
||
} catch (SQLException e) { | ||
assertTrue(e.getMessage().equals(rsReaderError)); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be placed in a separate static method with
@BeforeAll
annotation.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, used TestNg's @BeforeClass