From 987ca661eced172b8b5512d1f11131654a01e04f Mon Sep 17 00:00:00 2001 From: David Levy Date: Tue, 27 Jan 2026 18:49:49 -0600 Subject: [PATCH 1/4] fix: skip live connection tests when SQL auth credentials unavailable The Azure DevOps pipeline runs tests twice: 1. SQL2022: Uses SQL auth (sa user) - tests will run 2. SQLDB: Uses Azure AD auth (service principal) - tests should skip The live connection e2e tests require SQL auth credentials (SQLCMDUSER/SQLCMDPASSWORD) which aren't available in the Azure AD auth scenario. Added skipIfNoSQLAuth() to properly skip these tests instead of failing with 'Login failed for user ''. --- cmd/modern/e2e_test.go | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/cmd/modern/e2e_test.go b/cmd/modern/e2e_test.go index 3c13ce59..e02a2c38 100644 --- a/cmd/modern/e2e_test.go +++ b/cmd/modern/e2e_test.go @@ -72,6 +72,12 @@ func hasLiveConnection() bool { return os.Getenv("SQLCMDSERVER") != "" } +// hasSQLAuthCredentials returns true if SQL authentication credentials are available. +// For Azure AD/Entra authentication (service principal), we need different handling. +func hasSQLAuthCredentials() bool { + return os.Getenv("SQLCMDUSER") != "" && os.Getenv("SQLCMDPASSWORD") != "" +} + // skipIfNoLiveConnection skips the test if no live SQL Server connection is available. func skipIfNoLiveConnection(t *testing.T) { t.Helper() @@ -80,6 +86,17 @@ func skipIfNoLiveConnection(t *testing.T) { } } +// skipIfNoSQLAuth skips the test if SQL authentication credentials are not available. +// Tests requiring SQL auth should use this instead of skipIfNoLiveConnection when they +// don't support Azure AD/Entra authentication. +func skipIfNoSQLAuth(t *testing.T) { + t.Helper() + skipIfNoLiveConnection(t) + if !hasSQLAuthCredentials() { + t.Skip("Skipping: SQLCMDUSER/SQLCMDPASSWORD not set, SQL auth not available (may be using Azure AD)") + } +} + type buildError struct { err error output string @@ -143,9 +160,10 @@ func TestE2E_PipedInput_NoPanic(t *testing.T) { } // TestE2E_PipedInput_LiveConnection tests piping input with a real SQL Server connection. -// This test only runs when SQLCMDSERVER is set. +// This test only runs when SQLCMDSERVER and SQL auth credentials are set. +// It does not support Azure AD/Entra authentication yet. func TestE2E_PipedInput_LiveConnection(t *testing.T) { - skipIfNoLiveConnection(t) + skipIfNoSQLAuth(t) binary := buildBinary(t) cmd := exec.Command(binary, "-C") @@ -208,9 +226,9 @@ func TestE2E_QueryFlag_NoServer(t *testing.T) { } // TestE2E_QueryFlag_LiveConnection tests the -Q flag with a real SQL Server connection. -// This test only runs when SQLCMDSERVER is set. +// This test only runs when SQLCMDSERVER and SQL auth credentials are set. func TestE2E_QueryFlag_LiveConnection(t *testing.T) { - skipIfNoLiveConnection(t) + skipIfNoSQLAuth(t) binary := buildBinary(t) cmd := exec.Command(binary, "-C", "-Q", "SELECT 42 AS Answer") @@ -237,9 +255,9 @@ func TestE2E_InputFile_NotFound(t *testing.T) { } // TestE2E_InputFile_LiveConnection tests the -i flag with a real SQL Server connection. -// This test only runs when SQLCMDSERVER is set. +// This test only runs when SQLCMDSERVER and SQL auth credentials are set. func TestE2E_InputFile_LiveConnection(t *testing.T) { - skipIfNoLiveConnection(t) + skipIfNoSQLAuth(t) binary := buildBinary(t) // Create a temporary SQL file From bb3bb71692b9be54194bf4949e58b9fb6116afe1 Mon Sep 17 00:00:00 2001 From: David Levy Date: Tue, 27 Jan 2026 18:55:11 -0600 Subject: [PATCH 2/4] fix: generalize Azure AD comment per code review --- cmd/modern/e2e_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/modern/e2e_test.go b/cmd/modern/e2e_test.go index e02a2c38..02bfd4db 100644 --- a/cmd/modern/e2e_test.go +++ b/cmd/modern/e2e_test.go @@ -73,7 +73,7 @@ func hasLiveConnection() bool { } // hasSQLAuthCredentials returns true if SQL authentication credentials are available. -// For Azure AD/Entra authentication (service principal), we need different handling. +// For Azure AD/Entra authentication methods, we need different handling. func hasSQLAuthCredentials() bool { return os.Getenv("SQLCMDUSER") != "" && os.Getenv("SQLCMDPASSWORD") != "" } From 3b693d51f3988089a0bb401288cabc4768a11356 Mon Sep 17 00:00:00 2001 From: David Levy Date: Wed, 28 Jan 2026 12:41:04 -0600 Subject: [PATCH 3/4] feat: support Azure AD auth in e2e tests Instead of skipping tests when SQL auth credentials are unavailable, detect Azure AD authentication and use appropriate --authentication-method flag, matching the approach used in pkg/sqlcmd/sqlcmd_test.go. Tests now run on both SQL2022 (SQL auth) and SQLDB (Azure AD) pipelines. --- cmd/modern/e2e_test.go | 56 ++++++++++++++++++++++++++++-------------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/cmd/modern/e2e_test.go b/cmd/modern/e2e_test.go index 02bfd4db..c5bc1402 100644 --- a/cmd/modern/e2e_test.go +++ b/cmd/modern/e2e_test.go @@ -73,11 +73,19 @@ func hasLiveConnection() bool { } // hasSQLAuthCredentials returns true if SQL authentication credentials are available. -// For Azure AD/Entra authentication methods, we need different handling. func hasSQLAuthCredentials() bool { return os.Getenv("SQLCMDUSER") != "" && os.Getenv("SQLCMDPASSWORD") != "" } +// canTestAzureAuth returns true if Azure AD authentication should be used. +// This matches the logic in pkg/sqlcmd/sqlcmd_test.go - use AAD when +// SQLCMDSERVER is an Azure SQL endpoint and SQLCMDUSER is not set. +func canTestAzureAuth() bool { + server := os.Getenv("SQLCMDSERVER") + userName := os.Getenv("SQLCMDUSER") + return strings.Contains(server, ".database.windows.net") && userName == "" +} + // skipIfNoLiveConnection skips the test if no live SQL Server connection is available. func skipIfNoLiveConnection(t *testing.T) { t.Helper() @@ -86,15 +94,22 @@ func skipIfNoLiveConnection(t *testing.T) { } } -// skipIfNoSQLAuth skips the test if SQL authentication credentials are not available. -// Tests requiring SQL auth should use this instead of skipIfNoLiveConnection when they -// don't support Azure AD/Entra authentication. -func skipIfNoSQLAuth(t *testing.T) { +// getAuthArgs returns the command-line arguments needed for authentication. +// For SQL auth, it returns empty (sqlcmd picks up SQLCMDUSER/SQLCMDPASSWORD from env). +// For Azure AD, it returns the appropriate --authentication-method flag. +func getAuthArgs(t *testing.T) []string { t.Helper() - skipIfNoLiveConnection(t) - if !hasSQLAuthCredentials() { - t.Skip("Skipping: SQLCMDUSER/SQLCMDPASSWORD not set, SQL auth not available (may be using Azure AD)") + if canTestAzureAuth() { + // Check if running in Azure Pipelines with service connection + if os.Getenv("AZURESUBSCRIPTION_SERVICE_CONNECTION_NAME") != "" { + t.Log("Using ActiveDirectoryAzurePipelines authentication") + return []string{"--authentication-method", "ActiveDirectoryAzurePipelines"} + } + t.Log("Using ActiveDirectoryDefault authentication") + return []string{"--authentication-method", "ActiveDirectoryDefault"} } + // SQL auth - credentials come from environment variables + return []string{} } type buildError struct { @@ -160,15 +175,16 @@ func TestE2E_PipedInput_NoPanic(t *testing.T) { } // TestE2E_PipedInput_LiveConnection tests piping input with a real SQL Server connection. -// This test only runs when SQLCMDSERVER and SQL auth credentials are set. -// It does not support Azure AD/Entra authentication yet. +// This test runs when SQLCMDSERVER is set and uses appropriate auth method +// (SQL auth or Azure AD) based on environment configuration. func TestE2E_PipedInput_LiveConnection(t *testing.T) { - skipIfNoSQLAuth(t) + skipIfNoLiveConnection(t) binary := buildBinary(t) - cmd := exec.Command(binary, "-C") + args := append([]string{"-C"}, getAuthArgs(t)...) + cmd := exec.Command(binary, args...) cmd.Stdin = strings.NewReader("SELECT 1 AS TestValue\nGO\n") - cmd.Env = os.Environ() // Inherit SQLCMDSERVER, SQLCMDUSER, SQLCMDPASSWORD + cmd.Env = os.Environ() // Inherit environment variables output, err := cmd.CombinedOutput() outputStr := string(output) @@ -226,12 +242,13 @@ func TestE2E_QueryFlag_NoServer(t *testing.T) { } // TestE2E_QueryFlag_LiveConnection tests the -Q flag with a real SQL Server connection. -// This test only runs when SQLCMDSERVER and SQL auth credentials are set. +// This test runs when SQLCMDSERVER is set and uses appropriate auth method. func TestE2E_QueryFlag_LiveConnection(t *testing.T) { - skipIfNoSQLAuth(t) + skipIfNoLiveConnection(t) binary := buildBinary(t) - cmd := exec.Command(binary, "-C", "-Q", "SELECT 42 AS Answer") + args := append([]string{"-C", "-Q", "SELECT 42 AS Answer"}, getAuthArgs(t)...) + cmd := exec.Command(binary, args...) cmd.Env = os.Environ() output, err := cmd.CombinedOutput() @@ -255,9 +272,9 @@ func TestE2E_InputFile_NotFound(t *testing.T) { } // TestE2E_InputFile_LiveConnection tests the -i flag with a real SQL Server connection. -// This test only runs when SQLCMDSERVER and SQL auth credentials are set. +// This test runs when SQLCMDSERVER is set and uses appropriate auth method. func TestE2E_InputFile_LiveConnection(t *testing.T) { - skipIfNoSQLAuth(t) + skipIfNoLiveConnection(t) binary := buildBinary(t) // Create a temporary SQL file @@ -269,7 +286,8 @@ func TestE2E_InputFile_LiveConnection(t *testing.T) { require.NoError(t, err) require.NoError(t, tmpFile.Close()) - cmd := exec.Command(binary, "-C", "-i", tmpFile.Name()) + args := append([]string{"-C", "-i", tmpFile.Name()}, getAuthArgs(t)...) + cmd := exec.Command(binary, args...) cmd.Env = os.Environ() output, err := cmd.CombinedOutput() From 6b6d53539405f08c3fe1ac189e2292a8b1191f8c Mon Sep 17 00:00:00 2001 From: David Levy Date: Wed, 28 Jan 2026 12:52:49 -0600 Subject: [PATCH 4/4] fix: remove unused hasSQLAuthCredentials function --- cmd/modern/e2e_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/cmd/modern/e2e_test.go b/cmd/modern/e2e_test.go index c5bc1402..9c3d14e6 100644 --- a/cmd/modern/e2e_test.go +++ b/cmd/modern/e2e_test.go @@ -72,11 +72,6 @@ func hasLiveConnection() bool { return os.Getenv("SQLCMDSERVER") != "" } -// hasSQLAuthCredentials returns true if SQL authentication credentials are available. -func hasSQLAuthCredentials() bool { - return os.Getenv("SQLCMDUSER") != "" && os.Getenv("SQLCMDPASSWORD") != "" -} - // canTestAzureAuth returns true if Azure AD authentication should be used. // This matches the logic in pkg/sqlcmd/sqlcmd_test.go - use AAD when // SQLCMDSERVER is an Azure SQL endpoint and SQLCMDUSER is not set.