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.

Sign up for GitHub

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

Jump to bottom

Port connection pool tests from devdiv #1155

Merged
merged 17 commits into from
Sep 20, 2021
Merged

Port connection pool tests from devdiv #1155

merged 17 commits into from
Sep 20, 2021

Conversation

johnnypham
Copy link
Contributor

No description provided.

@cheenamalhotra cheenamalhotra added the Area\Tests Issues that are targeted to tests or test projects label Jul 7, 2021
@cheenamalhotra cheenamalhotra added this to the 4.0.0-preview1 milestone Jul 27, 2021
cheenamalhotra
cheenamalhotra reviewed Jul 27, 2021
View reviewed changes
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 suggestions to use using to ensure resources are disposed and cleaned up by GC.

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs Outdated Show resolved Hide resolved
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs Outdated Show resolved Hide resolved
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs Outdated Show resolved Hide resolved
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs Outdated Show resolved Hide resolved
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs Outdated Show resolved Hide resolved
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs Outdated Show resolved Hide resolved
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs Outdated Show resolved Hide resolved
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs Outdated Show resolved Hide resolved
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs Outdated Show resolved Hide resolved
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/AsyncTest/BeginExecAsyncTest.cs Outdated Show resolved Hide resolved
DavoudEshtehari
DavoudEshtehari reviewed Aug 5, 2021
View reviewed changes
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.

NIT: I'd rather manage a large block of code in a separate file instead of using #if-#endif preprocessor directives.

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs Outdated Show resolved Hide resolved
@johnnypham
Copy link
Contributor Author

NIT: I'd rather manage a large block of code in a separate file instead of using #if-#endif preprocessor directives.

Created two debug and transaction pool files.

@cheenamalhotra
Copy link
Member

/azp run CI-Win10-SQL19W-TR CI-Win10-SQL19W

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@cheenamalhotra cheenamalhotra removed this from the 4.0.0-preview1 milestone Aug 19, 2021
@cheenamalhotra cheenamalhotra added this to the 4.0.0-preview2 milestone Aug 19, 2021
@cheenamalhotra cheenamalhotra mentioned this pull request Sep 1, 2021
Merged
@cheenamalhotra
Copy link
Member

cheenamalhotra commented Sep 1, 2021 •
edited
Loading

The failing tests are due to deadlock as issue #1124.
PR #1242 shall fix these failures as well.

@cheenamalhotra cheenamalhotra merged commit 96a8b05 into dotnet:main Sep 20, 2021
@johnnypham johnnypham deleted the devdivtests branch September 20, 2021 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JRahnama JRahnama JRahnama left review comments

@DavoudEshtehari DavoudEshtehari DavoudEshtehari left review comments

@cheenamalhotra cheenamalhotra cheenamalhotra approved these changes

Assignees
No one assigned
Labels
Area\Tests Issues that are targeted to tests or test projects
Projects
None yet
Milestone
4.0.0-preview2
Development

Successfully merging this pull request may close these issues.
4 participants
@johnnypham @cheenamalhotra @JRahnama @DavoudEshtehari