Tests | Fix TVP query hint test timeouts against shared Azure SQL DB (#4593)

Author: cheenamalhotra

src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/DatabaseUser.cs

@@ -35,7 +35,13 @@ protected override void CreateObject(string definition)
 
     protected override void DropObject()
     {
-        using SqlCommand dropCommand = new($"IF USER_ID('{UnescapedName}') IS NOT NULL DROP USER {Name}", Connection);
+        // NOTE: The name is passed to USER_ID() as a parameter rather than being interpolated into
+        //   a string literal, because it embeds Environment.UserName/MachineName (see
+        //   DatabaseObject.GenerateLongName) and an apostrophe in either would break the batch.
+        //   The identifier in DROP USER is already bracket-quoted by GenerateLongName.
+        using SqlCommand dropCommand = new($"IF USER_ID(@name) IS NOT NULL DROP USER {Name}", Connection);
+
+        dropCommand.Parameters.AddWithValue("@name", UnescapedName);
 
         ExecuteCommandInDatabase(dropCommand);
     }

src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/ServerLogin.cs

@@ -92,7 +92,13 @@ protected override void CreateObject(string definition)
 
     protected override void DropObject()
     {
-        using SqlCommand dropCommand = new($"IF SUSER_ID('{UnescapedName}') IS NOT NULL DROP LOGIN {Name}", Connection);
+        // NOTE: The name is passed to SUSER_ID() as a parameter rather than being interpolated into
+        //   a string literal, because it embeds Environment.UserName/MachineName (see
+        //   DatabaseObject.GenerateLongName) and an apostrophe in either would break the batch.
+        //   The identifier in DROP LOGIN is already bracket-quoted by GenerateLongName.
+        using SqlCommand dropCommand = new($"IF SUSER_ID(@name) IS NOT NULL DROP LOGIN {Name}", Connection);
+
+        dropCommand.Parameters.AddWithValue("@name", UnescapedName);
 
         dropCommand.ExecuteNonQuery();
     }

src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/StoredProcedure.cs

@@ -34,7 +34,13 @@ protected override void CreateObject(string definition)
 
     protected override void DropObject()
     {
-        using SqlCommand dropCommand = new($"IF (OBJECT_ID('{Name}') IS NOT NULL) DROP PROCEDURE {Name}", Connection);
+        // NOTE: The name is passed to OBJECT_ID() as a parameter rather than being interpolated
+        //   into a string literal, because it embeds Environment.UserName/MachineName (see
+        //   DatabaseObject.GenerateLongName) and an apostrophe in either would break the batch.
+        //   The identifier in DROP PROCEDURE is already bracket-quoted by GenerateLongName.
+        using SqlCommand dropCommand = new($"IF (OBJECT_ID(@name) IS NOT NULL) DROP PROCEDURE {Name}", Connection);
+
+        dropCommand.Parameters.AddWithValue("@name", Name);
 
         dropCommand.ExecuteNonQuery();
     }

src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/Table.cs

@@ -34,7 +34,13 @@ protected override void CreateObject(string definition)
 
     protected override void DropObject()
     {
-        using SqlCommand dropCommand = new($"IF (OBJECT_ID('{Name}') IS NOT NULL) DROP TABLE {Name}", Connection);
+        // NOTE: The name is passed to OBJECT_ID() as a parameter rather than being interpolated
+        //   into a string literal, because it embeds Environment.UserName/MachineName (see
+        //   DatabaseObject.GenerateLongName) and an apostrophe in either would break the batch.
+        //   The identifier in DROP TABLE is already bracket-quoted by GenerateLongName.
+        using SqlCommand dropCommand = new($"IF (OBJECT_ID(@name) IS NOT NULL) DROP TABLE {Name}", Connection);
+
+        dropCommand.Parameters.AddWithValue("@name", Name);
 
         dropCommand.ExecuteNonQuery();
     }

src/Microsoft.Data.SqlClient/tests/Common/Fixtures/DatabaseObjects/UserDefinedType.cs

@@ -34,7 +34,16 @@ protected override void CreateObject(string definition)
 
     protected override void DropObject()
     {
-        using SqlCommand dropCommand = new($"IF (OBJECT_ID('{Name}') IS NOT NULL) DROP TYPE {Name}", Connection);
+        // NOTE: User-defined types live in sys.types, not sys.objects, so OBJECT_ID() always
+        //   returns NULL for them. Using it here silently skipped every drop and leaked the type
+        //   into the (shared) test database. TYPE_ID() is the correct lookup.
+        // NOTE: The name is passed to TYPE_ID() as a parameter rather than being interpolated into
+        //   a string literal, because it embeds Environment.UserName/MachineName (see
+        //   DatabaseObject.GenerateLongName) and an apostrophe in either would break the batch.
+        //   The identifier in DROP TYPE is already bracket-quoted by GenerateLongName.
+        using SqlCommand dropCommand = new($"IF (TYPE_ID(@typeName) IS NOT NULL) DROP TYPE {Name}", Connection);
+
+        dropCommand.Parameters.AddWithValue("@typeName", Name);
 
         dropCommand.ExecuteNonQuery();
     }

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpQueryHintsTests.cs

@@ -6,58 +6,121 @@
 using System.Collections.Generic;
 using System.Data;
 using Microsoft.Data.SqlClient.Server;
+using Microsoft.Data.SqlClient.Tests.Common.Fixtures.DatabaseObjects;
 using Xunit;
 
 namespace Microsoft.Data.SqlClient.ManualTesting.Tests
 {
     /// <summary>
-    /// Tests for TVP query hints (sort order, uniqueness, default columns).
+    /// Creates the table type and stored procedure used by <see cref="TvpQueryHintsTests"/> exactly
+    /// once for the whole test class.
     /// </summary>
-    [Trait("Set", "3")]
-    public sealed class TvpQueryHintsTests : IDisposable
+    /// <remarks>
+    /// Every test in the class needs an identically shaped table type and procedure, so creating them
+    /// per test only multiplies the amount of DDL issued against the (shared) test database. On Azure
+    /// SQL Database the resulting schema-modification lock contention was enough to push
+    /// CREATE/DROP TYPE and CREATE/DROP PROCEDURE past the default 30 second command timeout, which
+    /// showed up as sporadic "Execution Timeout Expired" failures in the manual test legs. Sharing a
+    /// single type/procedure via a class fixture cuts the DDL statement count by 5x, and the extended
+    /// command timeout below absorbs the contention that remains.
+    /// </remarks>
+    public sealed class TvpQueryHintsFixture : IDisposable
     {
-        private readonly SqlConnection _conn;
-        private readonly SqlCommand _cmd;
-        private readonly SqlParameter _param;
-        private readonly string _procName;
-        private readonly string _typeName;
+        /// <summary>
+        /// Command timeout (in seconds) applied to every command issued on <see cref="Connection"/>.
+        /// Deliberately generous: DDL against a shared Azure SQL Database can block for a long time
+        /// behind concurrently executing test legs.
+        /// </summary>
+        private const int CommandTimeoutSeconds = 120;
+
+        private readonly UserDefinedType _tableType;
+        private readonly StoredProcedure _procedure;
 
-        public TvpQueryHintsTests()
+        public SqlConnection Connection { get; }
+
+        public string ProcedureName => _procedure.Name;
+
+        public TvpQueryHintsFixture()
         {
-            Guid randomizer = Guid.NewGuid();
-            _typeName = string.Format("dbo.[QHint_{0}]", randomizer);
-            _procName = string.Format("dbo.[QHint_Proc_{0}]", randomizer);
-            string createTypeSql = string.Format(
-                    "CREATE TYPE {0} AS TABLE("
+            SqlConnectionStringBuilder builder = new(DataTestUtility.TCPConnectionString)
+            {
+                CommandTimeout = CommandTimeoutSeconds
+            };
+
+            Connection = new SqlConnection(builder.ConnectionString);
+
+            // Partial construction must not leak the objects created so far, otherwise a transient
+            // failure here would orphan a type in the shared database.
+            try
+            {
+                Connection.Open();
+
+                _tableType = new UserDefinedType(Connection, "QHint",
+                    "TABLE("
                         + " c1 Int DEFAULT -1,"
                         + " c2 NVarChar(40) DEFAULT N'DEFUALT',"
                         + " c3 DateTime DEFAULT '1/1/2006',"
-                        + " c4 Int DEFAULT -1)",
-                        _typeName);
-            string createProcSql = string.Format(
-                    "CREATE PROC {0}(@tvp {1} READONLY) AS SELECT TOP(2) * FROM @tvp ORDER BY c1", _procName, _typeName);
-
-            _conn = new SqlConnection(DataTestUtility.TCPConnectionString);
-            _conn.Open();
+                        + " c4 Int DEFAULT -1)");
+
+                try
+                {
+                    _procedure = new StoredProcedure(Connection, "QHint_Proc",
+                        $"(@tvp {_tableType.Name} READONLY) AS SELECT TOP(2) * FROM @tvp ORDER BY c1");
+                }
+                catch
+                {
+                    _tableType.Dispose();
+                    throw;
+                }
+            }
+            catch
+            {
+                Connection.Dispose();
+                throw;
+            }
+        }
 
-            _cmd = new SqlCommand(createTypeSql, _conn);
-            _cmd.ExecuteNonQuery();
+        public void Dispose()
+        {
+            // Each step runs even if an earlier one fails, so a transient error while dropping the
+            // procedure cannot leave the type (or the connection) behind.
+            try
+            {
+                _procedure.Dispose();
+            }
+            finally
+            {
+                try
+                {
+                    _tableType.Dispose();
+                }
+                finally
+                {
+                    Connection.Dispose();
+                }
+            }
+        }
+    }
 
-            _cmd.CommandText = createProcSql;
-            _cmd.ExecuteNonQuery();
+    /// <summary>
+    /// Tests for TVP query hints (sort order, uniqueness, default columns).
+    /// </summary>
+    [Trait("Set", "3")]
+    public sealed class TvpQueryHintsTests : IClassFixture<TvpQueryHintsFixture>, IDisposable
+    {
+        private readonly SqlCommand _cmd;
+        private readonly SqlParameter _param;
 
-            _cmd.CommandText = _procName;
-            _cmd.CommandType = CommandType.StoredProcedure;
+        public TvpQueryHintsTests(TvpQueryHintsFixture fixture)
+        {
+            _cmd = new SqlCommand(fixture.ProcedureName, fixture.Connection)
+            {
+                CommandType = CommandType.StoredProcedure
+            };
             _param = _cmd.Parameters.Add("@tvp", SqlDbType.Structured);
         }
 
-        public void Dispose()
-        {
-            string dropSql = string.Format("DROP PROC {0}; DROP TYPE {1}", _procName, _typeName);
-            using SqlCommand cmd = new(dropSql, _conn);
-            cmd.ExecuteNonQuery();
-            _conn.Dispose();
-        }
+        public void Dispose() => _cmd.Dispose();
 
         [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse))]
         public void SortOrderSimple()