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

have cause of SQLServerException exception at any place where it possible #202

Merged

Conversation

xiangyushawn
Copy link
Contributor

for issue #139

@codecov-io
Copy link

codecov-io commented Mar 14, 2017

Codecov Report

Merging #202 into dev will increase coverage by 0.08%.
The diff coverage is 4%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev     #202      +/-   ##
============================================
+ Coverage     33.45%   33.53%   +0.08%     
- Complexity     1488     1489       +1     
============================================
  Files            97       97              
  Lines         23390    23390              
  Branches       3840     3840              
============================================
+ Hits           7824     7845      +21     
+ Misses        14002    13980      -22     
- Partials       1564     1565       +1
Flag Coverage Δ Complexity Δ
#JDBC41 33.47% <4%> (+0.09%) 1485 <0> (ø) ⬇️
#JDBC42 33.41% <4%> (+0.1%) 1486 <0> (+9) ⬆️
Impacted Files Coverage Δ Complexity Δ
.../microsoft/sqlserver/jdbc/SQLServerConnection.java 41.1% <0%> (ø) 227 <0> (ø) ⬇️
...microsoft/sqlserver/jdbc/SQLServerADAL4JUtils.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...crosoft/sqlserver/jdbc/KeyStoreProviderCommon.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...in/java/com/microsoft/sqlserver/jdbc/IOBuffer.java 37.33% <0%> (+0.38%) 0 <0> (ø) ⬇️
...QLServerColumnEncryptionAzureKeyVaultProvider.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...c/main/java/com/microsoft/sqlserver/jdbc/Util.java 39.9% <0%> (ø) 52 <0> (ø) ⬇️
...om/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java 45.05% <0%> (ø) 182 <0> (-2) ⬇️
...rc/main/java/com/microsoft/sqlserver/jdbc/dtv.java 33.96% <0%> (ø) 0 <0> (ø) ⬇️
...m/microsoft/sqlserver/jdbc/SQLServerStatement.java 57.95% <0%> (ø) 128 <0> (ø) ⬇️
...oft/sqlserver/jdbc/SQLServerBulkCSVFileRecord.java 39.62% <20%> (+1.41%) 25 <0> (ø) ⬇️
... and 4 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 b2f5c37...33221e3. Read the comment docs.

Copy link
Contributor

@v-nisidh v-nisidh left a comment

Choose a reason for hiding this comment

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

Please create some tests which expects exceptions and test if exceptions having appropriate cause.

Copy link
Contributor

@v-nisidh v-nisidh left a comment

Choose a reason for hiding this comment

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

Approved. Please fix 2 Review Comments before merge.

}

private void dropWaitForDelayProcedure(SQLServerConnection conn) throws SQLException {
String sql = " IF EXISTS (select * from sysobjects where id = object_id(N'" + waitForDelaySPName
Copy link
Contributor

Choose a reason for hiding this comment

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

Use new Utility for delete object. Utils.dropObjectIfExists(...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This PR is pretty old, before we have this Utils.dropObjectIfExists(...) method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed to Utils.dropProcedureIfExists after merging/updating with Dev branch

*
* @return location of resource file
*/
static String getCurrentClassPath() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move this to Util class as it is very much common to whole framework.

@v-suhame : Possibly we already might be having this functionality of getCurrentClassPath somewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we have getCurrentClassPath() in BulkCopyCSVTest as well. I moved them into Utils class.

@xiangyushawn xiangyushawn merged commit 0a0f330 into microsoft:dev Apr 4, 2017
@xiangyushawn xiangyushawn deleted the add-cause-to-SQLServerException branch April 4, 2017 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants