Skip to content

FIX: Connection pooling flaky tests #159

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

Merged
merged 7 commits into from
Aug 6, 2025

Conversation

bewithgaurav
Copy link
Collaborator

@bewithgaurav bewithgaurav commented Jul 31, 2025

Work Item / Issue Reference

AB#38075

GitHub Issue: #<ISSUE_NUMBER>


Summary

This pull request enhances the connection pooling tests in tests/test_003_connection.py by introducing more detailed and robust test cases. The changes include adding new tests for connection pooling performance and connection reuse, as well as improving the structure and clarity of the existing test logic.

Enhancements to connection pooling tests:

  • Performance testing for pooled vs. non-pooled connections:

    • Added a new test (test_connection_pooling_speed) to measure and compare the average time taken for establishing connections with and without pooling. The test ensures that pooled connections are at least 20% faster than non-pooled connections.
  • Connection reuse validation:

    • Introduced a new test (test_connection_pooling_reuse_spid) to verify that connections are reused from the pool. The test checks that the SQL Server process ID (SPID) remains consistent across reused connections, indicating proper pooling behavior.
  • Improved test setup and cleanup:

    • Added explicit enabling and disabling of connection pooling at the start and end of tests to ensure isolation and prevent interference with other tests.

These changes improve the reliability and comprehensiveness of the connection pooling tests.

@Copilot Copilot AI review requested due to automatic review settings July 31, 2025 09:39
@github-actions github-actions bot added the pr-size: small Minimal code update label Jul 31, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses flaky connection pooling tests by improving test reliability and adding more comprehensive validation. The changes focus on making the connection pooling tests more robust and deterministic.

  • Enhanced connection speed comparison tests with statistical averaging and warm-up procedures
  • Added new test to verify actual connection reuse through SQL Server Process ID (SPID) validation
  • Improved test isolation with explicit pooling state management

@github-actions github-actions bot added pr-size: small Minimal code update and removed pr-size: small Minimal code update labels Jul 31, 2025
Copy link
Contributor

@sumitmsft sumitmsft left a comment

Choose a reason for hiding this comment

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

Added couple of comments. Please validate

@github-actions github-actions bot added pr-size: medium Moderate update size and removed pr-size: small Minimal code update labels Aug 5, 2025
@github-actions github-actions bot added pr-size: medium Moderate update size and removed pr-size: medium Moderate update size labels Aug 5, 2025
@github-actions github-actions bot added pr-size: medium Moderate update size and removed pr-size: medium Moderate update size labels Aug 5, 2025
@github-actions github-actions bot added pr-size: medium Moderate update size and removed pr-size: medium Moderate update size labels Aug 6, 2025
@github-actions github-actions bot added pr-size: medium Moderate update size and removed pr-size: medium Moderate update size labels Aug 6, 2025
@bewithgaurav bewithgaurav merged commit c03b348 into main Aug 6, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-size: medium Moderate update size
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants