FIX: Fixing test that does not work properly / Failing in connection pool#564
Open
subrata-ms wants to merge 2 commits into
Open
FIX: Fixing test that does not work properly / Failing in connection pool#564subrata-ms wants to merge 2 commits into
subrata-ms wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request updates the connection pooling test suite to reduce flakiness by isolating certain pooling-dependent tests into fresh subprocesses and by using more robust session identity checks when validating pool behavior.
Changes:
- Added a
_run_in_subprocesshelper to run pooling tests in isolated Python processes sopooling(...)configuration isn’t affected by earlier tests. - Refactored the idle-timeout pooling test to validate session replacement using
(SPID, login_time)instead of SPID alone. - Refactored the invalid-connection pooling test to simulate server-side session termination via
KILLand verify the pool does not return the killed session; improved password mutation logic in a recovery test using a case-insensitive regex.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+467
to
+476
| # KILL is processed asynchronously on the server. Poll until the | ||
| # victim's session has actually disappeared from sys.dm_exec_sessions | ||
| # before returning anything to the pool. | ||
| deadline = time.monotonic() + 10.0 | ||
| while time.monotonic() < deadline: | ||
| row = admin.cursor().execute( | ||
| "SELECT COUNT(*) FROM sys.dm_exec_sessions WHERE session_id = ?", | ||
| victim_spid, | ||
| ).fetchone() | ||
| if row[0] == 0: |
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changesNo lines with coverage information in this diff. 📋 Files Needing Attention📉 Files with overall lowest coverage (click to expand)mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.pybind.ddbc_bindings.h: 67.9%
mssql_python.row.py: 70.5%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 74.6%
mssql_python.pybind.connection.connection.cpp: 76.2%
mssql_python.__init__.py: 77.3%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 79.6%
mssql_python.connection.py: 85.3%🔗 Quick Links
|
sumitmsft
previously approved these changes
May 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Work Item / Issue Reference
Summary
This pull request significantly improves the reliability and accuracy of the connection pooling tests by running certain tests in isolated subprocesses and updating their logic to more robustly verify pooling behavior. The changes address issues with flaky or unreliable tests, ensure proper cleanup of pooled connections, and use more precise session identity checks.
Test infrastructure improvements:
_run_in_subprocessto execute test bodies in fresh Python subprocesses, ensuring that pooling configuration is isolated and not affected by previous tests. This is crucial because the pooling settings are locked in for the lifetime of the process due to the underlying C++ implementation.Connection pooling test robustness:
test_pool_idle_timeout_removes_connectionsto run in a subprocess and to use a unique session identity (SPID and login_time) instead of just SPID, making the test more reliable against SQL Server's SPID reuse.test_pool_removes_invalid_connectionsto run in a subprocess and simulate server-side session termination using KILL, then verify that the pool never returns a killed session. The test now uses only public APIs and is more realistic and robust.Password handling in tests:
test_pool_recovery_after_failed_connectionby using a case-insensitive regular expression to modify the password in the connection string, ensuring compatibility with various ODBC connection string formats.