Tests | Address additional flaky tests (#4305)

* Update LocalAppContextSwitchesTest to use RAII helper

This prevents checks of the default AppContext switch values which would run in parallel to other tests which modify these values via the RAII helper.

* Address duplicate AKV key names

* Swap System.Random with the same RNG which SqlClient uses.
* Make a fixed number of attempts to reattempt key creation if a key with the generated name already exists.

* Remove hardcoded stored procedure name

* Accept either a SqlException or an InvalidOperationException to indicate operation cancellation

* Update assertion for UseLegacyFailoverAlternationOnLoginSqlErrors

* Correct test failures

LocalAppContextSwitchesHelper was returning the field values, skipping the default values exposed by LocalAppContextSwitches when the AppContext switch had not been set one way or another.

* Modify AppContext switch helper to include UseOverallConnectTimeoutForPoolWait

---------

Co-authored-by: Malcolm Daigle <mdaigle@microsoft.com>
Co-authored-by: Malcolm Daigle <daiglemalcolm@gmail.com>

Author: edwardneal

src/Microsoft.Data.SqlClient/tests/Common/Fixtures/AzureKeyVaultKeyFixtureBase.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Security.Cryptography;
 using Azure.Core;
 using Azure.Security.KeyVault.Keys;
 
@@ -18,20 +19,41 @@ namespace Microsoft.Data.SqlClient.Tests.Common.Fixtures;
 public abstract class AzureKeyVaultKeyFixtureBase : IDisposable
 {
     private readonly KeyClient _keyClient;
-    private readonly Random _randomGenerator;
+    private readonly RandomNumberGenerator _randomGenerator;
 
     private readonly List<KeyVaultKey> _createdKeys = new List<KeyVaultKey>();
 
     protected AzureKeyVaultKeyFixtureBase(Uri keyVaultUri, TokenCredential keyVaultToken)
     {
         _keyClient = new KeyClient(keyVaultUri, keyVaultToken);
-        _randomGenerator = new Random();
+        _randomGenerator = RandomNumberGenerator.Create();
     }
 
     protected Uri CreateKey(string name, int keySize)
     {
-        CreateRsaKeyOptions createOptions = new CreateRsaKeyOptions(GenerateUniqueName(name)) { KeySize = keySize };
-        KeyVaultKey created = _keyClient.CreateRsaKey(createOptions);
+        const int MaxConflictResolutions = 5;
+        KeyVaultKey created;
+        int i = 0;
+
+        while (true)
+        {
+            CreateRsaKeyOptions createOptions = new CreateRsaKeyOptions(GenerateUniqueName(name)) { KeySize = keySize };
+
+            try
+            {
+                created = _keyClient.CreateRsaKey(createOptions);
+                break;
+            }
+            // It's possible for a key to already exist with the same name, even in a deleted state. If so, CreateRsaKey
+            // will throw an exception with HTTP status code 409 (Conflict.)
+            // We can't assume we possess permissions to purge or to recover the key, so regenerate the name and try again.
+            // Only make MaxConflictResolutions attempts, to avoid possible infinite loops.
+            catch (Azure.RequestFailedException conflictException)
+                when (conflictException.Status == 409 && i < MaxConflictResolutions)
+            {
+                i++;
+            }
+        }
 
         _createdKeys.Add(created);
         return created.Id;
@@ -41,7 +63,7 @@ private string GenerateUniqueName(string name)
     {
         byte[] rndBytes = new byte[16];
 
-        _randomGenerator.NextBytes(rndBytes);
+        _randomGenerator.GetBytes(rndBytes);
         return name + "-" + BitConverter.ToString(rndBytes);
     }
 
@@ -64,5 +86,7 @@ protected virtual void Dispose(bool disposing)
                 continue;
             }
         }
+
+        _randomGenerator.Dispose();
     }
 }

src/Microsoft.Data.SqlClient/tests/Common/LocalAppContextSwitchesHelper.cs

@@ -214,7 +214,8 @@ public void Dispose()
 
     #region Switch Value Getters and Setters
 
-    // These properties get or set the like-named underlying switch field value.
+    // These properties get the like-named underlying switch *property* value and set the underlying
+    // switch *field* value. This allows tests to verify the default switch values.
     //
     // They all throw if the value cannot be retrieved or set.
 
@@ -224,7 +225,7 @@ public void Dispose()
     /// </summary>
     public bool? DisableTnirByDefault
     {
-        get => GetSwitchValue("s_disableTnirByDefault");
+        get => GetSwitchPropertyValue(nameof(DisableTnirByDefault));
         set => SetSwitchValue("s_disableTnirByDefault", value);
     }
     #endif
@@ -234,7 +235,7 @@ public bool? DisableTnirByDefault
     /// </summary>
     public bool? EnableMultiSubnetFailoverByDefault
     {
-        get => GetSwitchValue("s_enableMultiSubnetFailoverByDefault");
+        get => GetSwitchPropertyValue(nameof(EnableMultiSubnetFailoverByDefault));
         set => SetSwitchValue("s_enableMultiSubnetFailoverByDefault", value);
     }
 
@@ -244,7 +245,7 @@ public bool? EnableMultiSubnetFailoverByDefault
     /// </summary>
     public bool? GlobalizationInvariantMode
     {
-        get => GetSwitchValue("s_globalizationInvariantMode");
+        get => GetSwitchPropertyValue(nameof(GlobalizationInvariantMode));
         set => SetSwitchValue("s_globalizationInvariantMode", value);
     }
     #endif
@@ -254,7 +255,7 @@ public bool? GlobalizationInvariantMode
     /// </summary>
     public bool? IgnoreServerProvidedFailoverPartner
     {
-        get => GetSwitchValue("s_ignoreServerProvidedFailoverPartner");
+        get => GetSwitchPropertyValue(nameof(IgnoreServerProvidedFailoverPartner));
         set => SetSwitchValue("s_ignoreServerProvidedFailoverPartner", value);
     }
 
@@ -263,7 +264,7 @@ public bool? IgnoreServerProvidedFailoverPartner
     /// </summary>
     public bool? UseLegacyFailoverAlternationOnLoginSqlErrors
     {
-        get => GetSwitchValue("s_useLegacyFailoverAlternationOnLoginSqlErrors");
+        get => GetSwitchPropertyValue(nameof(UseLegacyFailoverAlternationOnLoginSqlErrors));
         set => SetSwitchValue("s_useLegacyFailoverAlternationOnLoginSqlErrors", value);
     }
 
@@ -272,7 +273,7 @@ public bool? UseLegacyFailoverAlternationOnLoginSqlErrors
     /// </summary>
     public bool? LegacyRowVersionNullBehavior
     {
-        get => GetSwitchValue("s_legacyRowVersionNullBehavior");
+        get => GetSwitchPropertyValue(nameof(LegacyRowVersionNullBehavior));
         set => SetSwitchValue("s_legacyRowVersionNullBehavior", value);
     }
 
@@ -281,7 +282,7 @@ public bool? LegacyRowVersionNullBehavior
     /// </summary>
     public bool? LegacyVarTimeZeroScaleBehaviour
     {
-        get => GetSwitchValue("s_legacyVarTimeZeroScaleBehaviour");
+        get => GetSwitchPropertyValue(nameof(LegacyVarTimeZeroScaleBehaviour));
         set => SetSwitchValue("s_legacyVarTimeZeroScaleBehaviour", value);
     }
 
@@ -290,7 +291,7 @@ public bool? LegacyVarTimeZeroScaleBehaviour
     /// </summary>
     public bool? MakeReadAsyncBlocking
     {
-        get => GetSwitchValue("s_makeReadAsyncBlocking");
+        get => GetSwitchPropertyValue(nameof(MakeReadAsyncBlocking));
         set => SetSwitchValue("s_makeReadAsyncBlocking", value);
     }
 
@@ -299,7 +300,7 @@ public bool? MakeReadAsyncBlocking
     /// </summary>
     public bool? SuppressInsecureTlsWarning
     {
-        get => GetSwitchValue("s_suppressInsecureTlsWarning");
+        get => GetSwitchPropertyValue(nameof(SuppressInsecureTlsWarning));
         set => SetSwitchValue("s_suppressInsecureTlsWarning", value);
     }
 
@@ -308,7 +309,7 @@ public bool? SuppressInsecureTlsWarning
     /// </summary>
     public bool? TruncateScaledDecimal
     {
-        get => GetSwitchValue("s_truncateScaledDecimal");
+        get => GetSwitchPropertyValue(nameof(TruncateScaledDecimal));
         set => SetSwitchValue("s_truncateScaledDecimal", value);
     }
 
@@ -317,7 +318,7 @@ public bool? TruncateScaledDecimal
     /// </summary>
     public bool? UseCompatibilityAsyncBehaviour
     {
-        get => GetSwitchValue("s_useCompatibilityAsyncBehaviour");
+        get => GetSwitchPropertyValue(nameof(UseCompatibilityAsyncBehaviour));
         set => SetSwitchValue("s_useCompatibilityAsyncBehaviour", value);
     }
 
@@ -326,7 +327,7 @@ public bool? UseCompatibilityAsyncBehaviour
     /// </summary>
     public bool? UseCompatibilityProcessSni
     {
-        get => GetSwitchValue("s_useCompatibilityProcessSni");
+        get => GetSwitchPropertyValue(nameof(UseCompatibilityProcessSni));
         set => SetSwitchValue("s_useCompatibilityProcessSni", value);
     }
 
@@ -335,7 +336,7 @@ public bool? UseCompatibilityProcessSni
     /// </summary>
     public bool? UseConnectionPoolV2
     {
-        get => GetSwitchValue("s_useConnectionPoolV2");
+        get => GetSwitchPropertyValue(nameof(UseConnectionPoolV2));
         set => SetSwitchValue("s_useConnectionPoolV2", value);
     }
 
@@ -344,7 +345,7 @@ public bool? UseConnectionPoolV2
     /// </summary>
     public bool? UseLegacyIdleTimeoutBehavior
     {
-        get => GetSwitchValue("s_useLegacyIdleTimeoutBehavior");
+        get => GetSwitchPropertyValue(nameof(UseLegacyIdleTimeoutBehavior));
         set => SetSwitchValue("s_useLegacyIdleTimeoutBehavior", value);
     }
 
@@ -353,7 +354,7 @@ public bool? UseLegacyIdleTimeoutBehavior
     /// </summary>
     public bool? UseOverallConnectTimeoutForPoolWait
     {
-        get => GetSwitchValue("s_useOverallConnectTimeoutForPoolWait");
+        get => GetSwitchPropertyValue(nameof(UseOverallConnectTimeoutForPoolWait));
         set => SetSwitchValue("s_useOverallConnectTimeoutForPoolWait", value);
     }
 
@@ -363,7 +364,7 @@ public bool? UseOverallConnectTimeoutForPoolWait
     /// </summary>
     public bool? UseManagedNetworking
     {
-        get => GetSwitchValue("s_useManagedNetworking");
+        get => GetSwitchPropertyValue(nameof(UseManagedNetworking));
         set => SetSwitchValue("s_useManagedNetworking", value);
     }
     #endif
@@ -373,7 +374,7 @@ public bool? UseManagedNetworking
     /// </summary>
     public bool? UseMinimumLoginTimeout
     {
-        get => GetSwitchValue("s_useMinimumLoginTimeout");
+        get => GetSwitchPropertyValue(nameof(UseMinimumLoginTimeout));
         set => SetSwitchValue("s_useMinimumLoginTimeout", value);
     }
 
@@ -386,19 +387,7 @@ public bool? UseMinimumLoginTimeout
     /// </summary>
     private static bool? GetSwitchValue(string fieldName)
     {
-        var assembly = Assembly.GetAssembly(typeof(SqlConnection));
-        if (assembly is null)
-        {
-            throw new InvalidOperationException(
-                "Could not get assembly for Microsoft.Data.SqlClient");
-        }
-        
-        var type = assembly.GetType("Microsoft.Data.SqlClient.LocalAppContextSwitches");
-        if (type is null)
-        {
-            throw new InvalidOperationException(
-                "Could not get type LocalAppContextSwitches");
-        }
+        var type = GetLocalAppContextSwitchesType();
 
         var field = type.GetField(
             fieldName,
@@ -433,19 +422,7 @@ public bool? UseMinimumLoginTimeout
     /// </summary>
     private static void SetSwitchValue(string fieldName, bool? value)
     {
-        var assembly = Assembly.GetAssembly(typeof(SqlConnection));
-        if (assembly is null)
-        {
-            throw new InvalidOperationException(
-                "Could not get assembly for Microsoft.Data.SqlClient");
-        }
-        
-        var type = assembly.GetType("Microsoft.Data.SqlClient.LocalAppContextSwitches");
-        if (type is null)
-        {
-            throw new InvalidOperationException(
-                "Could not get type LocalAppContextSwitches");
-        }
+        var type = GetLocalAppContextSwitchesType();
 
         var field = type.GetField(
             fieldName,
@@ -470,5 +447,49 @@ private static void SetSwitchValue(string fieldName, bool? value)
         field.SetValue(null, Enum.ToObject(field.FieldType, byteValue));
     }
 
+    /// <summary>
+    /// Use reflection to get a switch property value from LocalAppContextSwitches.
+    /// </summary>
+    /// <remarks>
+    /// Each property in LocalAppContextSwitchHelper corresponds to a like-named property in
+    /// LocalAppContextSwitches, which may return a different value when the AppContext switch
+    /// has not been set.
+    /// </remarks>
+    private static bool GetSwitchPropertyValue(string propertyName)
+    {
+        var type = GetLocalAppContextSwitchesType();
+        var property = type.GetProperty(
+            propertyName,
+            BindingFlags.Static | BindingFlags.Public);
+
+        if (property == null)
+        {
+            throw new InvalidOperationException(
+                $"Property '{propertyName}' not found in LocalAppContextSwitches");
+        }
+
+        object? value = property.GetValue(null);
+
+        return value is bool boolValue
+            ? boolValue
+            : throw new InvalidOperationException($"Property '{propertyName}' is not of type bool.");
+    }
+
+    private static Type GetLocalAppContextSwitchesType()
+    {
+        var assembly = Assembly.GetAssembly(typeof(SqlConnection));
+        if (assembly is null)
+        {
+            throw new InvalidOperationException("Could not get assembly for Microsoft.Data.SqlClient");
+        }
+
+        var type = assembly.GetType("Microsoft.Data.SqlClient.LocalAppContextSwitches");
+        if (type is null)
+        {
+            throw new InvalidOperationException("Could not get type LocalAppContextSwitches");
+        }
+        return type;
+    }
+
     #endregion
 }