Skip to content

Install-DbaMaintenanceSolution: make off switches work and increase test coverage#10172

Merged
potatoqualitee merged 5 commits intodataplat:developmentfrom
ReeceGoding:Ola-Turn-Off-Verify
Feb 19, 2026
Merged

Install-DbaMaintenanceSolution: make off switches work and increase test coverage#10172
potatoqualitee merged 5 commits intodataplat:developmentfrom
ReeceGoding:Ola-Turn-Off-Verify

Conversation

@ReeceGoding
Copy link
Contributor

@ReeceGoding ReeceGoding commented Feb 16, 2026

Type of Change

Purpose

Closes #10133 and every problem I found along the way. In summary:

  • Many parts of Install-DbaMaintenanceSolution can now be turned off (Verify, Checksum, Compress). We previously incorrectly claimed they could be. For example, any attempts to turn off backup verification would fail.
  • Attempts to make a job that simultaneously verifies backups and writes backups to NUL now fail.
  • Prevented adding the ModificationLevel parameter to the log backup job and updated the documentation to remove the false claim that this worked.
  • Increased test coverage. For example, CopyOnly and ModificationLevel were not tested.
  • Added an AfterEach block to the tests, so we know that the Agent Job commands that we generate are valid SQL. This helps to protect against things like typos and extra commas.

Approach

Classic TDD, which isn't something I often get to do as a SQL guy. The first commit in this PR adds a lot of tests and is by far the biggest part of this PR. If you pull only the first commit and run the tests, 8 will fail:

  • Additional backup parameters all enabled.Should NOT add ModificationLevel parameter to LOG backup job
  • Additional backup parameters all disabled.Should have Compress parameter set to N in backup jobs
  • Additional backup parameters all disabled.Should have Verify parameter set to N in backup jobs
  • Additional backup parameters all disabled.Should have CheckSum parameter set to N in backup jobs
  • Additional backup parameters all but Verify disabled.Should have Compress parameter set to N in backup jobs
  • Additional backup parameters all but Verify disabled.Should have CheckSum parameter set to N in backup jobs
  • Backup to Nul with Verify on.Should error out and tell us our mistake
  • Checksum tests when instance defaults to checksum on.Should have CheckSum parameter set to N in backup jobs

By the end of this PR, everything passes.

The scariest part of this PR is my edit to the if ($InstallJobs -and ($ChangeBackupType -or $Compress -or $CopyOnly -or $Verify -or $CheckSum -or $ModificationLevel) guard clause.

Commands to test

Install-DbaMaintenanceSolution. Test it extremely! Trust nothing I've done! I really don't want to break something this important.

Learning

This is my first time getting serious with Pester! It may not be up to standard.

See dataplat#10133 for why we need so many. We need to check:

* That turning off particular settings actually works.
* That Ola's checks for if the instance has checksum on do not break
anything.
* That Verify with backing up to NUL breaks.
* That ModificationLevel is not added to LOG backups.

Additionally added check to make sure that the T-SQL in whatever job
steps we are running tests on is valid.
@potatoqualitee
Copy link
Member

Your first time with Pester looks great 🙌🏼 It LGTM but if anyone would like to give a second approval, pls do.

…sum off at the instance level

(do Install-DbaMaintenanceSolution)
@ReeceGoding
Copy link
Contributor Author

Thanks! I was worried that there was too much boilerplate in my tests.

Just committed a fix to a really minor issue in one of them. One of the bits was the wrong way around.

@potatoqualitee potatoqualitee merged commit b82f01e into dataplat:development Feb 19, 2026
3 checks passed
@potatoqualitee
Copy link
Member

Thank you! Will release today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Install-DbaMaintenanceSolution: verification cannot be turned off, even when backing up to NUL should force us to

2 participants

Comments