Add additional tests. Refactor to test error behavior.

Author: mdaigle

src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPoolTest.cs

@@ -7,6 +7,7 @@
 using System.Threading;
 using System.Threading.Tasks;
 using System.Transactions;
+using Microsoft.Data.Common;
 using Microsoft.Data.Common.ConnectionString;
 using Microsoft.Data.ProviderBase;
 using Microsoft.Data.SqlClient.ConnectionPool;
@@ -16,17 +17,19 @@ namespace Microsoft.Data.SqlClient.UnitTests
 {
     public class ChannelDbConnectionPoolTest
     {
-        private readonly ChannelDbConnectionPool pool;
-        private readonly DbConnectionPoolGroup dbConnectionPoolGroup;
-        private readonly DbConnectionPoolGroupOptions poolGroupOptions;
-        private readonly DbConnectionFactory connectionFactory;
-        private readonly DbConnectionPoolIdentity identity;
-        private readonly DbConnectionPoolProviderInfo connectionPoolProviderInfo;
-
-        public ChannelDbConnectionPoolTest()
+        private ChannelDbConnectionPool pool;
+        private DbConnectionFactory connectionFactory;
+        private DbConnectionPoolGroup dbConnectionPoolGroup;
+        private DbConnectionPoolGroupOptions poolGroupOptions;
+        private DbConnectionPoolIdentity identity;
+        private DbConnectionPoolProviderInfo connectionPoolProviderInfo;
+
+        private static readonly DbConnectionFactory SuccessfulConnectionFactory = new SuccessfulDbConnectionFactory();
+        private static readonly DbConnectionFactory TimeoutConnectionFactory = new TimeoutDbConnectionFactory();
+
+        private void Setup(DbConnectionFactory connectionFactory)
         {
-            // Use a stubbed connection factory to avoid network code
-            connectionFactory = new SuccessfulConnectionFactory();
+            this.connectionFactory = connectionFactory;
             identity = DbConnectionPoolIdentity.NoIdentity;
             connectionPoolProviderInfo = new DbConnectionPoolProviderInfo();
             poolGroupOptions = new DbConnectionPoolGroupOptions(
@@ -54,8 +57,11 @@ public ChannelDbConnectionPoolTest()
         [InlineData(1)]
         [InlineData(5)]
         [InlineData(10)]
-        public void TestGetConnectionFromEmptyPoolSync_ShouldCreateNewConnection(int numConnections)
+        public void GetConnectionFromEmptyPoolSync_ShouldCreateNewConnection(int numConnections)
         {
+            // Arrange
+            Setup(SuccessfulConnectionFactory);
+
             // Act
             for (int i = 0; i < numConnections; i++)
             {
@@ -81,8 +87,11 @@ out internalConnection
         [InlineData(1)]
         [InlineData(5)]
         [InlineData(10)]
-        public async Task TestGetConnectionFromEmptyPoolAsync_ShouldCreateNewConnection(int numConnections)
+        public async Task GetConnectionFromEmptyPoolAsync_ShouldCreateNewConnection(int numConnections)
         {
+            // Arrange
+            Setup(SuccessfulConnectionFactory);
+
             // Act
             for (int i = 0; i < numConnections; i++)
             {
@@ -106,8 +115,106 @@ out internalConnection
             Assert.Equal(numConnections, pool.Count);
         }
 
-        // Test that requests to get connection from the pool fails when max pool size is reached
-       
+        [Fact]
+        public void GetConnectionRespectsMaxPoolSize()
+        {
+            // Arrange
+            Setup(SuccessfulConnectionFactory);
+
+            for (int i = 0; i < poolGroupOptions.MaxPoolSize; i++)
+            {
+                DbConnectionInternal internalConnection = null;
+                var completed = pool.TryGetConnection(
+                    new SqlConnection(),
+                    taskCompletionSource: null,
+                    new DbConnectionOptions("", null),
+                    out internalConnection
+                );
+
+                Assert.True(completed);
+                Assert.NotNull(internalConnection);
+            }
+
+            try
+            {
+                // Act
+                DbConnectionInternal extraConnection = null;
+                var exceeded = pool.TryGetConnection(
+                    new SqlConnection("Timeout=1"),
+                    taskCompletionSource: null,
+                    new DbConnectionOptions("", null),
+                    out extraConnection
+                );
+            }
+            catch (Exception ex)
+            {
+                // Assert
+                Assert.IsType<InvalidOperationException>(ex);
+                Assert.Equal("Timeout expired.  The timeout period elapsed prior to obtaining a connection from the pool.  This may have occurred because all pooled connections were in use and max pool size was reached.", ex.Message);
+            }
+
+            // Assert
+            Assert.Equal(poolGroupOptions.MaxPoolSize, pool.Count);
+        }
+
+        [Fact]
+        [ActiveIssue("Requires ReturnInternalConnection to be implemented in ChannelDbConnectionPool")]
+        public void ConnectionsAreReused()
+        {
+            // Arrange
+            Setup(SuccessfulConnectionFactory);
+            DbConnectionInternal internalConnection1 = null;
+            DbConnectionInternal internalConnection2 = null;
+
+            // Act: Get the first connection
+            var completed1 = pool.TryGetConnection(
+                new SqlConnection(),
+                null,
+                new DbConnectionOptions("", null),
+                out internalConnection1
+            );
+
+            // Assert: First connection should succeed
+            Assert.True(completed1);
+            Assert.NotNull(internalConnection1);
+
+            // Act: Return the first connection to the pool
+            pool.ReturnInternalConnection(internalConnection1, null);
+
+            // Act: Get the second connection (should reuse the first one)
+            var completed2 = pool.TryGetConnection(
+                new SqlConnection(),
+                null,
+                new DbConnectionOptions("", null),
+                out internalConnection2
+            );
+
+            // Assert: Second connection should succeed and reuse the first connection
+            Assert.True(completed2);
+            Assert.NotNull(internalConnection2);
+            Assert.Same(internalConnection1, internalConnection2);
+        }
+
+        [Fact]
+        public void GetConnectionWithTimeout_ShouldThrowTimeoutException()
+        {
+            // Arrange
+            Setup(TimeoutConnectionFactory);
+            DbConnectionInternal internalConnection = null;
+
+            // Act & Assert
+            var ex = Assert.Throws<InvalidOperationException>(() =>
+            {
+                var completed = pool.TryGetConnection(
+                    new SqlConnection(),
+                    taskCompletionSource: null,
+                    new DbConnectionOptions("", null),
+                    out internalConnection
+                );
+            });
+
+            Assert.Equal("Timeout expired.  The timeout period elapsed prior to obtaining a connection from the pool.  This may have occurred because all pooled connections were in use and max pool size was reached.", ex.Message);
+        }
 
         #region Property Tests
 
@@ -243,49 +350,10 @@ public void TestTransactionEnded()
         {
             Assert.Throws<NotImplementedException>(() => pool.TransactionEnded(null!, null!));
         }
-        [Fact]
-        public void TestGetConnectionFailsWhenMaxPoolSizeReached()
-        {
-            // Arrange
-            for (int i = 0; i < poolGroupOptions.MaxPoolSize; i++)
-            {
-                DbConnectionInternal internalConnection = null;
-                var completed = pool.TryGetConnection(
-                    new SqlConnection(),
-                    taskCompletionSource: null,
-                    new DbConnectionOptions("", null),
-                    out internalConnection
-                );
-
-                Assert.True(completed);
-                Assert.NotNull(internalConnection);
-            }
-
-            try
-            {
-                // Act
-                DbConnectionInternal extraConnection = null;
-                var exceeded = pool.TryGetConnection(
-                    //TODO: set timeout to make this faster
-                    new SqlConnection(),
-                    taskCompletionSource: null,
-                    new DbConnectionOptions("", null),
-                    out extraConnection
-                );
-            } catch (Exception ex)
-            {
-                // Assert
-                Assert.IsType<InvalidOperationException>(ex);
-                Assert.Equal("Timeout expired.  The timeout period elapsed prior to obtaining a connection from the pool.  This may have occurred because all pooled connections were in use and max pool size was reached.", ex.Message);
-            }
-
-            // Assert
-            Assert.Equal(poolGroupOptions.MaxPoolSize, pool.Count);
-        }
         #endregion
 
         #region Test classes
-        internal class SuccessfulConnectionFactory : DbConnectionFactory
+        internal class SuccessfulDbConnectionFactory : DbConnectionFactory
         {
             protected override DbConnectionInternal CreateConnection(DbConnectionOptions options, DbConnectionPoolKey poolKey, object poolGroupProviderInfo, IDbConnectionPool pool, DbConnection owningConnection)
             {
@@ -347,89 +415,89 @@ internal override void SetInnerConnectionTo(DbConnection owningObject, DbConnect
             #endregion
         }
 
-        internal class SuccessfulDbConnectionInternal : DbConnectionInternal
+        internal class TimeoutDbConnectionFactory : DbConnectionFactory
         {
-            #region Not Implemented Members
-            public override string ServerVersion => throw new NotImplementedException();
-
-            public override DbTransaction BeginTransaction(System.Data.IsolationLevel il)
+            protected override DbConnectionInternal CreateConnection(DbConnectionOptions options, DbConnectionPoolKey poolKey, object poolGroupProviderInfo, IDbConnectionPool pool, DbConnection owningConnection)
             {
-                throw new NotImplementedException();
+                throw ADP.PooledOpenTimeout();
             }
 
-            public override void EnlistTransaction(Transaction transaction)
+            #region Not Implemented Members
+            public override DbProviderFactory ProviderFactory => throw new NotImplementedException();
+
+            protected override DbConnectionOptions CreateConnectionOptions(string connectionString, DbConnectionOptions previous)
             {
                 throw new NotImplementedException();
             }
 
-            protected override void Activate(Transaction transaction)
+            protected override DbConnectionPoolGroupOptions CreateConnectionPoolGroupOptions(DbConnectionOptions options)
             {
                 throw new NotImplementedException();
             }
 
-            protected override void Deactivate()
+            protected override int GetObjectId(DbConnection connection)
             {
                 throw new NotImplementedException();
             }
-            #endregion
-        }
 
-        internal class TimeoutConnectionFactory : DbConnectionFactory
-        {
-            protected override DbConnectionInternal CreateConnection(DbConnectionOptions options, DbConnectionPoolKey poolKey, object poolGroupProviderInfo, IDbConnectionPool pool, DbConnection owningConnection)
+            internal override DbConnectionPoolGroup GetConnectionPoolGroup(DbConnection connection)
             {
-                return new SuccessfulDbConnectionInternal();
+                throw new NotImplementedException();
             }
 
-            #region Not Implemented Members
-            public override DbProviderFactory ProviderFactory => throw new NotImplementedException();
-
-            protected override DbConnectionOptions CreateConnectionOptions(string connectionString, DbConnectionOptions previous)
+            internal override DbConnectionInternal GetInnerConnection(DbConnection connection)
             {
                 throw new NotImplementedException();
             }
 
-            protected override DbConnectionPoolGroupOptions CreateConnectionPoolGroupOptions(DbConnectionOptions options)
+            internal override void PermissionDemand(DbConnection outerConnection)
             {
                 throw new NotImplementedException();
             }
 
-            protected override int GetObjectId(DbConnection connection)
+            internal override void SetConnectionPoolGroup(DbConnection outerConnection, DbConnectionPoolGroup poolGroup)
             {
                 throw new NotImplementedException();
             }
 
-            internal override DbConnectionPoolGroup GetConnectionPoolGroup(DbConnection connection)
+            internal override void SetInnerConnectionEvent(DbConnection owningObject, DbConnectionInternal to)
             {
                 throw new NotImplementedException();
             }
 
-            internal override DbConnectionInternal GetInnerConnection(DbConnection connection)
+            internal override bool SetInnerConnectionFrom(DbConnection owningObject, DbConnectionInternal to, DbConnectionInternal from)
             {
                 throw new NotImplementedException();
             }
 
-            internal override void PermissionDemand(DbConnection outerConnection)
+            internal override void SetInnerConnectionTo(DbConnection owningObject, DbConnectionInternal to)
             {
                 throw new NotImplementedException();
             }
+            #endregion
+        }
 
-            internal override void SetConnectionPoolGroup(DbConnection outerConnection, DbConnectionPoolGroup poolGroup)
+        internal class SuccessfulDbConnectionInternal : DbConnectionInternal
+        {
+            #region Not Implemented Members
+            public override string ServerVersion => throw new NotImplementedException();
+
+            public override DbTransaction BeginTransaction(System.Data.IsolationLevel il)
             {
                 throw new NotImplementedException();
             }
 
-            internal override void SetInnerConnectionEvent(DbConnection owningObject, DbConnectionInternal to)
+            public override void EnlistTransaction(Transaction transaction)
             {
                 throw new NotImplementedException();
             }
 
-            internal override bool SetInnerConnectionFrom(DbConnection owningObject, DbConnectionInternal to, DbConnectionInternal from)
+            protected override void Activate(Transaction transaction)
             {
                 throw new NotImplementedException();
             }
 
-            internal override void SetInnerConnectionTo(DbConnection owningObject, DbConnectionInternal to)
+            protected override void Deactivate()
             {
                 throw new NotImplementedException();
             }