From 9c776f3eee6322bb668d6a14952169db6c3992c5 Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Mon, 18 Dec 2023 12:10:45 -0800 Subject: [PATCH 01/20] Remove ActiveIssue to test pipeline. --- .../tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 087b44d964..25a818b786 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -86,7 +86,6 @@ public static void ConnectManagedWithInstanceNameTest(bool useMultiSubnetFailove // Note: This Unit test was tested in a domain-joined VM connecting to a remote // SQL Server using Kerberos in the same domain. - [ActiveIssue("27824")] // When specifying instance name and port number, this method call always returns false [ConditionalFact(nameof(IsKerberos))] public static void PortNumberInSPNTest() { From f84884c0bb347869a6b9fdf65e5b6a6ce091242e Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Wed, 20 Dec 2023 08:59:23 -0800 Subject: [PATCH 02/20] Fix SPN port number Unit Test to use TCP and NP connection strings and Named Instance with or without port number. --- .../Microsoft/Data/SqlClient/SNI/SNIProxy.cs | 6 +- .../ManualTests/DataCommon/DataTestUtility.cs | 10 +- .../SQL/InstanceNameTest/InstanceNameTest.cs | 110 ++++++++++++------ 3 files changed, 83 insertions(+), 43 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs index 3df369a2f6..d39f382bd6 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs @@ -653,10 +653,10 @@ private bool InferConnectionDetails() Port = port; } - // Instance Name Handling. Only if we found a '\' and we did not find a port in the Data Source - else if (backSlashIndex > -1) + // Instance Name Handling. + if (backSlashIndex > -1) { - // This means that there will not be any part separated by comma. + // This means that there is a part separated by '\' InstanceName = tokensByCommaAndSlash[1].Trim(); if (string.IsNullOrWhiteSpace(InstanceName)) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs index e167a264a2..5577ff0bec 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs @@ -979,8 +979,13 @@ public static bool ParseDataSource(string dataSource, out string hostname, out i port = -1; instanceName = string.Empty; - if (dataSource.Contains(",") && dataSource.Contains("\\")) + // Named pipe protocol data source does not support port number + if ((dataSource.ToUpper().Contains(@"\MSSQL$") && dataSource.ToUpper().Contains(@"\SQL\QUERY") && dataSource.Contains(",")) || + (dataSource.ToUpper().Contains(@"np:") && dataSource.Contains(","))) + { + port = -2; return false; + } if (dataSource.Contains(":")) { @@ -993,7 +998,8 @@ public static bool ParseDataSource(string dataSource, out string hostname, out i { return false; } - dataSource = dataSource.Substring(0, dataSource.IndexOf(",", StringComparison.Ordinal) - 1); + // IndexOf is zero-based, no need to subtract one + dataSource = dataSource.Substring(0, dataSource.IndexOf(",", StringComparison.Ordinal)); } if (dataSource.Contains("\\")) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 25a818b786..c83f342c18 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -84,47 +84,84 @@ public static void ConnectManagedWithInstanceNameTest(bool useMultiSubnetFailove } } - // Note: This Unit test was tested in a domain-joined VM connecting to a remote - // SQL Server using Kerberos in the same domain. - [ConditionalFact(nameof(IsKerberos))] - public static void PortNumberInSPNTest() + [ActiveIssue("27981")] // DataSource.InferNamedPipesInformation is not initializing InstanceName field + [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.IsNotAzureServer), nameof(DataTestUtility.IsNotAzureSynapse), nameof(DataTestUtility.AreConnStringsSetup))] + [InlineData("")] + [InlineData("44444")] + public static void PortNumberInSPNTestForNP(string port) + { + string connectionString = DataTestUtility.NPConnectionString; + SqlConnectionStringBuilder builder = new(connectionString); + + if (!string.IsNullOrWhiteSpace(port)) + builder.DataSource = $"{builder.DataSource},{port}"; + + PortNumberInSPNTest(builder.ConnectionString); + } + + [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.IsNotAzureServer), nameof(DataTestUtility.IsNotAzureSynapse), nameof(DataTestUtility.AreConnStringsSetup))] + [InlineData("")] + [InlineData("44444")] + public static void PortNumberInSPNTestForTCP(string port) + { + string connectionString = DataTestUtility.TCPConnectionString; + SqlConnectionStringBuilder builder = new(connectionString); + + if (!string.IsNullOrWhiteSpace(port)) + builder.DataSource = $"{builder.DataSource},{port}"; + + PortNumberInSPNTest(builder.ConnectionString); + } + + private static void PortNumberInSPNTest(string connStr) { - string connStr = DataTestUtility.TCPConnectionString; - // If config.json.SupportsIntegratedSecurity = true, replace all keys defined below with Integrated Security=true + // If config.json.Supports IntegratedSecurity = true, replace all keys defined below with Integrated Security=true if (DataTestUtility.IsIntegratedSecuritySetup()) { string[] removeKeys = { "Authentication", "User ID", "Password", "UID", "PWD", "Trusted_Connection" }; - connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.TCPConnectionString, removeKeys) + $"Integrated Security=true"; + connStr = DataTestUtility.RemoveKeysInConnStr(connStr, removeKeys) + $"Integrated Security=true"; } SqlConnectionStringBuilder builder = new(connStr); - Assert.True(DataTestUtility.ParseDataSource(builder.DataSource, out string hostname, out _, out string instanceName), "Data source to be parsed must contain a host name and instance name"); + string hostname = ""; + string instanceName = ""; + int port = -1; + DataTestUtility.ParseDataSource(builder.DataSource, out hostname, out port, out instanceName); + Assert.True(port != -2, "Named pipe protocol in data source does not support port number."); + Assert.True(!string.IsNullOrEmpty(hostname), "Hostname must be included in the data source."); + Assert.True(!string.IsNullOrEmpty(instanceName), "Instance name must be included in the data source."); + + bool isBrowserRunning = IsBrowserAlive(hostname); + Assert.True(isBrowserRunning, "Browser service is not running."); - bool condition = IsBrowserAlive(hostname) && IsValidInstance(hostname, instanceName); - Assert.True(condition, "Browser service is not running or instance name is invalid"); + bool isInstanceExisting = IsValidInstance(hostname, instanceName); + Assert.True(isInstanceExisting, "Instance name is invalid."); - if (condition) + if (isBrowserRunning && isInstanceExisting) { + // Create a connection object to ensure SPN info is available via reflection using SqlConnection connection = new(builder.ConnectionString); connection.Open(); - using SqlCommand command = new("SELECT auth_scheme, local_tcp_port from sys.dm_exec_connections where session_id = @@spid", connection); - using SqlDataReader reader = command.ExecuteReader(); - Assert.True(reader.Read(), "Expected to receive one row data"); - Assert.Equal("KERBEROS", reader.GetString(0)); - int localTcpPort = reader.GetInt32(1); - - int spnPort = -1; - string spnInfo = GetSPNInfo(builder.DataSource, out spnPort); - - // sample output to validate = MSSQLSvc/machine.domain.tld:spnPort" - Assert.Contains($"MSSQLSvc/{hostname}", spnInfo); - // the local_tcp_port should be the same as the inferred SPN port from instance name - Assert.Equal(localTcpPort, spnPort); + + // Get the SPN info using reflection + string spnInfo = GetSPNInfo(builder.DataSource); + + // The expected output to validate is supposed to be in the format "MSSQLSvc/machine.domain.tld:spnPort". + // So, we want to get the port number from the SPN and ensure it is a valid port number. + string[] spnStrs = spnInfo.Split(':'); + int portInSPN = 0; + if (spnStrs.Length > 1) + { + int.TryParse(spnStrs[1], out portInSPN); + } + Assert.True(portInSPN > 0, "The expected SPN must include a valid port number."); + + connection.Close(); } } - private static string GetSPNInfo(string datasource, out int out_port) + private static string GetSPNInfo(string datasource) { Assembly sqlConnectionAssembly = Assembly.GetAssembly(typeof(SqlConnection)); @@ -159,22 +196,23 @@ private static string GetSPNInfo(string datasource, out int out_port) object dataSourceObj = dataSourceCtor.Invoke(new object[] { datasource }); // Instantiate SSRP - object ssrp = SSRPCtor.Invoke(new object[] { }); + object ssrpObj = SSRPCtor.Invoke(new object[] { }); // Instantiate TimeoutTimer - object timeoutTimer = timeoutTimerCtor.Invoke(new object[] { }); + object timeoutTimerObj = timeoutTimerCtor.Invoke(new object[] { }); // Get TimeoutTimer.StartSecondsTimeout Method - MethodInfo startSecondsTimeout = timeoutTimer.GetType().GetMethod("StartSecondsTimeout", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, startSecondsTimeoutTypesArray, null); + MethodInfo startSecondsTimeout = timeoutTimerObj.GetType().GetMethod("StartSecondsTimeout", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, startSecondsTimeoutTypesArray, null); + // Create a timeoutTimer that expires in 30 seconds - timeoutTimer = startSecondsTimeout.Invoke(dataSourceObj, new object[] { 30 }); + timeoutTimerObj = startSecondsTimeout.Invoke(dataSourceObj, new object[] { 30 }); // Parse the datasource to separate the server name and instance name MethodInfo ParseServerName = dataSourceObj.GetType().GetMethod("ParseServerName", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, dataSourceConstructorTypesArray, null); object dataSrcInfo = ParseServerName.Invoke(dataSourceObj, new object[] { datasource }); // Get the GetPortByInstanceName method of SSRP - MethodInfo getPortByInstanceName = ssrp.GetType().GetMethod("GetPortByInstanceName", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, getPortByInstanceNameTypesArray, null); + MethodInfo getPortByInstanceName = ssrpObj.GetType().GetMethod("GetPortByInstanceName", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, getPortByInstanceNameTypesArray, null); // Get the server name PropertyInfo serverInfo = dataSrcInfo.GetType().GetProperty("ServerName", BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); @@ -185,7 +223,7 @@ private static string GetSPNInfo(string datasource, out int out_port) string instanceName = instanceNameInfo.GetValue(dataSrcInfo, null).ToString(); // Get the port number using the GetPortByInstanceName method of SSRP - object port = getPortByInstanceName.Invoke(ssrp, parameters: new object[] { serverName, instanceName, timeoutTimer, false, 0 }); + object port = getPortByInstanceName.Invoke(ssrpObj, parameters: new object[] { serverName, instanceName, timeoutTimerObj, false, 0 }); // Set the resolved port property of datasource PropertyInfo resolvedPortInfo = dataSrcInfo.GetType().GetProperty("ResolvedPort", BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); @@ -201,17 +239,13 @@ private static string GetSPNInfo(string datasource, out int out_port) // Example result: MSSQLSvc/machine.domain.tld:port" string spnInfo = Encoding.Unicode.GetString(result[0]); - out_port = (int)port; - return spnInfo; } - private static bool IsKerberos() + private static bool IsNotLocalHost() { - return (DataTestUtility.AreConnStringsSetup() - && DataTestUtility.IsNotLocalhost() - && DataTestUtility.IsKerberosTest - && DataTestUtility.IsNotAzureServer() + return (DataTestUtility.AreConnStringsSetup() + && DataTestUtility.IsNotAzureServer() && DataTestUtility.IsNotAzureSynapse()); } From 36c235fc4257b807d6af047d3db14fb8a01f6682 Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Wed, 20 Dec 2023 11:10:33 -0800 Subject: [PATCH 03/20] Try adding retry and delay in creating and using the connection object to fix intermittent issue in the pipeline. --- .../SQL/InstanceNameTest/InstanceNameTest.cs | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index c83f342c18..905fdde598 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -142,7 +142,20 @@ private static void PortNumberInSPNTest(string connStr) { // Create a connection object to ensure SPN info is available via reflection using SqlConnection connection = new(builder.ConnectionString); - connection.Open(); + + // This is failing intermittently in the pipeline. Perhaps it is hitting a limit on connection related issues. + int maxTry = 5; + while (connection.State != System.Data.ConnectionState.Open && maxTry > 1) + { + connection.Open(); + if (connection.State != System.Data.ConnectionState.Open) + // Wait for 30 seconds for available connection + System.Threading.Thread.Sleep(30000); + + maxTry--; + } + + // Get the SPN info using reflection string spnInfo = GetSPNInfo(builder.DataSource); @@ -158,11 +171,16 @@ private static void PortNumberInSPNTest(string connStr) Assert.True(portInSPN > 0, "The expected SPN must include a valid port number."); connection.Close(); + // Wait 2 seconds for connection to completely close before running second test + System.Threading.Thread.Sleep(2000); } } private static string GetSPNInfo(string datasource) { + // This is failing intermittently in the pipeline. Perhaps it is hitting a limit on connection related issues. + System.Threading.Thread.Sleep(500); + Assembly sqlConnectionAssembly = Assembly.GetAssembly(typeof(SqlConnection)); // Get all required types using reflection From 0caa4cef8a030766bc7af134fe63f051836905e1 Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Wed, 20 Dec 2023 12:08:13 -0800 Subject: [PATCH 04/20] Added IsUsingManagedSNI annotation since reflection is using ManagedSNI dll. --- .../SQL/InstanceNameTest/InstanceNameTest.cs | 37 +++++++++++++++---- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 905fdde598..3ccdc2feb1 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -7,8 +7,11 @@ using System.Net.Sockets; using System.Reflection; using System.Text; +using System.Threading; using System.Threading.Tasks; using Xunit; +using Xunit.Abstractions; +using Xunit.Sdk; namespace Microsoft.Data.SqlClient.ManualTesting.Tests { @@ -85,7 +88,7 @@ public static void ConnectManagedWithInstanceNameTest(bool useMultiSubnetFailove } [ActiveIssue("27981")] // DataSource.InferNamedPipesInformation is not initializing InstanceName field - [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.IsNotAzureServer), nameof(DataTestUtility.IsNotAzureSynapse), nameof(DataTestUtility.AreConnStringsSetup))] + [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.IsNotAzureServer), nameof(DataTestUtility.IsNotAzureSynapse), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsUsingManagedSNI))] [InlineData("")] [InlineData("44444")] public static void PortNumberInSPNTestForNP(string port) @@ -99,7 +102,7 @@ public static void PortNumberInSPNTestForNP(string port) PortNumberInSPNTest(builder.ConnectionString); } - [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.IsNotAzureServer), nameof(DataTestUtility.IsNotAzureSynapse), nameof(DataTestUtility.AreConnStringsSetup))] + [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.IsNotAzureServer), nameof(DataTestUtility.IsNotAzureSynapse), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsUsingManagedSNI))] [InlineData("")] [InlineData("44444")] public static void PortNumberInSPNTestForTCP(string port) @@ -142,20 +145,39 @@ private static void PortNumberInSPNTest(string connStr) { // Create a connection object to ensure SPN info is available via reflection using SqlConnection connection = new(builder.ConnectionString); + + // This is failing intermittently in the pipeline. Perhaps it is hitting a limit on connection related issues. --------------- + connection.Open(); + + Console.WriteLine($"Connection state after Open = {connection.State}"); - // This is failing intermittently in the pipeline. Perhaps it is hitting a limit on connection related issues. int maxTry = 5; while (connection.State != System.Data.ConnectionState.Open && maxTry > 1) { - connection.Open(); if (connection.State != System.Data.ConnectionState.Open) // Wait for 30 seconds for available connection System.Threading.Thread.Sleep(30000); + if (connection.State == System.Data.ConnectionState.Broken || connection.State == System.Data.ConnectionState.Closed) + { + if (connection.State == System.Data.ConnectionState.Broken) + { + Console.WriteLine($"Connection state before close = {connection.State}"); + connection.Close(); + // Wait for 5 seconds to fully close + System.Threading.Thread.Sleep(5000); + Console.WriteLine($"Connection state after close = {connection.State}"); + } + // Open connection once more + connection.Open(); + Console.WriteLine($"Connection state after re-open = {connection.State}"); + // Wait for 5 seconds to fully open + System.Threading.Thread.Sleep(5000); + } + maxTry--; } - - + // ---------------------------------------------------------------------------------------------------------------------------- // Get the SPN info using reflection string spnInfo = GetSPNInfo(builder.DataSource); @@ -178,7 +200,8 @@ private static void PortNumberInSPNTest(string connStr) private static string GetSPNInfo(string datasource) { - // This is failing intermittently in the pipeline. Perhaps it is hitting a limit on connection related issues. + // This method is failing intermittently in the pipeline with NRE using reflection. + // Perhaps some underlying objects are not completely initialized yet. System.Threading.Thread.Sleep(500); Assembly sqlConnectionAssembly = Assembly.GetAssembly(typeof(SqlConnection)); From c47a16e7ab7d60fc9e3df40ba236c9ac9bd3ea35 Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Wed, 20 Dec 2023 12:26:43 -0800 Subject: [PATCH 05/20] Add wrapper for all required annotations for the unit test as it is too long. --- .../SQL/InstanceNameTest/InstanceNameTest.cs | 44 +++++-------------- 1 file changed, 10 insertions(+), 34 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 3ccdc2feb1..98fc6c4dde 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -88,7 +88,7 @@ public static void ConnectManagedWithInstanceNameTest(bool useMultiSubnetFailove } [ActiveIssue("27981")] // DataSource.InferNamedPipesInformation is not initializing InstanceName field - [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.IsNotAzureServer), nameof(DataTestUtility.IsNotAzureSynapse), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsUsingManagedSNI))] + [ConditionalTheory(nameof(IsSPNPortNumberTest))] [InlineData("")] [InlineData("44444")] public static void PortNumberInSPNTestForNP(string port) @@ -102,7 +102,7 @@ public static void PortNumberInSPNTestForNP(string port) PortNumberInSPNTest(builder.ConnectionString); } - [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.IsNotAzureServer), nameof(DataTestUtility.IsNotAzureSynapse), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsUsingManagedSNI))] + [ConditionalTheory(nameof(IsSPNPortNumberTest))] [InlineData("")] [InlineData("44444")] public static void PortNumberInSPNTestForTCP(string port) @@ -145,40 +145,8 @@ private static void PortNumberInSPNTest(string connStr) { // Create a connection object to ensure SPN info is available via reflection using SqlConnection connection = new(builder.ConnectionString); - - // This is failing intermittently in the pipeline. Perhaps it is hitting a limit on connection related issues. --------------- connection.Open(); - Console.WriteLine($"Connection state after Open = {connection.State}"); - - int maxTry = 5; - while (connection.State != System.Data.ConnectionState.Open && maxTry > 1) - { - if (connection.State != System.Data.ConnectionState.Open) - // Wait for 30 seconds for available connection - System.Threading.Thread.Sleep(30000); - - if (connection.State == System.Data.ConnectionState.Broken || connection.State == System.Data.ConnectionState.Closed) - { - if (connection.State == System.Data.ConnectionState.Broken) - { - Console.WriteLine($"Connection state before close = {connection.State}"); - connection.Close(); - // Wait for 5 seconds to fully close - System.Threading.Thread.Sleep(5000); - Console.WriteLine($"Connection state after close = {connection.State}"); - } - // Open connection once more - connection.Open(); - Console.WriteLine($"Connection state after re-open = {connection.State}"); - // Wait for 5 seconds to fully open - System.Threading.Thread.Sleep(5000); - } - - maxTry--; - } - // ---------------------------------------------------------------------------------------------------------------------------- - // Get the SPN info using reflection string spnInfo = GetSPNInfo(builder.DataSource); @@ -290,6 +258,14 @@ private static bool IsNotLocalHost() && DataTestUtility.IsNotAzureSynapse()); } + private static bool IsSPNPortNumberTest() + { + return (DataTestUtility.AreConnStringsSetup() + && DataTestUtility.IsUsingManagedSNI() + && DataTestUtility.IsNotAzureServer() + && DataTestUtility.IsNotAzureSynapse()); + } + private static bool IsBrowserAlive(string browserHostname) { const byte ClntUcastEx = 0x03; From 64c0103a74ba6255bd1ef3067359042fc80cec79 Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Wed, 20 Dec 2023 12:35:59 -0800 Subject: [PATCH 06/20] Removed all Sleeps as they are not needed. --- .../SQL/InstanceNameTest/InstanceNameTest.cs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 98fc6c4dde..98acd647ab 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -161,17 +161,11 @@ private static void PortNumberInSPNTest(string connStr) Assert.True(portInSPN > 0, "The expected SPN must include a valid port number."); connection.Close(); - // Wait 2 seconds for connection to completely close before running second test - System.Threading.Thread.Sleep(2000); } } private static string GetSPNInfo(string datasource) { - // This method is failing intermittently in the pipeline with NRE using reflection. - // Perhaps some underlying objects are not completely initialized yet. - System.Threading.Thread.Sleep(500); - Assembly sqlConnectionAssembly = Assembly.GetAssembly(typeof(SqlConnection)); // Get all required types using reflection @@ -251,13 +245,6 @@ private static string GetSPNInfo(string datasource) return spnInfo; } - private static bool IsNotLocalHost() - { - return (DataTestUtility.AreConnStringsSetup() - && DataTestUtility.IsNotAzureServer() - && DataTestUtility.IsNotAzureSynapse()); - } - private static bool IsSPNPortNumberTest() { return (DataTestUtility.AreConnStringsSetup() From af685d32662c85cbbcec21ccc038635524150f27 Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Wed, 20 Dec 2023 13:01:05 -0800 Subject: [PATCH 07/20] Add annotation to skip .net framework for SPN port unit test. --- .../tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 98acd647ab..e15e21befc 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -91,6 +91,7 @@ public static void ConnectManagedWithInstanceNameTest(bool useMultiSubnetFailove [ConditionalTheory(nameof(IsSPNPortNumberTest))] [InlineData("")] [InlineData("44444")] + [SkipOnTargetFramework(~TargetFrameworkMonikers.NetFramework)] public static void PortNumberInSPNTestForNP(string port) { string connectionString = DataTestUtility.NPConnectionString; @@ -105,6 +106,7 @@ public static void PortNumberInSPNTestForNP(string port) [ConditionalTheory(nameof(IsSPNPortNumberTest))] [InlineData("")] [InlineData("44444")] + [SkipOnTargetFramework(~TargetFrameworkMonikers.NetFramework)] public static void PortNumberInSPNTestForTCP(string port) { string connectionString = DataTestUtility.TCPConnectionString; From 6076e8e087c4f206fd00e32ef85aa1d1ff6cf96a Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Wed, 20 Dec 2023 13:18:26 -0800 Subject: [PATCH 08/20] Put [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] so its the first annotation as the unit test did not skip. --- .../ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index e15e21befc..2bd6fa59ef 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -88,10 +88,10 @@ public static void ConnectManagedWithInstanceNameTest(bool useMultiSubnetFailove } [ActiveIssue("27981")] // DataSource.InferNamedPipesInformation is not initializing InstanceName field + [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] [ConditionalTheory(nameof(IsSPNPortNumberTest))] [InlineData("")] [InlineData("44444")] - [SkipOnTargetFramework(~TargetFrameworkMonikers.NetFramework)] public static void PortNumberInSPNTestForNP(string port) { string connectionString = DataTestUtility.NPConnectionString; @@ -103,10 +103,10 @@ public static void PortNumberInSPNTestForNP(string port) PortNumberInSPNTest(builder.ConnectionString); } + [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] [ConditionalTheory(nameof(IsSPNPortNumberTest))] [InlineData("")] [InlineData("44444")] - [SkipOnTargetFramework(~TargetFrameworkMonikers.NetFramework)] public static void PortNumberInSPNTestForTCP(string port) { string connectionString = DataTestUtility.TCPConnectionString; From bea551345aeecf0b5d2f8786bb7449ea56a82fd9 Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Wed, 20 Dec 2023 13:44:24 -0800 Subject: [PATCH 09/20] Removed SkipOnTargetFramework as it did not stop the unit test from running in pipeline. Added conditional compilation instead. --- .../ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 2bd6fa59ef..015c76978b 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -87,8 +87,8 @@ public static void ConnectManagedWithInstanceNameTest(bool useMultiSubnetFailove } } +#if NETCOREAPP [ActiveIssue("27981")] // DataSource.InferNamedPipesInformation is not initializing InstanceName field - [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] [ConditionalTheory(nameof(IsSPNPortNumberTest))] [InlineData("")] [InlineData("44444")] @@ -103,7 +103,6 @@ public static void PortNumberInSPNTestForNP(string port) PortNumberInSPNTest(builder.ConnectionString); } - [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] [ConditionalTheory(nameof(IsSPNPortNumberTest))] [InlineData("")] [InlineData("44444")] @@ -117,6 +116,7 @@ public static void PortNumberInSPNTestForTCP(string port) PortNumberInSPNTest(builder.ConnectionString); } +#endif private static void PortNumberInSPNTest(string connStr) { From fa556f544ee3751c4e99148fa58d0ec99ee1eb1c Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Wed, 20 Dec 2023 16:38:54 -0800 Subject: [PATCH 10/20] Add validation of InstanceName within the ConditionalThreory annotation. --- .../SQL/InstanceNameTest/InstanceNameTest.cs | 32 ++++++++++++++++--- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 015c76978b..9733146ab1 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -89,7 +89,7 @@ public static void ConnectManagedWithInstanceNameTest(bool useMultiSubnetFailove #if NETCOREAPP [ActiveIssue("27981")] // DataSource.InferNamedPipesInformation is not initializing InstanceName field - [ConditionalTheory(nameof(IsSPNPortNumberTest))] + [ConditionalTheory(nameof(IsSPNPortNumberTestForNP))] [InlineData("")] [InlineData("44444")] public static void PortNumberInSPNTestForNP(string port) @@ -103,7 +103,7 @@ public static void PortNumberInSPNTestForNP(string port) PortNumberInSPNTest(builder.ConnectionString); } - [ConditionalTheory(nameof(IsSPNPortNumberTest))] + [ConditionalTheory(nameof(IsSPNPortNumberTestForTCP))] [InlineData("")] [InlineData("44444")] public static void PortNumberInSPNTestForTCP(string port) @@ -247,14 +247,38 @@ private static string GetSPNInfo(string datasource) return spnInfo; } - private static bool IsSPNPortNumberTest() + private static bool IsSPNPortNumberTestForTCP() { - return (DataTestUtility.AreConnStringsSetup() + return (IsInstanceNameValid(DataTestUtility.TCPConnectionString) + && DataTestUtility.AreConnStringsSetup() && DataTestUtility.IsUsingManagedSNI() && DataTestUtility.IsNotAzureServer() && DataTestUtility.IsNotAzureSynapse()); } + private static bool IsSPNPortNumberTestForNP() + { + return (IsInstanceNameValid(DataTestUtility.NPConnectionString) + && DataTestUtility.AreConnStringsSetup() + && DataTestUtility.IsUsingManagedSNI() + && DataTestUtility.IsNotAzureServer() + && DataTestUtility.IsNotAzureSynapse()); + } + + private static bool IsInstanceNameValid(string connStr) + { + string hostname = ""; + string instanceName = ""; + int port = -1; + + SqlConnectionStringBuilder builder = new(connStr); + + DataTestUtility.ParseDataSource(builder.DataSource, out hostname, out port, out instanceName); + + return !string.IsNullOrWhiteSpace(instanceName); + + } + private static bool IsBrowserAlive(string browserHostname) { const byte ClntUcastEx = 0x03; From 6ace809555aea595fd1d16f3863137bfa00c12b4 Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Thu, 21 Dec 2023 14:25:28 -0800 Subject: [PATCH 11/20] Removed and sorted using references. --- .../ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 9733146ab1..778ed05c32 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -7,11 +7,8 @@ using System.Net.Sockets; using System.Reflection; using System.Text; -using System.Threading; using System.Threading.Tasks; using Xunit; -using Xunit.Abstractions; -using Xunit.Sdk; namespace Microsoft.Data.SqlClient.ManualTesting.Tests { @@ -91,7 +88,7 @@ public static void ConnectManagedWithInstanceNameTest(bool useMultiSubnetFailove [ActiveIssue("27981")] // DataSource.InferNamedPipesInformation is not initializing InstanceName field [ConditionalTheory(nameof(IsSPNPortNumberTestForNP))] [InlineData("")] - [InlineData("44444")] + [InlineData("44444")] // Named Instance Sql Server Port will be setup in the pipeline to 44444 as well public static void PortNumberInSPNTestForNP(string port) { string connectionString = DataTestUtility.NPConnectionString; @@ -105,7 +102,7 @@ public static void PortNumberInSPNTestForNP(string port) [ConditionalTheory(nameof(IsSPNPortNumberTestForTCP))] [InlineData("")] - [InlineData("44444")] + [InlineData("44444")] // Named Instance Sql Server Port will be setup in the pipeline to 44444 as well public static void PortNumberInSPNTestForTCP(string port) { string connectionString = DataTestUtility.TCPConnectionString; From 660d99228c2eb7e1f5333813ee0e95bb50b89203 Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Fri, 22 Dec 2023 09:55:38 -0800 Subject: [PATCH 12/20] Fix ParseDataSource named pipe data source detection to be case insensitive. --- .../tests/ManualTests/DataCommon/DataTestUtility.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs index 5577ff0bec..f1a8e67906 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs @@ -981,7 +981,7 @@ public static bool ParseDataSource(string dataSource, out string hostname, out i // Named pipe protocol data source does not support port number if ((dataSource.ToUpper().Contains(@"\MSSQL$") && dataSource.ToUpper().Contains(@"\SQL\QUERY") && dataSource.Contains(",")) || - (dataSource.ToUpper().Contains(@"np:") && dataSource.Contains(","))) + (dataSource.ToUpper().Contains(@"NP:") && dataSource.Contains(","))) { port = -2; return false; From 192a1e21c59287e56ed38c43ed30ed9b863c1042 Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Fri, 5 Jan 2024 09:10:24 -0800 Subject: [PATCH 13/20] Applied PR review suggesstions. --- .../ManualTests/DataCommon/DataTestUtility.cs | 8 ---- .../SQL/InstanceNameTest/InstanceNameTest.cs | 41 +++++++++++-------- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs index f1a8e67906..ddfd8ea3b5 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs @@ -979,14 +979,6 @@ public static bool ParseDataSource(string dataSource, out string hostname, out i port = -1; instanceName = string.Empty; - // Named pipe protocol data source does not support port number - if ((dataSource.ToUpper().Contains(@"\MSSQL$") && dataSource.ToUpper().Contains(@"\SQL\QUERY") && dataSource.Contains(",")) || - (dataSource.ToUpper().Contains(@"NP:") && dataSource.Contains(","))) - { - port = -2; - return false; - } - if (dataSource.Contains(":")) { dataSource = dataSource.Substring(dataSource.IndexOf(":", StringComparison.Ordinal) + 1); diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 778ed05c32..10a68d987c 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -115,24 +115,34 @@ public static void PortNumberInSPNTestForTCP(string port) } #endif - private static void PortNumberInSPNTest(string connStr) + private static void PortNumberInSPNTest(string connectionString) { // If config.json.Supports IntegratedSecurity = true, replace all keys defined below with Integrated Security=true if (DataTestUtility.IsIntegratedSecuritySetup()) { string[] removeKeys = { "Authentication", "User ID", "Password", "UID", "PWD", "Trusted_Connection" }; - connStr = DataTestUtility.RemoveKeysInConnStr(connStr, removeKeys) + $"Integrated Security=true"; + connectionString = DataTestUtility.RemoveKeysInConnStr(connectionString, removeKeys) + $"Integrated Security=true"; } - SqlConnectionStringBuilder builder = new(connStr); + SqlConnectionStringBuilder builder = new(connectionString); string hostname = ""; string instanceName = ""; int port = -1; + + // Named pipe protocol data source does not support port number + string dataSource = builder.DataSource.ToUpper(); + if ((dataSource.Contains(@"\MSSQL$") && builder.DataSource.ToUpper().Contains(@"\SQL\QUERY") && dataSource.Contains(",")) || + (dataSource.Contains(@"NP:") && dataSource.Contains(","))) + { + port = -2; + } + Assert.False(port == -2, "Named pipe protocol in data source does not support port number."); + DataTestUtility.ParseDataSource(builder.DataSource, out hostname, out port, out instanceName); - Assert.True(port != -2, "Named pipe protocol in data source does not support port number."); - Assert.True(!string.IsNullOrEmpty(hostname), "Hostname must be included in the data source."); - Assert.True(!string.IsNullOrEmpty(instanceName), "Instance name must be included in the data source."); + + Assert.False(string.IsNullOrEmpty(hostname), "Hostname must be included in the data source."); + Assert.False(string.IsNullOrEmpty(instanceName), "Instance name must be included in the data source."); bool isBrowserRunning = IsBrowserAlive(hostname); Assert.True(isBrowserRunning, "Browser service is not running."); @@ -143,7 +153,7 @@ private static void PortNumberInSPNTest(string connStr) if (isBrowserRunning && isInstanceExisting) { // Create a connection object to ensure SPN info is available via reflection - using SqlConnection connection = new(builder.ConnectionString); + SqlConnection connection = new(builder.ConnectionString); connection.Open(); // Get the SPN info using reflection @@ -163,7 +173,7 @@ private static void PortNumberInSPNTest(string connStr) } } - private static string GetSPNInfo(string datasource) + private static string GetSPNInfo(string dataSource) { Assembly sqlConnectionAssembly = Assembly.GetAssembly(typeof(SqlConnection)); @@ -194,8 +204,8 @@ private static string GetSPNInfo(string datasource) // Instantiate SNIProxy object sniProxy = sniProxyCtor.Invoke(new object[] { }); - // Instantiate datasource - object dataSourceObj = dataSourceCtor.Invoke(new object[] { datasource }); + // Instantiate dataSource + object dataSourceObj = dataSourceCtor.Invoke(new object[] { dataSource }); // Instantiate SSRP object ssrpObj = SSRPCtor.Invoke(new object[] { }); @@ -209,9 +219,9 @@ private static string GetSPNInfo(string datasource) // Create a timeoutTimer that expires in 30 seconds timeoutTimerObj = startSecondsTimeout.Invoke(dataSourceObj, new object[] { 30 }); - // Parse the datasource to separate the server name and instance name + // Parse the dataSource to separate the server name and instance name MethodInfo ParseServerName = dataSourceObj.GetType().GetMethod("ParseServerName", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, dataSourceConstructorTypesArray, null); - object dataSrcInfo = ParseServerName.Invoke(dataSourceObj, new object[] { datasource }); + object dataSrcInfo = ParseServerName.Invoke(dataSourceObj, new object[] { dataSource }); // Get the GetPortByInstanceName method of SSRP MethodInfo getPortByInstanceName = ssrpObj.GetType().GetMethod("GetPortByInstanceName", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, getPortByInstanceNameTypesArray, null); @@ -227,7 +237,7 @@ private static string GetSPNInfo(string datasource) // Get the port number using the GetPortByInstanceName method of SSRP object port = getPortByInstanceName.Invoke(ssrpObj, parameters: new object[] { serverName, instanceName, timeoutTimerObj, false, 0 }); - // Set the resolved port property of datasource + // Set the resolved port property of dataSource PropertyInfo resolvedPortInfo = dataSrcInfo.GetType().GetProperty("ResolvedPort", BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); resolvedPortInfo.SetValue(dataSrcInfo, (int)port, null); @@ -262,18 +272,17 @@ private static bool IsSPNPortNumberTestForNP() && DataTestUtility.IsNotAzureSynapse()); } - private static bool IsInstanceNameValid(string connStr) + private static bool IsInstanceNameValid(string connectionString) { string hostname = ""; string instanceName = ""; int port = -1; - SqlConnectionStringBuilder builder = new(connStr); + SqlConnectionStringBuilder builder = new(connectionString); DataTestUtility.ParseDataSource(builder.DataSource, out hostname, out port, out instanceName); return !string.IsNullOrWhiteSpace(instanceName); - } private static bool IsBrowserAlive(string browserHostname) From 8a93097982bd0c9fb58d502e679a42b5a128cb94 Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Wed, 10 Jan 2024 16:13:36 -0800 Subject: [PATCH 14/20] Moved named pipe protocol port usage assert message to line 138 and used Assert.Fail. --- .../ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 10a68d987c..83640ef9d3 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -135,10 +135,9 @@ private static void PortNumberInSPNTest(string connectionString) if ((dataSource.Contains(@"\MSSQL$") && builder.DataSource.ToUpper().Contains(@"\SQL\QUERY") && dataSource.Contains(",")) || (dataSource.Contains(@"NP:") && dataSource.Contains(","))) { - port = -2; + Assert.Fail("Named pipe protocol in data source does not support port number."); } - Assert.False(port == -2, "Named pipe protocol in data source does not support port number."); - + DataTestUtility.ParseDataSource(builder.DataSource, out hostname, out port, out instanceName); Assert.False(string.IsNullOrEmpty(hostname), "Hostname must be included in the data source."); From 00d9a3c2e076da402ec1b4e7bf719d24beaa930b Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Fri, 19 Jan 2024 11:19:45 -0800 Subject: [PATCH 15/20] Removed all source codes that handle named pipe protocol. --- .../SQL/InstanceNameTest/InstanceNameTest.cs | 40 +------------------ 1 file changed, 2 insertions(+), 38 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 83640ef9d3..edba3a7091 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -85,21 +85,6 @@ public static void ConnectManagedWithInstanceNameTest(bool useMultiSubnetFailove } #if NETCOREAPP - [ActiveIssue("27981")] // DataSource.InferNamedPipesInformation is not initializing InstanceName field - [ConditionalTheory(nameof(IsSPNPortNumberTestForNP))] - [InlineData("")] - [InlineData("44444")] // Named Instance Sql Server Port will be setup in the pipeline to 44444 as well - public static void PortNumberInSPNTestForNP(string port) - { - string connectionString = DataTestUtility.NPConnectionString; - SqlConnectionStringBuilder builder = new(connectionString); - - if (!string.IsNullOrWhiteSpace(port)) - builder.DataSource = $"{builder.DataSource},{port}"; - - PortNumberInSPNTest(builder.ConnectionString); - } - [ConditionalTheory(nameof(IsSPNPortNumberTestForTCP))] [InlineData("")] [InlineData("44444")] // Named Instance Sql Server Port will be setup in the pipeline to 44444 as well @@ -128,17 +113,8 @@ private static void PortNumberInSPNTest(string connectionString) string hostname = ""; string instanceName = ""; - int port = -1; - // Named pipe protocol data source does not support port number - string dataSource = builder.DataSource.ToUpper(); - if ((dataSource.Contains(@"\MSSQL$") && builder.DataSource.ToUpper().Contains(@"\SQL\QUERY") && dataSource.Contains(",")) || - (dataSource.Contains(@"NP:") && dataSource.Contains(","))) - { - Assert.Fail("Named pipe protocol in data source does not support port number."); - } - - DataTestUtility.ParseDataSource(builder.DataSource, out hostname, out port, out instanceName); + DataTestUtility.ParseDataSource(builder.DataSource, out hostname, out _, out instanceName); Assert.False(string.IsNullOrEmpty(hostname), "Hostname must be included in the data source."); Assert.False(string.IsNullOrEmpty(instanceName), "Instance name must be included in the data source."); @@ -256,16 +232,6 @@ private static string GetSPNInfo(string dataSource) private static bool IsSPNPortNumberTestForTCP() { return (IsInstanceNameValid(DataTestUtility.TCPConnectionString) - && DataTestUtility.AreConnStringsSetup() - && DataTestUtility.IsUsingManagedSNI() - && DataTestUtility.IsNotAzureServer() - && DataTestUtility.IsNotAzureSynapse()); - } - - private static bool IsSPNPortNumberTestForNP() - { - return (IsInstanceNameValid(DataTestUtility.NPConnectionString) - && DataTestUtility.AreConnStringsSetup() && DataTestUtility.IsUsingManagedSNI() && DataTestUtility.IsNotAzureServer() && DataTestUtility.IsNotAzureSynapse()); @@ -273,13 +239,11 @@ private static bool IsSPNPortNumberTestForNP() private static bool IsInstanceNameValid(string connectionString) { - string hostname = ""; string instanceName = ""; - int port = -1; SqlConnectionStringBuilder builder = new(connectionString); - DataTestUtility.ParseDataSource(builder.DataSource, out hostname, out port, out instanceName); + DataTestUtility.ParseDataSource(builder.DataSource, out _, out _, out instanceName); return !string.IsNullOrWhiteSpace(instanceName); } From 41366f6fbd2cec389068a9161b284c5d43382c14 Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Wed, 24 Jan 2024 12:49:05 -0800 Subject: [PATCH 16/20] Removed unwanted comments as they cause code bloat. Rename variables to what they are. Changed ConditionalTheory to ConditionalFact and get the port number of named instance from Sql Browser. --- .../SQL/InstanceNameTest/InstanceNameTest.cs | 122 +++++++++--------- 1 file changed, 62 insertions(+), 60 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index edba3a7091..534708f491 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -14,6 +14,8 @@ namespace Microsoft.Data.SqlClient.ManualTesting.Tests { public static class InstanceNameTest { + private const char SemicolonSeparator = ';'; + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsNotAzureServer), nameof(DataTestUtility.IsNotAzureSynapse), nameof(DataTestUtility.AreConnStringsSetup))] public static void ConnectToSQLWithInstanceNameTest() { @@ -85,15 +87,14 @@ public static void ConnectManagedWithInstanceNameTest(bool useMultiSubnetFailove } #if NETCOREAPP - [ConditionalTheory(nameof(IsSPNPortNumberTestForTCP))] - [InlineData("")] - [InlineData("44444")] // Named Instance Sql Server Port will be setup in the pipeline to 44444 as well - public static void PortNumberInSPNTestForTCP(string port) + [ConditionalFact(nameof(IsSPNPortNumberTestForTCP))] + public static void PortNumberInSPNTestForTCP() { string connectionString = DataTestUtility.TCPConnectionString; SqlConnectionStringBuilder builder = new(connectionString); - if (!string.IsNullOrWhiteSpace(port)) + int port = GetNamedInstancePortNumberFromSqlBrowser(connectionString); + if (port > 0) builder.DataSource = $"{builder.DataSource},{port}"; PortNumberInSPNTest(builder.ConnectionString); @@ -102,7 +103,6 @@ public static void PortNumberInSPNTestForTCP(string port) private static void PortNumberInSPNTest(string connectionString) { - // If config.json.Supports IntegratedSecurity = true, replace all keys defined below with Integrated Security=true if (DataTestUtility.IsIntegratedSecuritySetup()) { string[] removeKeys = { "Authentication", "User ID", "Password", "UID", "PWD", "Trusted_Connection" }; @@ -115,27 +115,16 @@ private static void PortNumberInSPNTest(string connectionString) string instanceName = ""; DataTestUtility.ParseDataSource(builder.DataSource, out hostname, out _, out instanceName); - + Assert.False(string.IsNullOrEmpty(hostname), "Hostname must be included in the data source."); Assert.False(string.IsNullOrEmpty(instanceName), "Instance name must be included in the data source."); - bool isBrowserRunning = IsBrowserAlive(hostname); - Assert.True(isBrowserRunning, "Browser service is not running."); - - bool isInstanceExisting = IsValidInstance(hostname, instanceName); - Assert.True(isInstanceExisting, "Instance name is invalid."); - - if (isBrowserRunning && isInstanceExisting) + using (SqlConnection connection = new(builder.ConnectionString)) { - // Create a connection object to ensure SPN info is available via reflection - SqlConnection connection = new(builder.ConnectionString); connection.Open(); - // Get the SPN info using reflection string spnInfo = GetSPNInfo(builder.DataSource); - // The expected output to validate is supposed to be in the format "MSSQLSvc/machine.domain.tld:spnPort". - // So, we want to get the port number from the SPN and ensure it is a valid port number. string[] spnStrs = spnInfo.Split(':'); int portInSPN = 0; if (spnStrs.Length > 1) @@ -143,8 +132,6 @@ private static void PortNumberInSPNTest(string connectionString) int.TryParse(spnStrs[1], out portInSPN); } Assert.True(portInSPN > 0, "The expected SPN must include a valid port number."); - - connection.Close(); } } @@ -152,78 +139,57 @@ private static string GetSPNInfo(string dataSource) { Assembly sqlConnectionAssembly = Assembly.GetAssembly(typeof(SqlConnection)); - // Get all required types using reflection Type sniProxyType = sqlConnectionAssembly.GetType("Microsoft.Data.SqlClient.SNI.SNIProxy"); Type ssrpType = sqlConnectionAssembly.GetType("Microsoft.Data.SqlClient.SNI.SSRP"); Type dataSourceType = sqlConnectionAssembly.GetType("Microsoft.Data.SqlClient.SNI.DataSource"); Type timeoutTimerType = sqlConnectionAssembly.GetType("Microsoft.Data.ProviderBase.TimeoutTimer"); - // Used in Datasource constructor param type array Type[] dataSourceConstructorTypesArray = new Type[] { typeof(string) }; - // Used in GetSqlServerSPNs function param types array Type[] getSqlServerSPNsTypesArray = new Type[] { dataSourceType, typeof(string) }; - // GetPortByInstanceName parameters array Type[] getPortByInstanceNameTypesArray = new Type[] { typeof(string), typeof(string), timeoutTimerType, typeof(bool), typeof(Microsoft.Data.SqlClient.SqlConnectionIPAddressPreference) }; - // TimeoutTimer.StartSecondsTimeout params Type[] startSecondsTimeoutTypesArray = new Type[] { typeof(int) }; - // Get all types constructors - ConstructorInfo sniProxyCtor = sniProxyType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, Type.EmptyTypes, null); - ConstructorInfo SSRPCtor = ssrpType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, Type.EmptyTypes, null); - ConstructorInfo dataSourceCtor = dataSourceType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, dataSourceConstructorTypesArray, null); - ConstructorInfo timeoutTimerCtor = timeoutTimerType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, Type.EmptyTypes, null); + ConstructorInfo sniProxyConstructor = sniProxyType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, Type.EmptyTypes, null); + ConstructorInfo SSRPConstructor = ssrpType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, Type.EmptyTypes, null); + ConstructorInfo dataSourceConstructor = dataSourceType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, dataSourceConstructorTypesArray, null); + ConstructorInfo timeoutTimerConstructor = timeoutTimerType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, Type.EmptyTypes, null); + + object sniProxyObj = sniProxyConstructor.Invoke(new object[] { }); - // Instantiate SNIProxy - object sniProxy = sniProxyCtor.Invoke(new object[] { }); + object dataSourceObj = dataSourceConstructor.Invoke(new object[] { dataSource }); - // Instantiate dataSource - object dataSourceObj = dataSourceCtor.Invoke(new object[] { dataSource }); + object ssrpObj = SSRPConstructor.Invoke(new object[] { }); - // Instantiate SSRP - object ssrpObj = SSRPCtor.Invoke(new object[] { }); + object timeoutTimerObj = timeoutTimerConstructor.Invoke(new object[] { }); - // Instantiate TimeoutTimer - object timeoutTimerObj = timeoutTimerCtor.Invoke(new object[] { }); + MethodInfo startSecondsTimeoutInfo = timeoutTimerObj.GetType().GetMethod("StartSecondsTimeout", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, startSecondsTimeoutTypesArray, null); - // Get TimeoutTimer.StartSecondsTimeout Method - MethodInfo startSecondsTimeout = timeoutTimerObj.GetType().GetMethod("StartSecondsTimeout", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, startSecondsTimeoutTypesArray, null); - - // Create a timeoutTimer that expires in 30 seconds - timeoutTimerObj = startSecondsTimeout.Invoke(dataSourceObj, new object[] { 30 }); + timeoutTimerObj = startSecondsTimeoutInfo.Invoke(dataSourceObj, new object[] { 30 }); - // Parse the dataSource to separate the server name and instance name - MethodInfo ParseServerName = dataSourceObj.GetType().GetMethod("ParseServerName", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, dataSourceConstructorTypesArray, null); - object dataSrcInfo = ParseServerName.Invoke(dataSourceObj, new object[] { dataSource }); + MethodInfo parseServerNameInfo = dataSourceObj.GetType().GetMethod("ParseServerName", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, dataSourceConstructorTypesArray, null); + object dataSrcInfo = parseServerNameInfo.Invoke(dataSourceObj, new object[] { dataSource }); - // Get the GetPortByInstanceName method of SSRP - MethodInfo getPortByInstanceName = ssrpObj.GetType().GetMethod("GetPortByInstanceName", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, getPortByInstanceNameTypesArray, null); + MethodInfo getPortByInstanceNameInfo = ssrpObj.GetType().GetMethod("GetPortByInstanceName", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, getPortByInstanceNameTypesArray, null); - // Get the server name PropertyInfo serverInfo = dataSrcInfo.GetType().GetProperty("ServerName", BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); string serverName = serverInfo.GetValue(dataSrcInfo, null).ToString(); - // Get the instance name PropertyInfo instanceNameInfo = dataSrcInfo.GetType().GetProperty("InstanceName", BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); string instanceName = instanceNameInfo.GetValue(dataSrcInfo, null).ToString(); - // Get the port number using the GetPortByInstanceName method of SSRP - object port = getPortByInstanceName.Invoke(ssrpObj, parameters: new object[] { serverName, instanceName, timeoutTimerObj, false, 0 }); + object port = getPortByInstanceNameInfo.Invoke(ssrpObj, parameters: new object[] { serverName, instanceName, timeoutTimerObj, false, 0 }); - // Set the resolved port property of dataSource PropertyInfo resolvedPortInfo = dataSrcInfo.GetType().GetProperty("ResolvedPort", BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); resolvedPortInfo.SetValue(dataSrcInfo, (int)port, null); - // Prepare the GetSqlServerSPNs method string serverSPN = ""; - MethodInfo getSqlServerSPNs = sniProxy.GetType().GetMethod("GetSqlServerSPNs", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, getSqlServerSPNsTypesArray, null); + MethodInfo getSqlServerSPNs = sniProxyObj.GetType().GetMethod("GetSqlServerSPNs", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, getSqlServerSPNsTypesArray, null); - // Finally call GetSqlServerSPNs - byte[][] result = (byte[][])getSqlServerSPNs.Invoke(sniProxy, new object[] { dataSrcInfo, serverSPN }); + byte[][] result = (byte[][])getSqlServerSPNs.Invoke(sniProxyObj, new object[] { dataSrcInfo, serverSPN }); - // Example result: MSSQLSvc/machine.domain.tld:port" string spnInfo = Encoding.Unicode.GetString(result[0]); return spnInfo; @@ -242,7 +208,7 @@ private static bool IsInstanceNameValid(string connectionString) string instanceName = ""; SqlConnectionStringBuilder builder = new(connectionString); - + DataTestUtility.ParseDataSource(builder.DataSource, out _, out _, out instanceName); return !string.IsNullOrWhiteSpace(instanceName); @@ -263,6 +229,42 @@ private static bool IsValidInstance(string browserHostName, string instanceName) return response != null && response.Length > 0; } + private static int GetNamedInstancePortNumberFromSqlBrowser(string connectionString) + { + SqlConnectionStringBuilder builder = new(connectionString); + + string hostname = ""; + string instanceName = ""; + int port = 0; + + DataTestUtility.ParseDataSource(builder.DataSource, out hostname, out _, out instanceName); + + bool isBrowserRunning = IsBrowserAlive(hostname); + Assert.True(isBrowserRunning, "Browser service is not running."); + + bool isInstanceExisting = IsValidInstance(hostname, instanceName); + Assert.True(isInstanceExisting, "Instance name is invalid."); + + if (isBrowserRunning && isInstanceExisting) + { + byte[] request = CreateInstanceInfoRequest(instanceName); + byte[] response = QueryBrowser(hostname, request); + + string serverMessage = Encoding.ASCII.GetString(response, 3, response.Length - 3); + + string[] elements = serverMessage.Split(SemicolonSeparator); + int tcpIndex = Array.IndexOf(elements, "tcp"); + if (tcpIndex < 0 || tcpIndex == elements.Length - 1) + { + throw new SocketException(); + } + + port = (int)ushort.Parse(elements[tcpIndex + 1]); + } + + return port; + } + private static byte[] QueryBrowser(string browserHostname, byte[] requestPacket) { const int DefaultBrowserPort = 1434; From baa3ab98a905fb9089aea87b1ab88fa3ca9c0bfc Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Wed, 24 Jan 2024 13:28:26 -0800 Subject: [PATCH 17/20] Add SPN pattern validation and use Assert.Equal to compare expected port number agains port number returned in SPN. --- .../ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 534708f491..8b62777626 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -97,11 +97,11 @@ public static void PortNumberInSPNTestForTCP() if (port > 0) builder.DataSource = $"{builder.DataSource},{port}"; - PortNumberInSPNTest(builder.ConnectionString); + PortNumberInSPNTest(builder.ConnectionString, port); } #endif - private static void PortNumberInSPNTest(string connectionString) + private static void PortNumberInSPNTest(string connectionString, int expectedPortNumber) { if (DataTestUtility.IsIntegratedSecuritySetup()) { @@ -124,6 +124,7 @@ private static void PortNumberInSPNTest(string connectionString) connection.Open(); string spnInfo = GetSPNInfo(builder.DataSource); + Assert.Matches(@"MSSQLSvc\/.*:[\d]", spnInfo); string[] spnStrs = spnInfo.Split(':'); int portInSPN = 0; @@ -131,7 +132,7 @@ private static void PortNumberInSPNTest(string connectionString) { int.TryParse(spnStrs[1], out portInSPN); } - Assert.True(portInSPN > 0, "The expected SPN must include a valid port number."); + Assert.Equal(expectedPortNumber, portInSPN); } } From 2bc3f7cc7c25aae20f447c253bc811235a0e7cd4 Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Wed, 31 Jan 2024 17:18:59 -0800 Subject: [PATCH 18/20] Added validation for DataSource. --- .../ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 8b62777626..b80f489d09 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -210,9 +210,9 @@ private static bool IsInstanceNameValid(string connectionString) SqlConnectionStringBuilder builder = new(connectionString); - DataTestUtility.ParseDataSource(builder.DataSource, out _, out _, out instanceName); + bool isDataSourceValid = DataTestUtility.ParseDataSource(builder.DataSource, out _, out _, out instanceName); - return !string.IsNullOrWhiteSpace(instanceName); + return isDataSourceValid && !string.IsNullOrWhiteSpace(instanceName); } private static bool IsBrowserAlive(string browserHostname) @@ -238,7 +238,7 @@ private static int GetNamedInstancePortNumberFromSqlBrowser(string connectionStr string instanceName = ""; int port = 0; - DataTestUtility.ParseDataSource(builder.DataSource, out hostname, out _, out instanceName); + bool isDataSourceValid = DataTestUtility.ParseDataSource(builder.DataSource, out hostname, out _, out instanceName); bool isBrowserRunning = IsBrowserAlive(hostname); Assert.True(isBrowserRunning, "Browser service is not running."); @@ -246,7 +246,7 @@ private static int GetNamedInstancePortNumberFromSqlBrowser(string connectionStr bool isInstanceExisting = IsValidInstance(hostname, instanceName); Assert.True(isInstanceExisting, "Instance name is invalid."); - if (isBrowserRunning && isInstanceExisting) + if (isDataSourceValid && isBrowserRunning && isInstanceExisting) { byte[] request = CreateInstanceInfoRequest(instanceName); byte[] response = QueryBrowser(hostname, request); From c131562de6ea2c31a4dc8b26184495d6f7db24a4 Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Wed, 31 Jan 2024 17:21:53 -0800 Subject: [PATCH 19/20] Added assertion if DataSource is valid. --- .../tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index b80f489d09..19924b3d1f 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -239,6 +239,7 @@ private static int GetNamedInstancePortNumberFromSqlBrowser(string connectionStr int port = 0; bool isDataSourceValid = DataTestUtility.ParseDataSource(builder.DataSource, out hostname, out _, out instanceName); + Assert.True(isDataSourceValid, "DataSource is invalid"); bool isBrowserRunning = IsBrowserAlive(hostname); Assert.True(isBrowserRunning, "Browser service is not running."); From 0c5ed6f04c74dd1be0901111ff820030fd55a0e2 Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Fri, 23 Feb 2024 10:24:07 -0800 Subject: [PATCH 20/20] Replace If (port > 0) with Assert( port > 0, ...) --- .../ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 19924b3d1f..205b9d33f1 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -94,8 +94,8 @@ public static void PortNumberInSPNTestForTCP() SqlConnectionStringBuilder builder = new(connectionString); int port = GetNamedInstancePortNumberFromSqlBrowser(connectionString); - if (port > 0) - builder.DataSource = $"{builder.DataSource},{port}"; + Assert.True(port > 0, "Named instance must have a valid port number."); + builder.DataSource = $"{builder.DataSource},{port}"; PortNumberInSPNTest(builder.ConnectionString, port); }