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

Improve error message when adding wrong type to SqlParameterCollection #1547

Merged
merged 4 commits into from
Mar 18, 2022

Conversation

ErikEJ
Copy link
Contributor

@ErikEJ ErikEJ commented Mar 15, 2022

(to help conversions from System.Data.SqlClient)

fixes #1546

…tion

(to help conversions from System.Data.SqlClient)

fixes dotnet#1546
@DavoudEshtehari
Copy link
Contributor

Thanks @ErikEJ
Could you address the ParametersTest.CodeCoverageSqlClient failure too?

@DavoudEshtehari DavoudEshtehari added this to the 5.0.0-preview2 milestone Mar 15, 2022
@DavoudEshtehari DavoudEshtehari added the 💡 Enhancement Issues that are feature requests for the drivers we maintain. label Mar 15, 2022
@DavoudEshtehari DavoudEshtehari changed the title Improve error message when adding wrong type to to SqlParameterCollection Improve error message when adding wrong type to SqlParameterCollection Mar 15, 2022
@johnnypham
Copy link
Contributor

Test is still failing.

Copy link
Contributor

@DavoudEshtehari DavoudEshtehari left a comment

Choose a reason for hiding this comment

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

LGTM
For the sake of code coverage, may I ask you to include the following line of code in this block, please?

DataTestUtility.AssertThrowsWrapper<IndexOutOfRangeException>(() => opc["@p1"] = null, "A SqlParameter with ParameterName '@p1' is not contained by this SqlParameterCollection.");

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Mar 16, 2022

@DavoudEshtehari I have added the line, not sure how it releates to this PR...

@Wraith2
Copy link
Contributor

Wraith2 commented Mar 16, 2022

It doesn't, you're just getting columbo'ed ("just one more thing...") since code coverage is poor in places and you're editing the file.

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Mar 17, 2022

Happy to do a separate PR to increase code coverage.

Is there an issue for that?

@DavoudEshtehari
Copy link
Contributor

Happy to hear it @ErikEJ

I'm afraid we don't have an active issue for code coverage. We're working to add the code coverage report to our pipelines to verify any digression instantly that would be helpful to detect and improve it with the public.
In the meantime, you can use the internal reports if you had permission:

@DavoudEshtehari DavoudEshtehari merged commit d966a81 into dotnet:main Mar 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💡 Enhancement Issues that are feature requests for the drivers we maintain.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SqlParameterCollection.Add() gives confusing error message when mixing drivers
4 participants