Install-DbaMaintenanceSolution.Tests: make the additional backup parameters tests run

[ReeceGoding]

Type of Change

• Bug fix (non-breaking change, fixes part of Invoke-ManualPester claims that it supports -Show All #10144)
• New feature (non-breaking change, adds functionality, fixes # )
• Breaking change (affects multiple commands or functionality, fixes # )
• Ran manual Pester test and has passed (Invoke-ManualPester)
• Adding code coverage to existing functionality
• Pester test is included
• If new file reference added for test, has is been added to github.com/dataplat/appveyor-lab ?
• Unit test is included
• Documentation
• Build system

Purpose

As mentioned in passing in #10144, I'm struggling to convince myself that the "Additional backup parameters" in Install-DbaMaintenanceSolution.Tests.ps1 run. I can see warnings in appveyor

Running C:\github\dbatools\tests\Install-DbaMaintenanceSolution.Tests.ps1 ...

[00:38:26][Install-DbaMaintenanceSolution] Could not execute DatabaseIntegrityCheck.sql in tempdb on appveyor-vm\sql2022 | Log entry string is too long. A string written to the event log cannot exceed 32766 characters.

[00:38:26][Install-DbaMaintenanceSolution] Could not execute IndexOptimize.sql in tempdb on appveyor-vm\sql2022 | Log entry string is too long. A string written to the event log cannot exceed 32766 characters.

and Invoke-ManualPester -Path tests/Install-DbaMaintenanceSolution.Tests.ps1 -ScriptAnalyzer -Show Detailed -TestIntegration skipped them on my machine as well.

These tests were introduced in this commit and the PR suggests some difficulty with appveyor at the time. That said, I can believe that this is all just me being confused. Today is the first day that I've had to get serious about Invoke-ManualPester.

Once this has been looked at, I'll be able to approach #10133 with more confidence. #10133 will require me adding more tests to this file, so I would like to be confident in this file's code before I start copy and pasting it.

Approach

I'm really not a Pester expert, but my assessment is that -Skip:(-not $script:installationSucceeded) in the original code runs before $script:installationSucceeded is defined, so these tests are always skipped. Refactoring to use Set-ItResult is, as far as I know, idiomatic Pester.

I also made the tempdb block match the earlier one. All signs are that DROP PROCEDURE does not work with 3-part names.

Commands to test

Run Invoke-ManualPester -Path tests/Install-DbaMaintenanceSolution.Tests.ps1 -Show Diagnostic -TestIntegration on this branch and on the current development branch. If I'm right, only my branch will run this test.

[ReeceGoding]

Tests now pass locally but fail in AppVeyor. Not sure how to fix that yet, but it's progress!

\Tests\appveyor.pester.ps1
...We're in a PR
...Changed files are:
... - tests/Install-DbaMaintenanceSolution.Tests.ps1
PR Detection: Reduced to 1 tests based on changed files
PR Detection: Extended to 2 tests including dependencies
Test Groups : Reduced to 1 out of 713 tests
Test Parts : part 1/2 with 1
appveyor.pester: Running with Pester Version 5.7.1
Get-TestConfig: Setting up test configuration for AppVeyor
Running C:\github\dbatools\tests\Install-DbaMaintenanceSolution.Tests.ps1 ...[05:02:15][Install-DbaMaintenanceSolution] Could not execute DatabaseIntegrityCheck.sql in tempdb on APPVYR-WIN\sql2008r2sp2 | Log entry string is too long. A string written to the event log cannot exceed 32766 characters.
[05:02:15][Install-DbaMaintenanceSolution] Could not execute IndexOptimize.sql in tempdb on APPVYR-WIN\sql2008r2sp2 | Log entry string is too long. A string written to the event log cannot exceed 32766 characters.
Completed C:\github\dbatools\tests\Install-DbaMaintenanceSolution.Tests.ps1 in 19271ms
Uploading artifact TestFailureSummary_Pester5_1.json (699,553 bytes)...100%
appveyor.pester: Test failed with 6 failed tests. Retrying (attempt 2 of 3)...
Running C:\github\dbatools\tests\Install-DbaMaintenanceSolution.Tests.ps1 ...[05:02:53][Install-DbaMaintenanceSolution] Could not execute DatabaseIntegrityCheck.sql in tempdb on APPVYR-WIN\sql2008r2sp2 | Log entry string is too long. A string written to the event log cannot exceed 32766 characters.
[05:02:53][Install-DbaMaintenanceSolution] Could not execute IndexOptimize.sql in tempdb on APPVYR-WIN\sql2008r2sp2 | Log entry string is too long. A string written to the event log cannot exceed 32766 characters.
Completed C:\github\dbatools\tests\Install-DbaMaintenanceSolution.Tests.ps1 in 12336ms
Uploading artifact TestFailureSummary_Pester5_1.json (699,113 bytes)...100%
appveyor.pester: Test failed with 6 failed tests. Retrying (attempt 3 of 3)...
Running C:\github\dbatools\tests\Install-DbaMaintenanceSolution.Tests.ps1 ...[05:03:30][Install-DbaMaintenanceSolution] Could not execute DatabaseIntegrityCheck.sql in tempdb on APPVYR-WIN\sql2008r2sp2 | Log entry string is too long. A string written to the event log cannot exceed 32766 characters.
[05:03:30][Install-DbaMaintenanceSolution] Could not execute IndexOptimize.sql in tempdb on APPVYR-WIN\sql2008r2sp2 | Log entry string is too long. A string written to the event log cannot exceed 32766 characters.
Completed C:\github\dbatools\tests\Install-DbaMaintenanceSolution.Tests.ps1 in 12279ms
Uploading artifact TestFailureSummary_Pester5_1.json (699,533 bytes)...100%
appveyor.pester: Test failed with 6 failed tests. No more retries left.
Uploading artifact OverallTestSummary.json (1,210 bytes)...100%
Dumping message log into c:\github\dbatools\dbatools_messages.xml
Skipping dump of error log into c:\github\dbatools\dbatools_errors.xml
.\Tests\appveyor.pester.ps1 -Finalize
Uploading artifact dbatools_messages_and_errors.xml.zip (23,941 bytes)...100%
Failed tests summary:
Path : Install-DbaMaintenanceSolution/Additional backup parameters/Should add ChangeBackupType parameter to DIFF backup job
Name : It Should add ChangeBackupType parameter to DIFF backup job
Result : Failed
Message : Test failed but no error message could be extracted. Result: Failed | Test Name: Should add ChangeBackupType parameter to DIFF backup job
StackTrace :
RawErrorRecord : No ErrorRecord
Path : Install-DbaMaintenanceSolution/Additional backup parameters/Should add ChangeBackupType parameter to LOG backup job
Name : It Should add ChangeBackupType parameter to LOG backup job
Result : Failed
Message : Test failed but no error message could be extracted. Result: Failed | Test Name: Should add ChangeBackupType parameter to LOG backup job
StackTrace :
RawErrorRecord : No ErrorRecord
Path : Install-DbaMaintenanceSolution/Additional backup parameters/Should add Compress parameter to all backup jobs
Name : It Should add Compress parameter to all backup jobs
Result : Failed
Message : Test failed but no error message could be extracted. Result: Failed | Test Name: Should add Compress parameter to all backup jobs
StackTrace :
RawErrorRecord : No ErrorRecord
Path : Install-DbaMaintenanceSolution/Additional backup parameters/Should have CheckSum parameter set to Y in backup jobs
Name : It Should have CheckSum parameter set to Y in backup jobs
Result : Failed
Message : Test failed but no error message could be extracted. Result: Failed | Test Name: Should have CheckSum parameter set to Y in backup jobs
StackTrace :
RawErrorRecord : No ErrorRecord
Path : Install-DbaMaintenanceSolution/Additional backup parameters/Should have Verify parameter set to Y in backup jobs
Name : It Should have Verify parameter set to Y in backup jobs
Result : Failed
Message : Test failed but no error message could be extracted. Result: Failed | Test Name: Should have Verify parameter set to Y in backup jobs
StackTrace :
RawErrorRecord : No ErrorRecord
Path : Install-DbaMaintenanceSolution/Additional backup parameters/Should NOT add ChangeBackupType parameter to FULL backup job
Name : It Should NOT add ChangeBackupType parameter to FULL backup job
Result : Failed
Message : Test failed but no error message could be extracted. Result: Failed | Test Name: Should NOT add ChangeBackupType parameter to FULL backup job
StackTrace :
RawErrorRecord : No ErrorRecord
Uploading artifact DetailedTestFailures_Pester5.json (5,760 bytes)...100%
6 tests failed.

[ReeceGoding]

Adding -Verbose:$false 2>&1 didn't get AppVeyor to agree with these tests. I'm now out of quick and reasonable ideas.

I'll try some stupid ideas just in case anything works.

[ReeceGoding]

Progress!

It appears that appending *>&5 to every Install-DbaMaintenanceSolution call makes these tests pass in AppVeyor. Does anyone want to tell me why that's a stupid idea? I suspect it must be. I should also find a more reasonable choice than *. Even if this does work, I'll need to make sure that these tests still pass/fail locally and fail in AppVeyor when they should.

[potatoqualitee]

Thank you, @ReeceGoding ! @andreasjordan has made a lot of progress in the tests arena and I will let him approve this as I have not been following along as much.

[andreasjordan]

Root cause might be these messages:

[21:34:43][Install-DbaMaintenanceSolution] Could not execute DatabaseIntegrityCheck.sql in tempdb on appveyor-vm\sql2022 | Log entry string is too long. A string written to the event log cannot exceed 32766 characters.
[21:34:43][Install-DbaMaintenanceSolution] Could not execute IndexOptimize.sql in tempdb on appveyor-vm\sql2022 | Log entry string is too long. A string written to the event log cannot exceed 32766 characters.

We see them just on AppVeyor and I still don't know why.

Let me do some tests in my lab...

[ReeceGoding]

You might want to test on my branch, @andreasjordan . I think that if you use the current dev branch, you'll run into the skipping and tempdb issues I've mentioned.

[andreasjordan]

I think we should try to find the root cause for the the message "Log entry string is too long." Some process inside of dbatools write a lot of stuff to the eventlog. That happens in other tests as well. It does not happen in my lab, so it is hard to test.

The error is thrown here:

                        try {
                            foreach ($query in ($sql -Split "\nGO\b")) {
                                $null = $db.Invoke($query)
                            }
                        } catch {
                            $result = "Failed"
                            Stop-Function -Message "Could not execute $shortFileName in $Database on $instance" -ErrorRecord $_ -Target $db -Continue
                        }

$sql is one of the scripts from Ola Hallengren. So I think ".Invoke" does something special on the AppVeyor platform.

[andreasjordan]

We have another case in another test that calls Get-DbaUserPermission.

There this code failes:

                Write-Message -Level Verbose -Message "Removing STIG schema if it still exists from previous run"
                $tempdb.ExecuteNonQuery($removeStigSQL)
                Write-Message -Level Verbose -Message "Creating STIG schema customized for master database"
                $createStigSQL = $sql.Replace("<TARGETDB>", 'master')
                $tempdb.ExecuteNonQuery($createStigSQL)
                Write-Message -Level Verbose -Message "Building data table for server objects"

If I look at the verbose messages I can see that it fails between the second and third message. As .ExecuteNonQuery works in the first part, i suspect $sql.Replace("<TARGETDB>", 'master') to fail. It looks like we run in a mode that .Replace or -Split in our initial case log the string that is changed to the windows eventlog. Because that call to the windows eventlog failes. So this might be a powershell problem.

[andreasjordan]

Also effected:

Running C:\github\dbatools\tests\Export-DbaInstance.Tests.ps1 ...[13:33:28][Install-DbaWhoIsActive] Failed to install stored procedure. | Log entry string is too long. A string written to the event log cannot exceed 32766 characters.

But there is no string manipulation taking place.

Most likely $null = $server.databases[$Database].ExecuteNonQuery($batch) failes on AppVeyor.

[andreasjordan]

I made some progress!

I changed some code in Install-DbaWhoIsActive in another branch:

From:

foreach ($batch in $batches) {
    try {
        $null = $server.databases[$Database].ExecuteNonQuery($batch)
    } catch {
        Stop-Function -Message "Failed to install stored procedure." -ErrorRecord $_ -Continue -Target $instance
    }
}

To:

foreach ($batch in $batches) {
    try {
        $null = $server.databases[$Database].ExecuteNonQuery($batch)
    } catch {
        # No ErrorRecord with ExecuteNonQuery, to prevent too long eventlog entry on AppVeyor.
        Stop-Function -Message "Failed to install stored procedure using ExecuteNonQuery."
    }
}

And the errror message on AppVeyor changed.

From:

To:

So we have two problems:

• The SQL is failing on AppVeyor and we still don't know why
• The method .ExecuteNonQuery puts all the code in the ErrorRecord and that is catched by AppVeyor and forwarded to the Windows eventlog.

Maybe we should change all catch blocks followed by .ExecuteNonQuery to NOT use the ErrorRecord parameter. But I would need more feedback on that to be sure.

And I would love to know why very big batches fail on the AppVeyor platform and work just fine everywhere else.

[andreasjordan]

I've done a lot of tests but have not managed to get to the root cause.

I hope that using Invoke-DbaQuery for big batches will help. I just opened a new pull request to see if that helps.

[andreasjordan]

I opened #10149 to change the usage from .ExecuteNonQuery() to Invoke-DbaQuery. If that is merged to development, then you can merge development back in this branch and continue testing.

[potatoqualitee]

it is merged 👍🏼

[ReeceGoding]

Thanks all! I've rebased and pushed. The tests continue to work locally with my edits, except for when I deliberately make the skip conditions trigger.

This is now just a matter of waiting on AppVeyor. It will also be interesting to see if new branches unrelated to this one still have the Install-DbaMaintenanceSolution tests fail. Until this is merged, they should.

[andreasjordan]

The prefix script: should not be needed and should be removed everywhere in the code. This is a legacy issue.

[andreasjordan]

Even without removing the script:, this PR is ready to be merged. We can remove the script: later in all of the tests.