Skip to content
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

Closed
Closed
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);
Copy link
Member

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.

Copy link
Member Author

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

}

@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();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test should fail if the execution of getMetaData() succeeds.

Copy link
Member Author

Choose a reason for hiding this comment

The 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"));
Copy link
Contributor

Choose a reason for hiding this comment

The 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"

Copy link
Member Author

Choose a reason for hiding this comment

The 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();
Copy link
Member

Choose a reason for hiding this comment

The 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"));
Copy link
Contributor

Choose a reason for hiding this comment

The 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"

Copy link
Member Author

Choose a reason for hiding this comment

The 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));
}