resource bundle for junit test error strings #698

lilgreenbird · 2018-05-16T22:25:40Z

No description provided.

codecov-io · 2018-05-17T00:27:34Z

Codecov Report

Merging #698 into dev will increase coverage by 0.42%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##                dev     #698      +/-   ##
============================================
+ Coverage     47.96%   48.38%   +0.42%     
- Complexity     2579     2612      +33     
============================================
  Files           113      114       +1     
  Lines         26596    26601       +5     
  Branches       4454     4454              
============================================
+ Hits          12756    12871     +115     
+ Misses        11696    11668      -28     
+ Partials       2144     2062      -82

Flag	Coverage Δ	Complexity Δ
#JDBC42	`47.94% <ø> (-0.02%)`	`2572 <ø> (-7)`
#JDBC43	`48.29% <ø> (?)`	`2610 <ø> (?)`

Impacted Files	Coverage Δ	Complexity Δ
...om/microsoft/sqlserver/jdbc/ReaderInputStream.java	`44.94% <0%> (ø)`	`16% <0%> (ø)`	⬇️
.../com/microsoft/sqlserver/jdbc/SQLServerJdbc43.java	`40% <0%> (ø)`	`0% <0%> (?)`
...m/microsoft/sqlserver/jdbc/SQLServerResultSet.java	`33.21% <0%> (+0.07%)`	`245% <0%> (+1%)`	⬆️
...rc/main/java/com/microsoft/sqlserver/jdbc/dtv.java	`63.41% <0%> (+0.11%)`	`0% <0%> (ø)`	⬇️
...om/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java	`52.67% <0%> (+0.19%)`	`241% <0%> (+2%)`	⬆️
...oft/sqlserver/jdbc/SQLServerPreparedStatement.java	`50.68% <0%> (+0.24%)`	`157% <0%> (ø)`	⬇️
.../microsoft/sqlserver/jdbc/SQLServerDataSource.java	`45.19% <0%> (+0.25%)`	`66% <0%> (ø)`	⬇️
.../com/microsoft/sqlserver/jdbc/SQLServerDriver.java	`76.98% <0%> (+0.27%)`	`25% <0%> (ø)`	⬇️
...m/microsoft/sqlserver/jdbc/SQLServerStatement.java	`59.84% <0%> (+0.39%)`	`141% <0%> (+7%)`	⬆️
.../microsoft/sqlserver/jdbc/SQLServerConnection.java	`46.28% <0%> (+0.58%)`	`303% <0%> (+15%)`	⬆️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1153362...dd5c66a. Read the comment docs.

cheenamalhotra

Added some comments for few changes - rest everything looks good to me.

cheenamalhotra · 2018-05-22T23:21:53Z

src/test/java/com/microsoft/sqlserver/jdbc/AlwaysEncrypted/AESetup.java

@@ -140,7 +141,7 @@ private static void readFromFile(String inputFile,
        filePath = Utils.getCurrentClassPath();
        try {
            File f = new File(filePath + inputFile);
-            assumeTrue(f.exists(), "Aborting test case since no java key store and alias name exists!");
+            assumeTrue(f.exists(), "R_noKeyStore");


Isn't that supposed to be TestResource.getResource(.....)

cheenamalhotra · 2018-05-22T23:23:27Z

src/test/java/com/microsoft/sqlserver/jdbc/AlwaysEncrypted/CallableStatementTest.java

-            assertEquals(callableStatement.getObject(15), callableStatement.getObject(16), "Test for output parameter fails.\n");
-            assertEquals(callableStatement.getObject(17), callableStatement.getObject(18), "Test for output parameter fails.\n");
+                    TestResource.getResource("R_outputParamFailed"));
+            assertEquals(callableStatement.getObject(13), callableStatement.getObject(14), TestResource.getResource("R_outputParamFailed"));


I think we accidently removed getObject(11) and getObject(12) checks here.

cheenamalhotra · 2018-05-22T23:24:10Z

src/test/java/com/microsoft/sqlserver/jdbc/AlwaysEncrypted/CallableStatementTest.java

+                    TestResource.getResource("R_outputParamFailed"));
+            assertEquals(callableStatement.getObject(13), callableStatement.getObject(14), TestResource.getResource("R_outputParamFailed"));
+            assertEquals(callableStatement.getObject(15), callableStatement.getObject(16), TestResource.getResource("R_outputParamFailed"));
+TestResource.getResource("R_outputParamFailed");


This line is doing nothing - lets fix it

cheenamalhotra · 2018-05-22T23:27:52Z

src/test/java/com/microsoft/sqlserver/jdbc/AlwaysEncrypted/JDBCEncryptionDecryptionTest.java

-                    assertTrue(matches, "\nDecryption failed with getObject() at index: " + i + ", " + (i + 1) + ", " + (i + 2)
-                            + ".\nExpected Value at index: " + index);
+                    assertTrue(matches, TestResource.getResource("R_decryptionFailed") + "getObject(): " + i + ", " + (i + 1) + ", " + (i + 2)
+                            + ".\n" + TestResource.getResource("R_expectedValue") + index);


R_expectedValue is "Expected Value:", lets have another String here for "Expected Value at index:"

cheenamalhotra · 2018-05-22T23:30:54Z

src/test/java/com/microsoft/sqlserver/jdbc/bulkCopy/ISQLServerBulkRecordIssuesTest.java

@@ -145,11 +150,11 @@ public void testBinaryColumnAsByte() throws Exception {
        try (SQLServerBulkCopy bcOperation = new SQLServerBulkCopy(connectionString)) {
            bcOperation.setDestinationTableName(destTable);
            bcOperation.writeToServer(bData);
-            fail("BulkCopy executed for testBinaryColumnAsByte when it it was expected to fail");
+            fail("R_expectedFailPassed");


Don't we need from TestResource?

cheenamalhotra · 2018-05-22T23:31:15Z

src/test/java/com/microsoft/sqlserver/jdbc/bulkCopy/ISQLServerBulkRecordIssuesTest.java

@@ -172,11 +177,11 @@ public void testBinaryColumnAsString() throws Exception {
        try (SQLServerBulkCopy bcOperation = new SQLServerBulkCopy(connectionString)) {
            bcOperation.setDestinationTableName(destTable);
            bcOperation.writeToServer(bData);
-            fail("BulkCopy executed for testBinaryColumnAsString when it it was expected to fail");
+            fail("R_expectedFailPassed");


Same here too..

cheenamalhotra · 2018-05-22T23:35:01Z

src/test/java/com/microsoft/sqlserver/jdbc/databasemetadata/DatabaseMetaDataTest.java


        //Go through all columns.
        ResultSet rs1 = databaseMetaData.getColumnPrivileges(null, null, rsTables.getString("TABLE_NAME"), "%");

+        MessageFormat form2 = new MessageFormat(TestResource.getResource("R_nameEmpty"));
+        Object[][] msgArgs2 = {{"Category"}, {"SCHEMA"}, {"Table"}, {"COLUMN"}, {"GRANTOR"}, {"GRANTEE"}, {"PRIVILEGE"}, {"IS_GRANTABLE"}};


This is Nice! I liked the way we are using MessageFormat in this PR! 👍

ulvii · 2018-05-25T16:19:26Z

.gitattributes

@@ -1,33 +1,2 @@
 # Auto detect text files and perform LF normalization
-* text=auto


Could you explain why we are removing these lines? ...A better question, do we want to remove all of them?

we shouldn't be normalizing anything files should be left alone as is so they can be edited in all platforms. There is no need to specify individual extensions, wild card specifies all should be left alone.

ulvii · 2018-05-25T16:36:43Z

src/test/java/com/microsoft/sqlserver/jdbc/TestResource.java

+            {"R_connShouldNotBeOpen", "Connection should not be open"},
+            {"R_incorrectDriverNameFormat", "Driver name is not a correct format! "},
+            {"R_incorrectDriverVersionFormat", "Driver version number should be four parts! "},
+            {"R_previousShouldThrow", "Previous should have thrown an exception"},


Most of these messages are very specific and I don't see them being used in any other test.

cheenamalhotra

Looks good now!

lilgreenbird · 2018-05-29T22:24:27Z

resource bundle for error message strings in junit tests

* 623 fix * 623 change stash * Prepared Statement Caching fix for 'handle not found' errors * Fix for PS Caching issue - Calling reset instead of on type def changes * Updated comparison * Change back assert check. * Adding call to removeReference back + Fix for Batch processes intermittent failures. * Removed DBName and made changes to resetPrepStmtHandle method * Check for null handle before proceed * Adding Old Constrcutor back to AKV Implementation * Making baseURL final * Remove unnecessary code. * Update snapshot * Feature | AKV Old Constructor changes - Reformatted code + Deprecated old Constructor and added a new constructor with 1 param * Mark computed columns as IS_GENERATEDCOLUMN in the result set returned by getColumns() (#695) * Fix | getColumns() API, changed column name from SS_IS_COMPUTED to IS_AUTOINCREMENT per JDBC specs | issue #600 * Fix | getColumns() API, changed column name from SS_IS_COMPUTED to IS_GENERATEDCOLUMN per JDBC specs | issue #600 * fix issue with redirection * Fix | PS Caching - Remove commented lines * Trigger Appveyor test * Fix | AKV Old Constructor - Calling the other constructor instead. * Fix | Reversed null checks * Resolving alignment problems and comments * Applied formatter * Fix | Fix some of the Javadoc warnings (#702) * Resolved maven build warnings and java warnings regarding deprecated API (#701) * Resolving maven warnings * Removing jreVersion property Does not make sense now that we use final name in the build itself. Only used in 1 place, hard-coding java version for different builds as that's what it represents anyways. * java warnings * resource bundle for junit test error strings (#698) resource bundle for error message strings in junit tests * Add support for JDK 10 in both Maven and Gradle (#691) * Feature | Added support for JDK 10 in both Maven and Gradle - builds jre10 jars for the driver, replacing jre9 * JDK 10 | Merge 42 classes to base classes to reduce class redundancy. * JDK 10 | Attempt to run JDK 10 with Appveyor * Remove unwanted space * Updating Travis script to use JDK 10 * Testing without addons * Update script for Jacoco report to build 43 profile * Revert driver changes for 42 compliance - to be added in a separate PR * Revert Test class changes for 42 compliance - to be done in a separate PR * Reformatted code * Add ID to jacoco plugin execution task * Kerberos Constrained Delegation Impersonated Credential Expiry fix (#636) fix for automatic credential discarding * update felix to 3.5.0 * Revised implementation Decided to not dispose user created credentials at all. * Updated flag set location * changes for 6.5.3 preview release * Revert "changes for 6.5.3 preview release" This reverts commit 5c6ccd3. * Changes in preparation for 6.5.3 preview release (#710) * changes for preview release * requested changes * jre version update changes

* 623 fix * 623 change stash * Prepared Statement Caching fix for 'handle not found' errors * Fix for PS Caching issue - Calling reset instead of on type def changes * Updated comparison * Change back assert check. * Adding call to removeReference back + Fix for Batch processes intermittent failures. * Removed DBName and made changes to resetPrepStmtHandle method * Check for null handle before proceed * Adding Old Constrcutor back to AKV Implementation * Making baseURL final * Remove unnecessary code. * Use Bulk Copy API for batch insert operation * Parse bug fixing and test added * bug fix + additional tests * change reflection for testing * more test changes * Add parsing logic for -- comment * refactoring * Update snapshot * Bug fix / testing change * Reflect comment change * Feature | AKV Old Constructor changes - Reformatted code + Deprecated old Constructor and added a new constructor with 1 param * Mark computed columns as IS_GENERATEDCOLUMN in the result set returned by getColumns() (#695) * Fix | getColumns() API, changed column name from SS_IS_COMPUTED to IS_AUTOINCREMENT per JDBC specs | issue #600 * Fix | getColumns() API, changed column name from SS_IS_COMPUTED to IS_GENERATEDCOLUMN per JDBC specs | issue #600 * fix issue with redirection * Fix | PS Caching - Remove commented lines * Trigger Appveyor test * Fix | AKV Old Constructor - Calling the other constructor instead. * Fix | Reversed null checks * Resolving alignment problems and comments * Refactor two Bulk files into a common parent * javadoc changes * Applied formatter * fix problem with precision / scale * Fix | Fix some of the Javadoc warnings (#702) * fix issue with setting all to true * Resolved maven build warnings and java warnings regarding deprecated API (#701) * Resolving maven warnings * Removing jreVersion property Does not make sense now that we use final name in the build itself. Only used in 1 place, hard-coding java version for different builds as that's what it represents anyways. * java warnings * make bamoo fixes * resource bundle for junit test error strings (#698) resource bundle for error message strings in junit tests * undo some changes made to SQLServerConnection * apply resource bundling changes * Add support for JDK 10 in both Maven and Gradle (#691) * Feature | Added support for JDK 10 in both Maven and Gradle - builds jre10 jars for the driver, replacing jre9 * JDK 10 | Merge 42 classes to base classes to reduce class redundancy. * JDK 10 | Attempt to run JDK 10 with Appveyor * Remove unwanted space * Updating Travis script to use JDK 10 * Testing without addons * Update script for Jacoco report to build 43 profile * Revert driver changes for 42 compliance - to be added in a separate PR * Revert Test class changes for 42 compliance - to be done in a separate PR * Reformatted code * Add ID to jacoco plugin execution task * Kerberos Constrained Delegation Impersonated Credential Expiry fix (#636) fix for automatic credential discarding * update felix to 3.5.0 * Revised implementation Decided to not dispose user created credentials at all. * Updated flag set location * changes for 6.5.3 preview release * Revert "changes for 6.5.3 preview release" This reverts commit 5c6ccd3. * Changes in preparation for 6.5.3 preview release (#710) * changes for preview release * requested changes * jre version update changes * snapshot updates post release * remove on_dw, and remove redundant fmtonly * formatting * fix for getSchema when using "-" in name * Reformatting + adding more tests * inherit the connection property in statement + fix issue with null / empty string being passed in as values * Request Boundary methods - beginRequest()/endRequest() implementation (#708) * Add | Request Boundary Methods - beginRequest()/endRequest() implementation * Fix | Remove unused import from AbstractTest * Fix | Applying review comments * Fix | Moving RequestBoundaryMethodsTest.java to connection package * added error message in resource file and changed files accordingly * comment revisions * use TestResource * test changes removed finals removed database creation tracking * drop database before creating * replaced dropDBIfExists with Utils function * added try-with-resources nest avoid manually closing statements, and safetly handles resources. * Fixing logic / adding more tests * dont use test database in tests * Change exception handling as per JDBC specs * Add | Add missing license headers (#725) * remove some comments * Enable verify data (#724) Fix to enable data verification in Junit tests. Also addresses intermittent failures with Time/Timestamp where the precision was being inaccurately judged. * Fix | Refactored socket creation to simplify handling of socket creation Refactors socket creation in SocketFinder.findSocket(...) to simplify handling of socket creation. When the host resolves to a single address the driver now defers to getConnectedSocket(...) to create the socket without spawning any threads. This happens regardless of whether we're running on an IBM JDK. Previously the single address case would still use NIO on an IBM JDK. On non-IBM JDKs the driver now handles both IPv4 and IPv6 addresses concurrently with a single shared timeout. Previously hosts that resolved to both types of addresses were allowed half the timeout for socket creation per address type with the resolution performed sequentially. * reflect comments * Add support for UTF-8 feature extension. (#722) * Add | Support for UTF8 changes * changed how logger works, refactored code in SQLServerBulkCommon due to that, changed exception being thrown to BatchUpdateException, added same logic for parsing in executeLargeBatch, and added tests accordingly. * add more tests, make the prepared statement property go away * Feature | Introduce support for "Data Classification Specifications" on fetched resultsets (#709) * Feature | Data Classification Project | Phase 1 (contains temporary skipping 2 bytes) * Feature | Data Classification - Removing extra bytes added before * Feature | Data Classification - Added new test class for testing Data Classification support in the driver * Remove one println * Feature | Repackaged newly added files for Data Classification + improvements in source code * Feature | Changing tokens to bytes instead of int * Feature | Making variables private * Formatted code + dropTable method called from Utils * Feature | Data Classification - Changes as per review comments * Fix | Review comment changes * Change exception codes to follow series * Fix Conflict issue * Added missing Javadocs and headers for all new files * Added getter/setter public for the useBulkCopyForBatchInsert connection property. * Change implementation of child classes a bit * Remove dependencies from tests that are from outside required libraries * also remove hex from DBTable * Fix bamboo problem + refactor test code * Replace all connection and statements with try blocks * change spacing * refactor code * refactoring * Fix | Making driver default compliant to JDBC 4.2 Specs and update ADAL4J dependency to 1.6.0 (#711) * Feature | Added support for JDK 10 in both Maven and Gradle - builds jre10 jars for the driver, replacing jre9 * JDK 10 | Merge 42 classes to base classes to reduce class redundancy. * JDK 10 | Attempt to run JDK 10 with Appveyor * Remove unwanted space * Updating Travis script to use JDK 10 * Testing without addons * Update script for Jacoco report to build 43 profile * Minor fix in formatting to avoid conflicts * moving driver specific functions for SQLServerPreparedStatement * Remove unwanted code + Update Adal4J library dependency * changes for CallableStatement repeptitive delcarations * Remove an extra bracket due to conflict * changes for ISQLServerConnection there are problems with moving all Driver sepcific public methods. SQLServerConnectionPoolProxy also implements this interface and there are many public APIs (such as preparedstmt cacheing stuff) which it doesn't implement, and cannot be moved into the interface at this time. * lambda touch-up should generally stick to 1 line if possible. * changes for ISQLServerDataSource * updates for ISQLServerResultSet * Improvements | Missing interface APIs added + Code improvements * More changes for Interface missing methods * Implemented missing methods in SQLServerConnectionPoolProxy * Removed ISQLServerConnection43 for duplicated method definitions * Added APIs in interface for SQLServerResultSet * More cleanup done * Fix minor issues * Fix test failures and implement Serialization for HashKey * Fix JavaDoc errors and warnigs * More changes for CallableStatement APIs * More changes for Statement and Prepared Statament public APIs * Javadoc fix * More changes for SQL Server Bulk Record interface * Callable Statement missing APIs for Interface * Add missing desciptions * Reverting pom.xml change for this PR * Attempt to resolve conflicts * Remove Interface as not needed. * Added missing docs * Changes for Clob/Blob classes for compliance * Update ADAL4J with latest version * Changes for Data Source classes * Minor fixes to the new changes * Fix for failing tests * More changes for compliance * Add Javadocs and class headers * Fixed Malformed HTML Error in Javadocs * javadoc changes * more javadoc changes to make the abbreviations more clear * fix unchecked warning issue * Change HashKey in the driver to 256 Hash * Add Interface back to SQLServerConnection43 class * Revert "Change HashKey in the driver to 256 Hash" This reverts commit e6bef4e. * Changes for exceptions to throw SQLServerException type * 6.5.4 preview release changelog (#731) Release | Changelog for 6.5.4 preview release (#731) * Fix Conflict issues with master branch

susanh added 2 commits May 16, 2018 18:02

resource bundle for junit test error strings

58f07ef

resource bundle for junit test error strings

635a1f1

lilgreenbird assigned cheenamalhotra May 16, 2018

susanh added 2 commits May 16, 2018 19:11

fixed typo

1f47684

fixed typo

9506554

lilgreenbird unassigned cheenamalhotra May 16, 2018

susanh added 2 commits May 16, 2018 19:49

fixed typo

3d441d9

fixed typo

8e5ae2b

lilgreenbird assigned cheenamalhotra, ulvii, peterbae and rene-ye May 17, 2018

cheenamalhotra requested review from rene-ye, cheenamalhotra, ulvii and peterbae May 17, 2018 16:03

cheenamalhotra unassigned rene-ye, cheenamalhotra, ulvii and peterbae May 17, 2018

cheenamalhotra reviewed May 22, 2018

View reviewed changes

cheenamalhotra added this to the 6.5.3 milestone May 23, 2018

review updates

1a7c573

ulvii reviewed May 25, 2018

View reviewed changes

cheenamalhotra previously approved these changes May 29, 2018

View reviewed changes

more review changes

c33a325

lilgreenbird dismissed cheenamalhotra’s stale review via c33a325 May 29, 2018 18:52

Merge branch 'dev' into resource

dd5c66a

cheenamalhotra approved these changes May 29, 2018

View reviewed changes

peterbae approved these changes May 29, 2018

View reviewed changes

rene-ye approved these changes May 29, 2018

View reviewed changes

lilgreenbird closed this May 29, 2018

lilgreenbird reopened this May 29, 2018

lilgreenbird merged commit 4f1b3e1 into microsoft:dev May 29, 2018

cheenamalhotra mentioned this pull request Jun 1, 2018

Implement Resource Bundle in Test framework #466

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

resource bundle for junit test error strings #698

resource bundle for junit test error strings #698

lilgreenbird commented May 16, 2018

codecov-io commented May 17, 2018 •

edited

Loading

cheenamalhotra left a comment

cheenamalhotra May 22, 2018

cheenamalhotra May 22, 2018

cheenamalhotra May 22, 2018

cheenamalhotra May 22, 2018

cheenamalhotra May 22, 2018

cheenamalhotra May 22, 2018

cheenamalhotra May 22, 2018

ulvii May 25, 2018

lilgreenbird May 28, 2018

ulvii May 25, 2018

cheenamalhotra left a comment

lilgreenbird commented May 29, 2018

		@@ -1,33 +1,2 @@
		# Auto detect text files and perform LF normalization
		* text=auto

resource bundle for junit test error strings #698

resource bundle for junit test error strings #698

Conversation

lilgreenbird commented May 16, 2018

codecov-io commented May 17, 2018 • edited Loading

Codecov Report

cheenamalhotra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cheenamalhotra left a comment

Choose a reason for hiding this comment

lilgreenbird commented May 29, 2018

codecov-io commented May 17, 2018 •

edited

Loading