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

Implement missing MetaData #isWrapperFor methods #94

Merged
merged 1 commit into from
Feb 1, 2017
Merged

Implement missing MetaData #isWrapperFor methods #94

merged 1 commit into from
Feb 1, 2017

Conversation

marschall
Copy link
Contributor

The driver currently implements all #unwrap methods but some of the
matching #isWrapperFor methods are missing. This is caused by the fix
for #12 which should also have included the #isWrapperFor methods.

  • implement DatabaseMetaData#isWrapperFor
  • implement ParameterMetaData#isWrapperFor
  • implement ResultSetMetaData#isWrapperFor

@AfsanehR-zz AfsanehR-zz added the Under Review Used for pull requests under review label Jan 5, 2017
@AfsanehR-zz
Copy link
Contributor

Thank you @marschall for submitting a pr. The changes in the three source code files are good. But we updated the tests to use the new framework, that's why a conflict exists in your pr now.
You could either remove bvt test updates from the pr or you could get the latest code and commit changes in the test with the new test framework. Thank you again.

@marschall
Copy link
Contributor Author

I committed the tests with the new test framework, the test that's failing is testCreatepreparedStatement which has no connection to my changes so I'm a bit confused.

@AfsanehR-zz
Copy link
Contributor

@marschall thank you. You are correct, that build fail was not because of your changes. The next commits you make should not cause any failures.


assertSame(rs, rs.unwrap(ResultSet.class));
assertSame(rs, rs.unwrap(ISQLServerResultSet.class));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please drop the tables before the test finishes. It will throw exception that object already exists.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right, ParameterMetaDataTest had the same issue.

The driver currently implements all #unwrap methods but some of the
matching #isWrapperFor methods are missing. This is caused by the fix
for #12 which should also have included the #isWrapperFor methods.

 - implement DatabaseMetaData#isWrapperFor
 - implement ParameterMetaData#isWrapperFor
 - implement ResultSetMetaData#isWrapperFor
@AfsanehR-zz AfsanehR-zz merged commit 2779416 into microsoft:dev Feb 1, 2017
@AfsanehR-zz
Copy link
Contributor

Thank you for your contribution @marschall

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Under Review Used for pull requests under review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants