Update Pester to 6.0.0#2464
Conversation
WalkthroughPins Pester to version 6.0.0 in RequiredModules.psd1 and updates unit test files across the repository to add forwarding/default mocks for cmdlets like New-Object, Test-Path, Get-PrimaryReplicaServerObject, and Get-RegistryPropertyValue to accommodate Pester 6's no-fallthrough mocking behavior, plus consolidates duplicate cleanup logic and adds a changelog entry. ChangesPester 6 Test Mock Compatibility
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2464 +/- ##
======================================
+ Coverage 91% 94% +2%
======================================
Files 32 227 +195
Lines 5353 10871 +5518
======================================
+ Hits 4904 10235 +5331
- Misses 449 636 +187
🚀 New features to boost your workflow:
|
|
@nohwnd there are some errors in tests that was unexpected (might also be badly written tests): https://dev.azure.com/dsccommunity/SqlServerDsc/_build/results?buildId=13778&view=logs&jobId=693c28b6-fc92-5285-49e3-c3007ddf58e1&j=693c28b6-fc92-5285-49e3-c3007ddf58e1&t=26bd4ad7-0939-5a8f-a28d-c1a967a90564 |
|
Yup. This pipeline is being fixed, was just waiting for release of rc2 which reverts another issue that impacts this repo. |
Update the existing RequiredModules Pester prerelease pin from latest to 6.0.0-rc1. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
….0.0-rc2 Pester 6 no longer falls through to the real command when a mock with a -ParameterFilter does not match, removed Assert-* helpers, and throws on duplicate AfterAll blocks. Update the affected unit tests: - Add forwarding default mocks (-> real cmdlet via Get-Command -CommandType Cmdlet) for Test-Path and New-Object where mock bodies or validation call the cmdlet with arguments not covered by an existing -ParameterFilter (Save-SqlDscSqlServerMediaFile, Invoke-ReportServerSetupAction, DSC_SqlDatabaseObjectPermission, Connect-SQL, DSC_SqlLogin). - Add a $null default mock for Get-RegistryPropertyValue in SqlSetup\Get-TargetResource so registry lookups other than ImagePath resolve to $null as the real function does on a build agent. - Add a default Get-PrimaryReplicaServerObject mock returning the passed ServerObject in SqlAG\Set-TargetResource. - Filter the two New-Object Should -Invoke assertions in DSC_SqlLogin to the SMO Login type so the forwarding default does not change the counts. - Merge a duplicate AfterAll block in Get-SqlDscConfigurationOption. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
nohwnd
changed the title
|
Something shifted, build was green on rc4, and now there is test failure. Leaving this in your hands to fix or close. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
RequiredModules.psd1 (1)
14-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePester pin to 6.0.0 looks correct.
Stale comment: the note above the block says "Add a default Mock for commands where some calls don't match a -ParameterFilter — mocks no longer fall through to the real command" is actually the migration fix applied elsewhere in this PR — the comment at lines 14-17 in this file refers to a different, now-outdated concern ("if preview release of Pester prevents release we should temporary shift back to stable"). Since this is now pinned to the final 6.0.0 release rather than a preview, consider updating or removing this comment.
✏️ Suggested comment update
- <# - If preview release of Pester prevents release we should temporary shift - back to stable. - #> Pester = @{ Version = '6.0.0' }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@RequiredModules.psd1` around lines 14 - 20, The comment above the Pester dependency block is stale and refers to an outdated preview-release fallback that no longer matches the pinned 6.0.0 version. Update or remove the note in RequiredModules.psd1 near the Pester entry so it accurately reflects the current fixed release, keeping the comment aligned with the dependency version and the migration context in this file.tests/Unit/DSC_SqlDatabaseObjectPermission.Tests.ps1 (1)
92-99: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCorrect fix for Pester 6 no-fallthrough behavior.
The forwarding default mock correctly routes unmatched
New-Objectcalls to the real cmdlet viaGet-Command -CommandType Cmdlet, avoiding recursion into the mock itself. Registration order (generic default first, specificParameterFiltermock afterward in the nestedContext) is correct since Pester matches the most-recently-registered matching mock first.This exact snippet is duplicated verbatim in
tests/Unit/DSC_SqlLogin.Tests.ps1(twice) andtests/Unit/SqlServerDsc.Common/Public/Connect-Sql.Tests.ps1. Consider extracting it into a shared helper (e.g. inCommonTestHelper.psm1) to avoid maintaining four copies of the same Pester-6-compatibility workaround.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Unit/DSC_SqlDatabaseObjectPermission.Tests.ps1` around lines 92 - 99, The Pester 6 forwarding mock for New-Object is duplicated across multiple test files, so consolidate this workaround into a shared test helper instead of maintaining several identical copies. Move the generic forwarding mock logic into a reusable helper in CommonTestHelper.psm1 (or an equivalent shared test utility), then call that helper from DSC_SqlDatabaseObjectPermission.Tests.ps1, DSC_SqlLogin.Tests.ps1, and Connect-Sql.Tests.ps1. Keep the existing registration order and behavior intact so the default New-Object fallback still forwards unmatched calls to the real cmdlet without recursion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@RequiredModules.psd1`:
- Around line 14-20: The comment above the Pester dependency block is stale and
refers to an outdated preview-release fallback that no longer matches the pinned
6.0.0 version. Update or remove the note in RequiredModules.psd1 near the Pester
entry so it accurately reflects the current fixed release, keeping the comment
aligned with the dependency version and the migration context in this file.
In `@tests/Unit/DSC_SqlDatabaseObjectPermission.Tests.ps1`:
- Around line 92-99: The Pester 6 forwarding mock for New-Object is duplicated
across multiple test files, so consolidate this workaround into a shared test
helper instead of maintaining several identical copies. Move the generic
forwarding mock logic into a reusable helper in CommonTestHelper.psm1 (or an
equivalent shared test utility), then call that helper from
DSC_SqlDatabaseObjectPermission.Tests.ps1, DSC_SqlLogin.Tests.ps1, and
Connect-Sql.Tests.ps1. Keep the existing registration order and behavior intact
so the default New-Object fallback still forwards unmatched calls to the real
cmdlet without recursion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5c6b084d-f55e-4d85-9bee-62468f3fa455
📒 Files selected for processing (10)
CHANGELOG.mdRequiredModules.psd1tests/Unit/DSC_SqlAG.Tests.ps1tests/Unit/DSC_SqlDatabaseObjectPermission.Tests.ps1tests/Unit/DSC_SqlLogin.Tests.ps1tests/Unit/DSC_SqlSetup.Tests.ps1tests/Unit/Private/Invoke-ReportServerSetupAction.Tests.ps1tests/Unit/Public/Get-SqlDscConfigurationOption.Tests.ps1tests/Unit/Public/Save-SqlDscSqlServerMediaFile.Tests.ps1tests/Unit/SqlServerDsc.Common/Public/Connect-Sql.Tests.ps1
Yeah, there are intermittent error due to resources on build workers (certain times during the day are worse that others). It is normal for me to have to restart the jobs unfortunately. |
johlju
left a comment
johlju
left a comment
There was a problem hiding this comment.
@johlju reviewed 10 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on nohwnd).
2 participants
Pester 6.0.0 is now released 🎉 — thanks for letting me pilot the release candidates against your suite.
This updates the earlier release-candidate pin to the final 6.0.0 and keeps the test changes needed to run on Pester 6 (already in this PR). It's the same migration you'd make yourself, so you're welcome to merge it, cherry-pick just the compatibility changes, or use it purely as a reference.
Your unit + HQRM + full integration matrix all pass on 6.0.0 in this PR. 🟢
The real-world CI signal from this pilot was genuinely valuable in getting 6.0.0 right. Thanks for the CI time this used, and for maintaining a suite I could test against.
This PR was co-authored by GitHub Copilot.
— Jakub & Copilot
This change is