From e52bc7d35ad1f91ada09477fd8779015a74072f3 Mon Sep 17 00:00:00 2001 From: Andreas Jordan Date: Fri, 14 Aug 2026 10:59:11 +0200 Subject: [PATCH 1/3] Import-DbaSpConfigure - Leave the connection of the caller alone The command closed every connection it used in its end block, no matter who opened it. On a session-scoped connection that takes the session of the caller with it. Both connections are now reported by Connect-DbaInstance through IsNewConnectionReference, and only a connection opened here is closed. While in there: the file import set show advanced options on the Configuration collection without ever calling Alter(), so it never reached the instance but left a pending change on the server object of the caller. The file written by Export-DbaSpConfigure sets the option itself, so the two lines are gone. (do Import-DbaSpConfigure) Co-Authored-By: Claude Opus 5 (1M context) --- public/Import-DbaSpConfigure.ps1 | 41 +++++-- tests/Import-DbaSpConfigure.Tests.ps1 | 168 +++++++++++++++++++++++++- 2 files changed, 197 insertions(+), 12 deletions(-) diff --git a/public/Import-DbaSpConfigure.ps1 b/public/Import-DbaSpConfigure.ps1 index c3dbeec86da9..cdfc192e5af4 100644 --- a/public/Import-DbaSpConfigure.ps1 +++ b/public/Import-DbaSpConfigure.ps1 @@ -119,9 +119,20 @@ function Import-DbaSpConfigure { [switch]$EnableException ) begin { + # Connect-DbaInstance tells us whether it opened a connection for us. We must only close what we opened + # ourselves, because closing a connection of the caller takes their session with it. See #10554. + $isNewSourceConnection = $false + $isNewDestinationConnection = $false + $isNewServerConnection = $false + if (-not $PSBoundParameters.Path -and $PSBoundParameters.Source) { try { - $sourceserver = Connect-DbaInstance -SqlInstance $Source -SqlCredential $SourceSqlCredential + $splatConnectSource = @{ + SqlInstance = $Source + SqlCredential = $SourceSqlCredential + IsNewConnectionReference = [ref]$isNewSourceConnection + } + $sourceserver = Connect-DbaInstance @splatConnectSource } catch { Stop-Function -Message "Failure" -Category ConnectionError -ErrorRecord $_ -Target $Source return @@ -132,7 +143,12 @@ function Import-DbaSpConfigure { } try { - $destserver = Connect-DbaInstance -SqlInstance $Destination -SqlCredential $DestinationSqlCredential + $splatConnectDestination = @{ + SqlInstance = $Destination + SqlCredential = $DestinationSqlCredential + IsNewConnectionReference = [ref]$isNewDestinationConnection + } + $destserver = Connect-DbaInstance @splatConnectDestination } catch { Stop-Function -Message "Failure" -Category ConnectionError -ErrorRecord $_ -Target $Destination return @@ -146,7 +162,12 @@ function Import-DbaSpConfigure { $destination = $destserver.DomainInstanceName } else { try { - $server = Connect-DbaInstance -SqlInstance $SqlInstance -SqlCredential $SqlCredential + $splatConnectServer = @{ + SqlInstance = $SqlInstance + SqlCredential = $SqlCredential + IsNewConnectionReference = [ref]$isNewServerConnection + } + $server = Connect-DbaInstance @splatConnectServer } catch { Stop-Function -Message "Failure" -Category ConnectionError -ErrorRecord $_ -Target $SqlInstance return @@ -221,7 +242,10 @@ function Import-DbaSpConfigure { } else { if ($Pscmdlet.ShouldProcess($destination, "Importing sp_configure from $Path")) { - $server.Configuration.ShowAdvancedOptions.ConfigValue = $true + # 'show advanced options' is not touched here. It used to be set on the Configuration collection + # without ever calling Alter(), so it never reached the instance - but it did leave a pending + # change on the server object of the caller, which their next Alter() would have applied. + # The file written by Export-DbaSpConfigure sets the option itself, first to 1 and then back to 0. $sql = Get-Content $Path foreach ($line in $sql) { try { @@ -231,7 +255,6 @@ function Import-DbaSpConfigure { Stop-Function -Message "$line failed. Feature may not be supported." -ErrorRecord $_ -Continue } } - $server.Configuration.ShowAdvancedOptions.ConfigValue = $false Write-Message -Level Warning -Message "Some configuration options will be updated once SQL Server is restarted." } } @@ -239,10 +262,14 @@ function Import-DbaSpConfigure { end { if (Test-FunctionInterrupt) { return } - if ($PSBoundParameters.Path) { + # Only close the connections that were opened here. See #10554. + if ($isNewServerConnection) { $server.ConnectionContext.Disconnect() - } else { + } + if ($isNewSourceConnection) { $sourceserver.ConnectionContext.Disconnect() + } + if ($isNewDestinationConnection) { $destserver.ConnectionContext.Disconnect() } diff --git a/tests/Import-DbaSpConfigure.Tests.ps1 b/tests/Import-DbaSpConfigure.Tests.ps1 index eda14cbcd3e3..24eb08c2a056 100644 --- a/tests/Import-DbaSpConfigure.Tests.ps1 +++ b/tests/Import-DbaSpConfigure.Tests.ps1 @@ -25,8 +25,166 @@ Describe $CommandName -Tag UnitTests { } } } -<# - Integration test should appear below and are custom to the command you are writing. - Read https://github.com/dataplat/dbatools/blob/development/contributing.md#tests - for more guidence. -#> \ No newline at end of file + +Describe $CommandName -Tag IntegrationTests { + BeforeAll { + # We want to run all commands in the BeforeAll block with EnableException to ensure that the test fails if the setup fails. + $PSDefaultParameterValues["*-Dba*:EnableException"] = $true + + # The file is exported from the same instance it is imported into again, so that no test changes the + # configuration of the lab. + $exportPath = "$($TestConfig.Temp)\$CommandName-$(Get-Random)" + $null = New-Item -Path $exportPath -ItemType Directory + $configFile = Export-DbaSpConfigure -SqlInstance $TestConfig.InstanceSingle -Path $exportPath + + # We want to run all commands outside of the BeforeAll block without EnableException to be able to test for specific warnings. + $PSDefaultParameterValues.Remove("*-Dba*:EnableException") + } + + AfterAll { + # We want to run all commands in the AfterAll block with EnableException to ensure that the test fails if the cleanup fails. + $PSDefaultParameterValues["*-Dba*:EnableException"] = $true + + Remove-Item -Path $exportPath -Recurse -ErrorAction SilentlyContinue + } + + Context "The connection of the caller is left alone when importing from a file (#10554)" { + BeforeAll { + $PSDefaultParameterValues["*-Dba*:EnableException"] = $true + + # Only a non-pooled connection can show this. SMO silently reopens a pooled connection, so the test + # would pass even with the disconnect this is about. + $callerServer = Connect-DbaInstance -SqlInstance $TestConfig.InstanceSingle -NonPooledConnection + $null = $callerServer.ConnectionContext.ExecuteNonQuery("CREATE TABLE #dbatoolsci_marker (id INT)") + + $PSDefaultParameterValues.Remove("*-Dba*:EnableException") + + # The command always warns about a possible restart, so the warning is silenced and asserted below. + $splatImport = @{ + SqlInstance = $callerServer + Path = $configFile.FullName + WarningAction = "SilentlyContinue" + } + $null = Import-DbaSpConfigure @splatImport + } + + AfterAll { + $PSDefaultParameterValues["*-Dba*:EnableException"] = $true + + $null = $callerServer | Disconnect-DbaInstance + } + + It "leaves the connection open" { + $callerServer.ConnectionContext.IsOpen | Should -BeTrue + } + + It "leaves the connection open, so the session survives" { + { $callerServer.ConnectionContext.ExecuteScalar("SELECT COUNT(*) FROM #dbatoolsci_marker") } | Should -Not -Throw + } + + It "warns that a restart may be needed" { + $WarnVar | Should -Match "Some configuration options will be updated once SQL Server is restarted" + } + } + + Context "No pending configuration change is left on the server object of the caller (#10554)" { + BeforeAll { + $PSDefaultParameterValues["*-Dba*:EnableException"] = $true + + # The command used to set show advanced options on the Configuration collection of the caller without + # ever calling Alter(). That only shows while the option is enabled on the instance, because the + # pending value left behind was 0 while the running value was 1, and the next Alter() of the caller + # would have applied it. + $setupServer = Connect-DbaInstance -SqlInstance $TestConfig.InstanceSingle + $originalShowAdvancedOptions = $setupServer.Configuration.ShowAdvancedOptions.ConfigValue + $setupServer.Configuration.ShowAdvancedOptions.ConfigValue = $true + $setupServer.Configuration.Alter($true) + + $advancedServer = Connect-DbaInstance -SqlInstance $TestConfig.InstanceSingle -NonPooledConnection + $showAdvancedOptionsBefore = $advancedServer.Configuration.ShowAdvancedOptions.ConfigValue + + $PSDefaultParameterValues.Remove("*-Dba*:EnableException") + + $splatImportAdvanced = @{ + SqlInstance = $advancedServer + Path = $configFile.FullName + WarningAction = "SilentlyContinue" + } + $null = Import-DbaSpConfigure @splatImportAdvanced + } + + AfterAll { + $PSDefaultParameterValues["*-Dba*:EnableException"] = $true + + $null = $advancedServer | Disconnect-DbaInstance + + $setupServer.Configuration.ShowAdvancedOptions.ConfigValue = $originalShowAdvancedOptions + $setupServer.Configuration.Alter($true) + } + + It "reads the option as enabled before the import" { + $showAdvancedOptionsBefore | Should -Be 1 + } + + It "leaves the option on the server object of the caller as it was" { + $advancedServer.Configuration.ShowAdvancedOptions.ConfigValue | Should -Be $showAdvancedOptionsBefore + } + } + + Context "Both connections of the caller are left alone when copying (#10554)" { + BeforeAll { + $PSDefaultParameterValues["*-Dba*:EnableException"] = $true + + # Source and destination are the same instance on purpose: every value copied is the value that is + # already set, so both connections are exercised without changing the configuration of the lab. + $sourceServer = Connect-DbaInstance -SqlInstance $TestConfig.InstanceSingle -NonPooledConnection + $destinationServer = Connect-DbaInstance -SqlInstance $TestConfig.InstanceSingle -NonPooledConnection + $null = $sourceServer.ConnectionContext.ExecuteNonQuery("CREATE TABLE #dbatoolsci_source_marker (id INT)") + $null = $destinationServer.ConnectionContext.ExecuteNonQuery("CREATE TABLE #dbatoolsci_destination_marker (id INT)") + + $splatCopy = @{ + Source = $sourceServer + Destination = $destinationServer + } + $null = Import-DbaSpConfigure @splatCopy + + $PSDefaultParameterValues.Remove("*-Dba*:EnableException") + } + + AfterAll { + $PSDefaultParameterValues["*-Dba*:EnableException"] = $true + + $null = $sourceServer, $destinationServer | Disconnect-DbaInstance + } + + It "leaves the source connection open" { + $sourceServer.ConnectionContext.IsOpen | Should -BeTrue + } + + It "leaves the destination connection open" { + $destinationServer.ConnectionContext.IsOpen | Should -BeTrue + } + + It "leaves the source connection open, so the session survives" { + { $sourceServer.ConnectionContext.ExecuteScalar("SELECT COUNT(*) FROM #dbatoolsci_source_marker") } | Should -Not -Throw + } + + It "leaves the destination connection open, so the session survives" { + { $destinationServer.ConnectionContext.ExecuteScalar("SELECT COUNT(*) FROM #dbatoolsci_destination_marker") } | Should -Not -Throw + } + } + + Context "The command still closes the connections it opens itself" { + BeforeAll { + $splatCopyByName = @{ + Source = $TestConfig.InstanceSingle + Destination = $TestConfig.InstanceSingle + } + $null = Import-DbaSpConfigure @splatCopyByName + } + + It "does not warn when it connects to the instance by name" { + $WarnVar | Should -BeNullOrEmpty + } + } +} From 668b72b803d2ac0e0bee0ab323edd3b99695ac3b Mon Sep 17 00:00:00 2001 From: Andreas Jordan Date: Fri, 14 Aug 2026 11:15:46 +0200 Subject: [PATCH 2/3] Import-DbaSpConfigure - Make the tests work on SQL Server 2022 and newer Three things the CI run found that the lab instance could not: - The file import warns once per line the edition does not allow to be set, so the warning about the restart is the last one, not the only one. - Every AfterAll has to remove EnableException again. The hashtable is shared by the whole file, so the next context inherited it and the command threw where the test expected a warning. - The copy fails on SQL Server 2022 and newer before it is finished, because the command assigns every property of the destination even when the value does not change and Configuration.Alter() then rejects an option of that edition. That is a separate defect. The connections of the caller have to survive it either way, so the test catches the failure and asserts that. The last context now imports the file by instance name instead of copying by instance name, which is the path where the command owns the connection. (do Import-DbaSpConfigure) Co-Authored-By: Claude Opus 5 (1M context) --- tests/Import-DbaSpConfigure.Tests.ps1 | 47 +++++++++++++++++++-------- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/tests/Import-DbaSpConfigure.Tests.ps1 b/tests/Import-DbaSpConfigure.Tests.ps1 index 24eb08c2a056..e56af2997012 100644 --- a/tests/Import-DbaSpConfigure.Tests.ps1 +++ b/tests/Import-DbaSpConfigure.Tests.ps1 @@ -72,6 +72,8 @@ Describe $CommandName -Tag IntegrationTests { $PSDefaultParameterValues["*-Dba*:EnableException"] = $true $null = $callerServer | Disconnect-DbaInstance + + $PSDefaultParameterValues.Remove("*-Dba*:EnableException") } It "leaves the connection open" { @@ -83,7 +85,9 @@ Describe $CommandName -Tag IntegrationTests { } It "warns that a restart may be needed" { - $WarnVar | Should -Match "Some configuration options will be updated once SQL Server is restarted" + # The warning about the restart is the last one. On instances where the edition does not allow one of + # the options in the file to be set, the command warns about those lines first. + $WarnVar[-1] | Should -Match "Some configuration options will be updated once SQL Server is restarted" } } @@ -120,6 +124,8 @@ Describe $CommandName -Tag IntegrationTests { $setupServer.Configuration.ShowAdvancedOptions.ConfigValue = $originalShowAdvancedOptions $setupServer.Configuration.Alter($true) + + $PSDefaultParameterValues.Remove("*-Dba*:EnableException") } It "reads the option as enabled before the import" { @@ -142,19 +148,31 @@ Describe $CommandName -Tag IntegrationTests { $null = $sourceServer.ConnectionContext.ExecuteNonQuery("CREATE TABLE #dbatoolsci_source_marker (id INT)") $null = $destinationServer.ConnectionContext.ExecuteNonQuery("CREATE TABLE #dbatoolsci_destination_marker (id INT)") + $PSDefaultParameterValues.Remove("*-Dba*:EnableException") + + # On SQL Server 2022 and newer the copy fails before it is finished, for a reason that has nothing to + # do with the connections: the command assigns every property of the destination even when the value + # does not change, and Configuration.Alter() then fails on an option the edition does not allow to be + # set. That is a separate defect, described in the pull request that added this test. What matters + # here is that the connections of the caller survive either way, so the failure is caught. $splatCopy = @{ - Source = $sourceServer - Destination = $destinationServer + Source = $sourceServer + Destination = $destinationServer + WarningAction = "SilentlyContinue" + } + try { + $null = Import-DbaSpConfigure @splatCopy + } catch { + Write-Verbose -Message "Import-DbaSpConfigure failed: $PSItem" } - $null = Import-DbaSpConfigure @splatCopy - - $PSDefaultParameterValues.Remove("*-Dba*:EnableException") } AfterAll { $PSDefaultParameterValues["*-Dba*:EnableException"] = $true $null = $sourceServer, $destinationServer | Disconnect-DbaInstance + + $PSDefaultParameterValues.Remove("*-Dba*:EnableException") } It "leaves the source connection open" { @@ -174,17 +192,20 @@ Describe $CommandName -Tag IntegrationTests { } } - Context "The command still closes the connections it opens itself" { + Context "The command still closes the connection it opens itself" { BeforeAll { - $splatCopyByName = @{ - Source = $TestConfig.InstanceSingle - Destination = $TestConfig.InstanceSingle + # Passing the name instead of a server object is the other side of the guard: the command opens the + # connection here, so it is the one that has to close it again. + $splatImportByName = @{ + SqlInstance = $TestConfig.InstanceSingle + Path = $configFile.FullName + WarningAction = "SilentlyContinue" } - $null = Import-DbaSpConfigure @splatCopyByName + $null = Import-DbaSpConfigure @splatImportByName } - It "does not warn when it connects to the instance by name" { - $WarnVar | Should -BeNullOrEmpty + It "runs to the end and warns that a restart may be needed" { + $WarnVar[-1] | Should -Match "Some configuration options will be updated once SQL Server is restarted" } } } From cc4d56b2be79719d019ca9541379c019c1e148ac Mon Sep 17 00:00:00 2001 From: Andreas Jordan Date: Fri, 14 Aug 2026 11:36:11 +0200 Subject: [PATCH 3/3] Import-DbaSpConfigure - Make the migration between two instances work again The migration was broken on SQL Server 2022 and newer, even when copying an instance onto itself: Alter failed. Changes to server configuration option 'suppress recovery model errors' are not supported in this edition of SQL Server. Four things behind that, all fixed here: - Every option of the destination was assigned, even when the value was the one already set. That marks the property as changed, and Alter() then sends the whole configuration in one batch, which fails as a whole as soon as one option is not supported by the edition. Only options that really differ are touched now, and each one is altered on its own so a failure stays with it. - The Alter that was meant to switch 'show advanced options' on at the destination was run against the source, so the option never got there. - The failure of the batch Alter was swallowed as "needs restart". Whether a restart is needed is now read from IsDynamic of the option that changed, and a failure is reported as the warning it always claimed to be. - 'show advanced options' was left at 0 on both instances instead of being put back the way it was found. It is restored in a finally block, so an option that cannot be set does not leave it switched on either. Also, the two Stop-Function calls of the copy path passed an undefined $server as their target, and the .OUTPUTS block described a boolean the command has never returned. (do Import-DbaSpConfigure) Co-Authored-By: Claude Opus 5 (1M context) --- public/Import-DbaSpConfigure.ps1 | 92 ++++++++++++++++++--------- tests/Import-DbaSpConfigure.Tests.ps1 | 76 ++++++++++++++++------ 2 files changed, 120 insertions(+), 48 deletions(-) diff --git a/public/Import-DbaSpConfigure.ps1 b/public/Import-DbaSpConfigure.ps1 index cdfc192e5af4..d7fe2d6c2863 100644 --- a/public/Import-DbaSpConfigure.ps1 +++ b/public/Import-DbaSpConfigure.ps1 @@ -68,15 +68,11 @@ function Import-DbaSpConfigure { None You cannot pipe objects to Import-DbaSpConfigure .OUTPUTS - System.Boolean + None - Returns $true if the sp_configure settings were successfully applied to the destination instance, or $false if the operation failed. + This command writes no objects to the pipeline. Progress is reported as messages: one per configuration option that was changed, and a final message when the migration is finished. - When using the -ServerCopy parameter set, settings are migrated from the source instance to the destination instance and the command returns a boolean indicating success or failure of the overall migration process. - - When using the -FromFile parameter set, sp_configure settings from a SQL file are executed against the target instance and the command returns a boolean indicating success or failure of the configuration import. - - Note: The function may also display warning messages about configuration options that require SQL Server restart, but these do not affect the boolean return value. + A warning is written when an option could not be set on the destination, and when an option that was changed only takes effect after a restart of SQL Server. .EXAMPLE PS C:\> Import-DbaSpConfigure -Source sqlserver -Destination sqlcluster @@ -139,7 +135,7 @@ function Import-DbaSpConfigure { } if (-not (Test-SqlSa -SqlInstance $sourceserver -SqlCredential $SourceSqlCredential)) { - Stop-Function -Message "Not a sysadmin on $sourceserver. Quitting." -Category PermissionDenied -Target $server -Continue + Stop-Function -Message "Not a sysadmin on $sourceserver. Quitting." -Category PermissionDenied -Target $sourceserver -Continue } try { @@ -155,7 +151,7 @@ function Import-DbaSpConfigure { } if (-not (Test-SqlSa -SqlInstance $destserver -SqlCredential $DestinationSqlCredential)) { - Stop-Function -Message "Not a sysadmin on $destserver. Quitting." -Category PermissionDenied -Target $server -Continue + Stop-Function -Message "Not a sysadmin on $destserver. Quitting." -Category PermissionDenied -Target $destserver -Continue } $source = $sourceserver.DomainInstanceName @@ -197,37 +193,75 @@ function Import-DbaSpConfigure { } If ($Pscmdlet.ShouldProcess($destination, "Execute sp_configure")) { - $sourceserver.Configuration.ShowAdvancedOptions.ConfigValue = $true - $sourceserver.Configuration.Alter($true) - $destserver.Configuration.ShowAdvancedOptions.ConfigValue = $true - $sourceserver.Configuration.Alter($true) + # 'show advanced options' has to be on to read and to set the advanced options. It used to be + # switched on and then off again, which turned it off on instances that had it on. Both instances + # are now put back the way they were, in the finally block below, so that an option that cannot + # be set does not leave them switched on either. + $showAdvancedOptionsNumber = $sourceserver.Configuration.ShowAdvancedOptions.Number + $sourceShowAdvancedOptions = $sourceserver.Configuration.ShowAdvancedOptions.ConfigValue + $destShowAdvancedOptions = $destserver.Configuration.ShowAdvancedOptions.ConfigValue + + if ($sourceShowAdvancedOptions -eq 0) { + $sourceserver.Configuration.ShowAdvancedOptions.ConfigValue = $true + $sourceserver.Configuration.Alter($true) + } + if ($destShowAdvancedOptions -eq 0) { + # This used to alter the source a second time, so the option never reached the destination. + $destserver.Configuration.ShowAdvancedOptions.ConfigValue = $true + $destserver.Configuration.Alter($true) + } + $needsrestart = $false $destprops = $destserver.Configuration.Properties - foreach ($sourceprop in $sourceserver.Configuration.Properties) { - $displayname = $sourceprop.DisplayName + try { + foreach ($sourceprop in $sourceserver.Configuration.Properties) { + $displayname = $sourceprop.DisplayName + + # 'show advanced options' is the means to do the migration, not part of it. + if ($sourceprop.Number -eq $showAdvancedOptionsNumber) { + continue + } + + $destprop = $destprops | Where-Object { $_.Displayname -eq $displayname } + if ($null -eq $destprop) { + continue + } + + # Only options that really differ are touched. Assigning a value marks the property as + # changed even when it is the value that is already set, and Configuration.Alter() then + # sends every option of the instance in one batch, which fails as a whole as soon as one + # of them is not supported by the edition. That made the migration fail on SQL Server + # 2022 and newer even when both instances were already identical. + if ($destprop.ConfigValue -eq $sourceprop.ConfigValue) { + continue + } - $destprop = $destprops | Where-Object { $_.Displayname -eq $displayname } - if ($null -ne $destprop) { try { $destprop.configvalue = $sourceprop.configvalue - $null = $destserver.Query("RECONFIGURE WITH OVERRIDE") + $destserver.Configuration.Alter($true) + if (-not $destprop.IsDynamic) { + $needsrestart = $true + } Write-Message -Level Output -Message "updated $($destprop.displayname) to $($sourceprop.configvalue)." } catch { - Stop-Function -Message "Could not set $($destprop.displayname) to $($sourceprop.configvalue). Feature may not be supported." -ErrorRecord $_ -Continue + # An option that could not be set stays pending and would fail every following + # Alter() together with it, so the pending change is discarded before going on. + $destserver.Configuration.Refresh() + $destprops = $destserver.Configuration.Properties + Stop-Function -Message "Could not set $displayname to $($sourceprop.configvalue). Feature may not be supported." -ErrorRecord $_ -Continue } } + } finally { + if ($destserver.Configuration.ShowAdvancedOptions.ConfigValue -ne $destShowAdvancedOptions) { + $destserver.Configuration.ShowAdvancedOptions.ConfigValue = $destShowAdvancedOptions + $destserver.Configuration.Alter($true) + } + if ($sourceserver.Configuration.ShowAdvancedOptions.ConfigValue -ne $sourceShowAdvancedOptions) { + $sourceserver.Configuration.ShowAdvancedOptions.ConfigValue = $sourceShowAdvancedOptions + $sourceserver.Configuration.Alter($true) + } } - try { - $destserver.Configuration.Alter() - } catch { - $needsrestart = $true - } - - $sourceserver.Configuration.ShowAdvancedOptions.ConfigValue = $false - $sourceserver.Configuration.Alter($true) - $destserver.Configuration.ShowAdvancedOptions.ConfigValue = $false - $destserver.Configuration.Alter($true) if ($needsrestart -eq $true) { Write-Message -Level Warning -Message "Some configuration options will be updated once SQL Server is restarted." diff --git a/tests/Import-DbaSpConfigure.Tests.ps1 b/tests/Import-DbaSpConfigure.Tests.ps1 index e56af2997012..ba4ffc68b5ce 100644 --- a/tests/Import-DbaSpConfigure.Tests.ps1 +++ b/tests/Import-DbaSpConfigure.Tests.ps1 @@ -46,6 +46,8 @@ Describe $CommandName -Tag IntegrationTests { $PSDefaultParameterValues["*-Dba*:EnableException"] = $true Remove-Item -Path $exportPath -Recurse -ErrorAction SilentlyContinue + + $PSDefaultParameterValues.Remove("*-Dba*:EnableException") } Context "The connection of the caller is left alone when importing from a file (#10554)" { @@ -137,34 +139,53 @@ Describe $CommandName -Tag IntegrationTests { } } - Context "Both connections of the caller are left alone when copying (#10554)" { + Context "The copy applies what differs and leaves both callers alone (#10554)" { BeforeAll { $PSDefaultParameterValues["*-Dba*:EnableException"] = $true - # Source and destination are the same instance on purpose: every value copied is the value that is - # already set, so both connections are exercised without changing the configuration of the lab. + $setupServer = Connect-DbaInstance -SqlInstance $TestConfig.InstanceSingle + + # The command has to put this back the way it found it, so the test runs with it switched on. + $originalShowAdvancedOptions = $setupServer.Configuration.ShowAdvancedOptions.ConfigValue + $setupServer.Configuration.ShowAdvancedOptions.ConfigValue = $true + $setupServer.Configuration.Alter($true) + + # Source and destination are the same instance, so nothing but the one option below can change and + # the test does not need a second instance. The two server objects are made to disagree the way two + # instances would: the source is connected while the option still has the value that is to be + # copied, the instance is then changed, and only then is the destination connected. SMO reads the + # configuration once per server object, so each of them keeps the value it saw. + $originalCostThreshold = $setupServer.Configuration.CostThresholdForParallelism.RunValue + $changedCostThreshold = $originalCostThreshold + 7 + $sourceServer = Connect-DbaInstance -SqlInstance $TestConfig.InstanceSingle -NonPooledConnection - $destinationServer = Connect-DbaInstance -SqlInstance $TestConfig.InstanceSingle -NonPooledConnection + $null = $sourceServer.Configuration.Properties.Count $null = $sourceServer.ConnectionContext.ExecuteNonQuery("CREATE TABLE #dbatoolsci_source_marker (id INT)") + + $setupServer.Configuration.CostThresholdForParallelism.ConfigValue = $changedCostThreshold + $setupServer.Configuration.Alter($true) + + $destinationServer = Connect-DbaInstance -SqlInstance $TestConfig.InstanceSingle -NonPooledConnection + $null = $destinationServer.Configuration.Properties.Count $null = $destinationServer.ConnectionContext.ExecuteNonQuery("CREATE TABLE #dbatoolsci_destination_marker (id INT)") $PSDefaultParameterValues.Remove("*-Dba*:EnableException") - # On SQL Server 2022 and newer the copy fails before it is finished, for a reason that has nothing to - # do with the connections: the command assigns every property of the destination even when the value - # does not change, and Configuration.Alter() then fails on an option the edition does not allow to be - # set. That is a separate defect, described in the pull request that added this test. What matters - # here is that the connections of the caller survive either way, so the failure is caught. - $splatCopy = @{ - Source = $sourceServer - Destination = $destinationServer - WarningAction = "SilentlyContinue" - } - try { - $null = Import-DbaSpConfigure @splatCopy - } catch { - Write-Verbose -Message "Import-DbaSpConfigure failed: $PSItem" - } + $null = Import-DbaSpConfigure -Source $sourceServer -Destination $destinationServer + + # Every other command below writes to $WarnVar as well, so it has to be kept here. + $copyWarnings = $WarnVar + + $PSDefaultParameterValues["*-Dba*:EnableException"] = $true + + $configurationQuery = @" +SELECT name, value, value_in_use FROM sys.configurations WHERE name IN ('cost threshold for parallelism', 'show advanced options') +"@ + $configurationAfter = Invoke-DbaQuery -SqlInstance $TestConfig.InstanceSingle -Query $configurationQuery + $costThresholdAfter = ($configurationAfter | Where-Object name -eq "cost threshold for parallelism").value_in_use + $showAdvancedOptionsAfter = ($configurationAfter | Where-Object name -eq "show advanced options").value_in_use + + $PSDefaultParameterValues.Remove("*-Dba*:EnableException") } AfterAll { @@ -172,9 +193,26 @@ Describe $CommandName -Tag IntegrationTests { $null = $sourceServer, $destinationServer | Disconnect-DbaInstance + $setupServer.Configuration.Refresh() + $setupServer.Configuration.CostThresholdForParallelism.ConfigValue = $originalCostThreshold + $setupServer.Configuration.ShowAdvancedOptions.ConfigValue = $originalShowAdvancedOptions + $setupServer.Configuration.Alter($true) + $PSDefaultParameterValues.Remove("*-Dba*:EnableException") } + It "copies the option the two instances disagree about" { + $costThresholdAfter | Should -Be $originalCostThreshold + } + + It "leaves show advanced options the way it found it" { + $showAdvancedOptionsAfter | Should -Be 1 + } + + It "does not warn, because the option that changed takes effect without a restart" { + $copyWarnings | Should -BeNullOrEmpty + } + It "leaves the source connection open" { $sourceServer.ConnectionContext.IsOpen | Should -BeTrue }