Skip to content

Commit 70cf89e

Browse files
andreasjordanclaude
andreasjordan
and
claude
committed
Restore-DbaDatabase - Stop escaping the caller and resurrect the StopAt tests
Two Stop-Function calls in the end block carried -Continue although no loop encloses them. When a restore or its verification failed without -EnableException, the continue unwound out of the command and consumed an iteration of whatever loop the caller was running in - a caller restoring in a foreach silently skipped their next instance, and under Pester it corrupted the test runner, surfacing as "Cannot bind argument to parameter ErrorRecord because it is null". That crash is what kept the StopAt tests undiagnosable since 2020. Both sites now stop and return. The StopAt tests themselves were skipped because their static fixture was broken from its first commit - StopAt_22.trn never made it into the appveyor-lab repo. They now generate their own chain in BeforeAll: two transactions named dbatoolstest carry marks, with a timestamp captured between them, and the assertions follow the layout of the generated steps table instead of magic values. Two traps encoded in comments: STOPATMARK references the transaction name, not the WITH MARK description, and a mark in the last log file recovers the database in the same statement, so no separate -Recover may follow. Verified via the lab harness on SQL03\SQL2019: 81 tests, 77 passed, 0 failed, 3 skipped (the Azure contexts), no warnings. Before the flow-control fix the same file aborted the whole Pester run with the null-ErrorRecord crash. (do Restore-DbaDatabase) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 0473789 commit 70cf89e

2 files changed

Lines changed: 112 additions & 43 deletions

File tree

