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

Add SslOverTdsStream tests #497

Merged
merged 8 commits into from
Apr 23, 2020
Merged

Conversation

Wraith2
Copy link
Contributor

@Wraith2 Wraith2 commented Mar 30, 2020

I intend to change the internal class SslOverTdsStream to remove sync-over-async and remove some allocations. To do that I need to add tests to prevent adding of new bugs.

These use each pair of Read and Write overloads on Stream to write a known pattern of 4 encapsulated and 4 unencapsulated packets to a memory stream and then read them back and verify the data is the same. Reflection is used to find, instantiate and call FinishHandshake on the stream. I chose to do this rather than include the source files because it's easier in this case where there are only two methods required and it doesn't introduce a code share between dev and test.

@cheenamalhotra cheenamalhotra added the Area\Tests Issues that are targeted to tests or test projects label Apr 8, 2020
Copy link
Member

@cheenamalhotra cheenamalhotra left a comment

Choose a reason for hiding this comment

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

Just some minor cleanups.

@cheenamalhotra cheenamalhotra added this to the 2.0.0-preview3 milestone Apr 9, 2020
@Wraith2
Copy link
Contributor Author

Wraith2 commented Apr 14, 2020

/me pokes @cheenamalhotra

@cheenamalhotra
Copy link
Member

The PR is currently in review with team @Wraith2. We are hoping to get this merged soon! 😊

@cheenamalhotra cheenamalhotra merged commit 6e09b54 into dotnet:master Apr 23, 2020
@Wraith2 Wraith2 deleted the dev-sslovertds1 branch April 23, 2020 23:53
@Wraith2 Wraith2 mentioned this pull request Jul 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area\Tests Issues that are targeted to tests or test projects
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants