Refactor and enhance Pester tests batch #9759
Conversation
Updated Pester tests for several dbatools commands to use Pester 5 syntax, added param blocks for test configuration, improved parameter validation, and enhanced test structure and assertions. These changes improve test maintainability, consistency, and reliability across the test suite.
Updated Invoke-DbatoolsFormatter to use custom PSSA settings that preserve manually aligned hashtables and assignment operators. The script now only writes files if formatting changes are detected, reducing unnecessary file writes.
Added checks to skip directories and non-PowerShell files, improved error handling for file read/write operations, and ensured only valid content is processed. These changes enhance robustness and prevent errors when processing invalid or unreadable files.
Enhances the formatter to compare processed, formatted content rather than raw content, ensuring that only meaningful formatting changes trigger file writes. Also applies CBH (Comment-Based Help) fixes and whitespace normalization to both the original and formatted content for accurate comparison.
Simplifies the formatter by removing custom alignment-preserving settings and redundant code. Now uses a placeholder approach to preserve aligned assignments, streamlines file type checks, and only writes files if actual formatting changes are detected. Improves maintainability and reliability of the formatting process.
Enhanced Invoke-DbatoolsFormatter to display progress when formatting multiple files, including status updates for each file, error handling, and a summary of processed and updated files. This improves user feedback during batch operations.
Replaces inconsistent whitespace and blank lines in various test files to improve code readability and maintain consistent formatting. No functional changes were made.
Introduces a new 'Pattern' parameter to Repair-PullRequestTest, allowing users to filter test files by name using wildcards. Updates logic to apply this filter when processing failed tests and enhances verbose output to reflect the applied pattern.
Improved test setup and teardown in Get-DbaCredential.Tests.ps1 by managing EnableException in $PSDefaultParameterValues and refactoring credential creation. In Get-DbaDatabase.Tests.ps1, reorganized and consolidated integration and unit tests, removed redundant code, and improved test clarity and reliability.
Updated Pester test scripts for Get-DbaBuild, Get-DbaDatabase, and Get-DbaDbBackupHistory to use more concise parameter validation, improved variable scoping, and modern PowerShell syntax. Removed redundant code, streamlined test setup/teardown, and enhanced test clarity and maintainability. These changes improve test reliability and readability while aligning with current best practices.
Updated the integration test for Backup-DbaDatabase to include the -IgnoreFileChecks parameter, ensuring the test bypasses file checks when backing up the msdb database.
This reverts commit 9c9416b.
Updated the test to check for at least 3 results and verify that each expected certificate is present, rather than requiring exactly 3 results. This makes the test more robust to additional certificates.
This reverts commit 6bfbfbc.
This reverts commit 79174f2.
|
Backup-DbaDbCertificate couldn't be automatically updated |
There was a problem hiding this comment.
there are a few test files which IMHO use BeforeAll "just because".
I'd prefer to use BeforeAll when it's needed, i.e. in case of multiple Contexts or multiple Its . When there's just one let's aim at avoiding BeforeAll and just use what's needed into the single It
| Context "Gets Feature Usage using -ExcludeDatabase" { | ||
| $results = Get-DbaDbFeatureUsage -SqlInstance $TestConfig.instance2 -ExcludeDatabase $dbname | ||
| BeforeAll { |
There was a problem hiding this comment.
this beforeall is not useful as there's just one test, should be moved into the "It", same thing for the next one
| Context "Ensure array" { | ||
| $results = Get-Command -Name Get-DbaDbFile | Select-Object -ExpandProperty ScriptBlock | ||
| It "returns disks as an array" { | ||
| BeforeAll { |
There was a problem hiding this comment.
multiple contexts here have just one "It" and each with a single "BeforeAll". IMHO BeforeAll should be used when at least there are multiple "It"s
| Context "Should return file information" { | ||
| $result = Get-DbaDbFileGrowth -SqlInstance $TestConfig.instance2 | ||
| BeforeAll { |
There was a problem hiding this comment.
multiple beforeall also here
| Context "Should return file information for a single database" { | ||
| $results = Get-DbaDbFileMapping -SqlInstance $TestConfig.instance1 -Database tempdb | ||
| BeforeAll { |
There was a problem hiding this comment.
mutiple beforeall not needed
|
I'll include that in my next go round of instructions. Pls provide any feedback you like and I'll incorporate |
I don't see any "great general thing" on top of this to be honest, and I reviewed two batches of ~30 files each. I'm assuming that it's the only thing I'm not "liking" of the "automatic rewrite", every other thing is quite sane. |
|
will fix in the next round |
Updated Pester tests for several dbatools commands to use Pester 5 syntax, added param blocks for test configuration, improved parameter validation, and enhanced test structure and assertions. These changes improve test maintainability, consistency, and reliability across the test suite.
Type of Change
Invoke-ManualPester)Purpose
Approach
Commands to test
Screenshots
Learning