‎public/Restore-DbaDatabase.ps1‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -883,7 +883,10 @@ function Restore-DbaDatabase {
883883

884884
$null = $FilteredBackupHistory | Test-DbaBackupInformation @parms
885885
} catch {
886-
Stop-Function -ErrorRecord $_ -Message "Failure" -Continue
886+
# No -Continue here: this catch is in the end block outside of any loop, so the continue
887+
# would escape the command and eat an iteration of whatever loop the caller runs in.
888+
Stop-Function -ErrorRecord $_ -Message "Failure"
889+
return
887890
}
888891
if (Test-Bound -ParameterName TestBackupInformation) {
889892
Set-Variable -Name $TestBackupInformation -Value $FilteredBackupHistory -Scope Global
@@ -940,7 +943,12 @@ function Restore-DbaDatabase {
940943
}
941944
$FilteredBackupHistory | Where-Object { $_.IsVerified -eq $true } | Invoke-DbaAdvancedRestore @parms
942945
} catch {
943-
Stop-Function -Message "Failure" -ErrorRecord $_ -Continue -Target $RestoreInstance
946+
# No -Continue here: this catch is in the end block outside of any loop, so the continue
947+
# would escape the command and eat an iteration of whatever loop the caller runs in. Under
948+
# Pester that corrupted the test runner - the failure surfaced as "Cannot bind argument to
949+
# parameter 'ErrorRecord' because it is null" and kept the StopAt tests broken for years.
950+
Stop-Function -Message "Failure" -ErrorRecord $_ -Target $RestoreInstance
951+
return
944952
}
945953
if ($PSCmdlet.ParameterSetName -eq "RestorePage") {
946954
if ($RestoreInstance.Edition -like '*Enterprise*') {

‎tests/Restore-DbaDatabase.Tests.ps1‎

Lines changed: 102 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -873,54 +873,115 @@ use master
873873
}
874874

875875

876-
<#
877-
TODO:
878-
The next tests are skipped because they don't work as expected.
879-
In "$($TestConfig.appveyorlabrepo)\sql2008-backups\StopAt" the backup chain is maybe broken (is file StopAt_22.trn missing?)
880-
Restore-DbaDatabase writes a warning: Microsoft.Data.SqlClient.SqlError: The log in this backup set begins at LSN 19000000021500001, which is too recent to apply to the database. An earlier log backup that includes LSN 19000000020400004 can be restored.
881-
Pester does not like this warning, reason currently unknown. But the context and the complete test fail with "System.Management.Automation.ParameterBindingValidationException: Cannot bind argument to parameter 'ErrorRecord' because it is null".
882-
Maybe it's because the warning is written to $error but has no ErrorRecord.
883-
#>
884-
885-
Context -Skip "Test restoring with StopAt" {
876+
Context "Test restoring with StopMark, StopBefore, StopAfterDate and StopAtLsn" {
886877
BeforeAll {
887-
$null = Get-DbaDatabase -SqlInstance $TestConfig.InstanceSingle -ExcludeSystem -EnableException | Remove-DbaDatabase -EnableException
888-
}
889-
890-
It "Should have stoped at mark" {
891-
$restoreOutput = Restore-DbaDatabase -SqlInstance $TestConfig.InstanceSingle -Name StopAt2 -Path "$($TestConfig.appveyorlabrepo)\sql2008-backups\StopAt" -StopMark dbatoolstest -WarningAction SilentlyContinue -ErrorAction SilentlyContinue -ErrorVariable x
892-
$null = Restore-DbaDatabase -SqlInstance $TestConfig.InstanceSingle -Name StopAt2 -Recover
893-
$sqlOut = Invoke-DbaQuery -SqlInstance $TestConfig.InstanceSingle -Database StopAt2 -Query "select max(step) as ms from steps"
894-
$sqlOut.ms | Should -Be 9876
895-
}
896-
}
897-
898-
899-
Context -Skip "Test restoring with StopAtBefore" {
900-
BeforeAll {
901-
$null = Get-DbaDatabase -SqlInstance $TestConfig.InstanceSingle -ExcludeSystem -EnableException | Remove-DbaDatabase -EnableException
878+
$PSDefaultParameterValues["*-Dba*:EnableException"] = $true
879+
880+
# The old static fixture in sql2008-backups\StopAt was broken from its first commit
881+
# (StopAt_22.trn never made it into the repo), so these tests were skipped for years.
882+
# The chain is generated here instead: two transactions carry the same mark name, with
883+
# a timestamp captured between them so StopAfterDate can select the second one. The
884+
# steps table records how far a restore came: 1 before the first mark, 2 inside it,
885+
# 3 after it, 4 inside the second mark, 5 after that.
886+
$stopMarkDbName = "dbatoolsci_stopmark_$(Get-Random)"
887+
$null = New-DbaDatabase -SqlInstance $TestConfig.InstanceSingle -Name $stopMarkDbName
888+
$splatStopMarkQuery = @{
889+
SqlInstance = $TestConfig.InstanceSingle
890+
Database = $stopMarkDbName
891+
}
892+
Invoke-DbaQuery @splatStopMarkQuery -Query "CREATE TABLE steps (step int NOT NULL)"
893+
$splatStopMarkBackup = @{
894+
SqlInstance = $TestConfig.InstanceSingle
895+
Database = $stopMarkDbName
896+
Path = $backupPath
897+
}
898+
# STOPATMARK references the transaction NAME - the string after WITH MARK is only a
899+
# description - so the transactions themselves have to be named dbatoolstest.
900+
$firstMarkQuery = @"
901+
BEGIN TRAN dbatoolstest WITH MARK 'first dbatools test mark';
902+
INSERT INTO steps VALUES (2);
903+
COMMIT TRAN dbatoolstest
904+
"@
905+
$secondMarkQuery = @"
906+
BEGIN TRAN dbatoolstest WITH MARK 'second dbatools test mark';
907+
INSERT INTO steps VALUES (4);
908+
COMMIT TRAN dbatoolstest
909+
"@
910+
$stopMarkFull = Backup-DbaDatabase @splatStopMarkBackup -Type Full -FilePath "stopmark_full.bak"
911+
Invoke-DbaQuery @splatStopMarkQuery -Query "INSERT INTO steps VALUES (1)"
912+
Invoke-DbaQuery @splatStopMarkQuery -Query $firstMarkQuery
913+
Invoke-DbaQuery @splatStopMarkQuery -Query "INSERT INTO steps VALUES (3)"
914+
$stopMarkLog1 = Backup-DbaDatabase @splatStopMarkBackup -Type Log -FilePath "stopmark_log1.trn"
915+
# STOPATMARK ... AFTER selects the first mark after the given time, so the time has to
916+
# sit strictly between the commits of the two marked transactions.
917+
Start-Sleep -Seconds 2
918+
$betweenMarksTime = Get-Date
919+
Start-Sleep -Seconds 2
920+
Invoke-DbaQuery @splatStopMarkQuery -Query $secondMarkQuery
921+
Invoke-DbaQuery @splatStopMarkQuery -Query "INSERT INTO steps VALUES (5)"
922+
$stopMarkLog2 = Backup-DbaDatabase @splatStopMarkBackup -Type Log -FilePath "stopmark_log2.trn"
923+
$stopMarkBackupFiles = $stopMarkFull.BackupPath, $stopMarkLog1.BackupPath, $stopMarkLog2.BackupPath
924+
925+
$PSDefaultParameterValues.Remove("*-Dba*:EnableException")
902926
}
903927

904-
It "Should have stoped at mark" {
905-
$restoreOutput = Restore-DbaDatabase -SqlInstance $TestConfig.InstanceSingle -Name StopAt2 -Path "$($TestConfig.appveyorlabrepo)\sql2008-backups\StopAt" -StopMark dbatoolstest -StopBefore -WarningAction SilentlyContinue
906-
$null = Restore-DbaDatabase -SqlInstance $TestConfig.InstanceSingle -Name StopAt2 -Recover
907-
$sqlOut = Invoke-DbaQuery -SqlInstance $TestConfig.InstanceSingle -Database StopAt2 -Query "select max(step) as ms from steps"
908-
$sqlOut.ms | Should -Be 8764
909-
}
910-
}
928+
AfterAll {
929+
$PSDefaultParameterValues["*-Dba*:EnableException"] = $true
911930

931+
$null = Get-DbaDatabase -SqlInstance $TestConfig.InstanceSingle -Database $stopMarkDbName | Remove-DbaDatabase
912932

913-
Context -Skip "Test restoring with StopAt, StopAtLsn and StopAfterDate" {
914-
BeforeAll {
915-
$null = Get-DbaDatabase -SqlInstance $TestConfig.InstanceSingle -ExcludeSystem -EnableException | Remove-DbaDatabase -EnableException
933+
$PSDefaultParameterValues.Remove("*-Dba*:EnableException")
916934
}
917935

918-
It "Should have stoped at mark" {
919-
$restoreOutput = Restore-DbaDatabase -SqlInstance $TestConfig.InstanceSingle -Name StopAt2 -Path "$($TestConfig.appveyorlabrepo)\sql2008-backups\StopAt" -StopMark dbatoolstest -StopAfterDate (Get-Date "2020-05-12 13:33:35") -WarningAction SilentlyContinue
920-
$null = Restore-DbaDatabase -SqlInstance $TestConfig.InstanceSingle -Name StopAt2 -Recover
921-
$sqlOut = Invoke-DbaQuery -SqlInstance $TestConfig.InstanceSingle -Database StopAt2 -Query "select max(step) as ms from steps"
922-
$sqlOut.ms | Should -Be 29876
923-
$null = Remove-DbaDatabase -SqlInstance $TestConfig.InstanceSingle -Database StopAt2
936+
It "Should have stopped at the first mark" {
937+
$splatRestore = @{
938+
SqlInstance = $TestConfig.InstanceSingle
939+
Path = $stopMarkBackupFiles
940+
DatabaseName = $stopMarkDbName
941+
WithReplace = $true
942+
StopMark = "dbatoolstest"
943+
WarningAction = "SilentlyContinue"
944+
}
945+
$null = Restore-DbaDatabase @splatRestore
946+
$null = Restore-DbaDatabase -SqlInstance $TestConfig.InstanceSingle -DatabaseName $stopMarkDbName -Recover
947+
$maxStep = Invoke-DbaQuery @splatStopMarkQuery -Query "select max(step) as ms from steps" -As SingleValue
948+
# The marked transaction itself is included, everything after it is not.
949+
$maxStep | Should -Be 2
950+
}
951+
952+
It "Should have stopped before the first mark" {
953+
$splatRestore = @{
954+
SqlInstance = $TestConfig.InstanceSingle
955+
Path = $stopMarkBackupFiles
956+
DatabaseName = $stopMarkDbName
957+
WithReplace = $true
958+
StopMark = "dbatoolstest"
959+
StopBefore = $true
960+
WarningAction = "SilentlyContinue"
961+
}
962+
$null = Restore-DbaDatabase @splatRestore
963+
$null = Restore-DbaDatabase -SqlInstance $TestConfig.InstanceSingle -DatabaseName $stopMarkDbName -Recover
964+
$maxStep = Invoke-DbaQuery @splatStopMarkQuery -Query "select max(step) as ms from steps" -As SingleValue
965+
# The marked transaction itself is excluded this time.
966+
$maxStep | Should -Be 1
967+
}
968+
969+
It "Should have stopped at the second mark with StopAfterDate" {
970+
$splatRestore = @{
971+
SqlInstance = $TestConfig.InstanceSingle
972+
Path = $stopMarkBackupFiles
973+
DatabaseName = $stopMarkDbName
974+
WithReplace = $true
975+
StopMark = "dbatoolstest"
976+
StopAfterDate = $betweenMarksTime
977+
WarningAction = "SilentlyContinue"
978+
}
979+
$null = Restore-DbaDatabase @splatRestore
980+
# No -Recover here: the second mark sits in the last log file, so the restore stops at the
981+
# mark and recovers the database in the same statement.
982+
$maxStep = Invoke-DbaQuery @splatStopMarkQuery -Query "select max(step) as ms from steps" -As SingleValue
983+
# The first mark before the timestamp is skipped, the second marked transaction is included.
984+
$maxStep | Should -Be 4
924985
}
925986

926987
It "Should have stoped at lsn" {

0 commit comments

Comments
 (0)