Update Pester to 6.0.0#467
Conversation
Change RequiredModules.psd1 from the floating Pester latest pin to the 6.0.0-rc1 prerelease pin. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
WalkthroughThis PR pins Pester to version 6.0.0 in RequiredModules.psd1 and updates unit and integration tests for compatibility: replacing removed ChangesPester 6 Compatibility Migration
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 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 |
Replace the removed Assert-MockCalled with Should -Invoke. Move the data-driven $testCases for the "When description is not as specified" context into BeforeDiscovery, matching every other -ForEach context in the file, so the cases exist at discovery time - Pester v6 throws on a $null or empty -ForEach instead of silently skipping expansion, which surfaced this latent test bug. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Re-run the release-candidate pilot against Pester 6.0.0-rc2, which includes the revert of the Should -HaveCount dictionary-counting change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #467 +/- ##
===================================
Coverage 86% 87%
===================================
Files 21 21
Lines 2083 2122 +39
===================================
+ Hits 1805 1853 +48
+ Misses 278 269 -9 🚀 New features to boost your workflow:
|
Pester 6 throws instead of falling through to the real command when a mock has only -ParameterFilter behaviours and none match. Get-TimeZoneId looks up 'Get-TimeZone', which matches neither the 'Add-Type' nor 'Set-TimeZone' filter, so add a forwarding default Get-Command mock to keep the v5 fall-through. The existing assertions are parameter-filtered, so their counts are unaffected. 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
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@tests/Unit/DSC_SystemProtection.Tests.ps1`:
- Line 138: Update all migrated call-count assertions in
DSC_SystemProtection.Tests.ps1 to use Should -Invoke -Exactly -Times <n> -Scope
It. The affected assertions in the current test block and the other listed
locations should be changed to assert per-It counts only, matching the existing
pattern used in DSC_Computer.Tests.ps1. Use the Should -Invoke calls in this
file to locate and update each assertion so mock counts are not aggregated
across a Context or Describe.
In `@tests/Unit/DSC_SystemRestorePoint.Tests.ps1`:
- Line 116: The migrated Should -Invoke assertions in
DSC_SystemRestorePoint.Tests.ps1 are missing the required call-count qualifiers.
Update the affected test assertions to use Should -Invoke with -Exactly and
-Scope It alongside -Times 1 so the mock verification is scoped to the current
It block and checks for an exact invocation count. Apply this to both migrated
Write-Warning assertions in the test file, matching the pattern already used in
DSC_SystemProtection.Tests.ps1.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bb5c9f84-e5ca-4d3c-9eea-5665772725ef
📒 Files selected for processing (6)
CHANGELOG.mdRequiredModules.psd1tests/Integration/ComputerManagementDsc.Common.Tests.ps1tests/Unit/DSC_Computer.Tests.ps1tests/Unit/DSC_SystemProtection.Tests.ps1tests/Unit/DSC_SystemRestorePoint.Tests.ps1
|
|
||
| $protectionSettings.Ensure | Should -Be 'Absent' | ||
| Assert-MockCalled -CommandName Write-Warning -Times 1 | ||
| Should -Invoke -CommandName Write-Warning -Times 1 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add -Exactly and -Scope It to all Should -Invoke assertions.
Per the test guidelines, call-count assertions must use Should -Invoke -Exactly -Times <n> -Scope It. All migrated Should -Invoke calls in this file are missing both -Exactly and -Scope It. Without -Exactly, -Times 1 asserts "at least 1" rather than "exactly 1". Without -Scope It, mock calls may be counted across the entire Context or Describe instead of just the current It block — particularly impactful in Context 'When configuration is required' (line 351) which contains multiple It blocks. This is also inconsistent with the already-migrated Should -Invoke calls in DSC_Computer.Tests.ps1 (e.g., lines 789–790) which use -Exactly -Times 0 -Scope It.
As per coding guidelines, use Should -Invoke -Exactly -Times <n> -Scope It for call-count assertions, and assert <n> calls inside the It block; do not assert call counts across an entire Describe or Context.
🛡️ Proposed fix for all changed `Should -Invoke` calls
# Line 138
- Should -Invoke -CommandName Write-Warning -Times 1
+ Should -Invoke -CommandName Write-Warning -Exactly -Times 1 -Scope It
# Line 266
- Should -Invoke -CommandName Write-Warning -Times 2
+ Should -Invoke -CommandName Write-Warning -Exactly -Times 2 -Scope It
# Line 360
- Should -Invoke -CommandName Enable-ComputerRestore -Times 1
+ Should -Invoke -CommandName Enable-ComputerRestore -Exactly -Times 1 -Scope It
# Line 372
- Should -Invoke -CommandName Disable-ComputerRestore
+ Should -Invoke -CommandName Disable-ComputerRestore -Exactly -Times 1 -Scope It
# Lines 385-386
- Should -Invoke -CommandName Enable-ComputerRestore -Times 1
- Should -Invoke -CommandName Invoke-VssAdmin -Times 1
+ Should -Invoke -CommandName Enable-ComputerRestore -Exactly -Times 1 -Scope It
+ Should -Invoke -CommandName Invoke-VssAdmin -Exactly -Times 1 -Scope It
# Lines 428-431
- Should -Invoke -CommandName Enable-ComputerRestore -Times 1
- Should -Invoke -CommandName Invoke-VssAdmin -ParameterFilter { $Operation -eq 'Resize' } -Times 2
- Should -Invoke -CommandName Invoke-VssAdmin -ParameterFilter { $Operation -eq 'Delete' } -Times 1
- Should -Invoke -CommandName Write-Warning -Times 1
+ Should -Invoke -CommandName Enable-ComputerRestore -Exactly -Times 1 -Scope It
+ Should -Invoke -CommandName Invoke-VssAdmin -ParameterFilter { $Operation -eq 'Resize' } -Exactly -Times 2 -Scope It
+ Should -Invoke -CommandName Invoke-VssAdmin -ParameterFilter { $Operation -eq 'Delete' } -Exactly -Times 1 -Scope It
+ Should -Invoke -CommandName Write-Warning -Exactly -Times 1 -Scope ItAlso applies to: 266-266, 360-360, 372-372, 385-386, 428-431
🤖 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_SystemProtection.Tests.ps1` at line 138, Update all migrated
call-count assertions in DSC_SystemProtection.Tests.ps1 to use Should -Invoke
-Exactly -Times <n> -Scope It. The affected assertions in the current test block
and the other listed locations should be changed to assert per-It counts only,
matching the existing pattern used in DSC_Computer.Tests.ps1. Use the Should
-Invoke calls in this file to locate and update each assertion so mock counts
are not aggregated across a Context or Describe.
Source: Coding guidelines
|
|
||
| $protectionSettings.Ensure | Should -Be 'Absent' | ||
| Assert-MockCalled -CommandName Write-Warning -Times 1 | ||
| Should -Invoke -CommandName Write-Warning -Times 1 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add -Exactly and -Scope It to all Should -Invoke assertions.
Same issue as DSC_SystemProtection.Tests.ps1: both migrated Should -Invoke calls are missing -Exactly and -Scope It as required by the test guidelines. Without -Exactly, -Times 1 asserts "at least 1" rather than "exactly 1". Without -Scope It, mock calls may be counted across the entire Context instead of just the current It block.
As per coding guidelines, use Should -Invoke -Exactly -Times <n> -Scope It for call-count assertions.
🛡️ Proposed fix for all changed `Should -Invoke` calls
# Line 116
- Should -Invoke -CommandName Write-Warning -Times 1
+ Should -Invoke -CommandName Write-Warning -Exactly -Times 1 -Scope It
# Line 204
- Should -Invoke -CommandName Write-Warning -Times 2
+ Should -Invoke -CommandName Write-Warning -Exactly -Times 2 -Scope ItAlso applies to: 204-204
🤖 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_SystemRestorePoint.Tests.ps1` at line 116, The migrated Should
-Invoke assertions in DSC_SystemRestorePoint.Tests.ps1 are missing the required
call-count qualifiers. Update the affected test assertions to use Should -Invoke
with -Exactly and -Scope It alongside -Times 1 so the mock verification is
scoped to the current It block and checks for an exact invocation count. Apply
this to both migrated Write-Warning assertions in the test file, matching the
pattern already used in DSC_SystemProtection.Tests.ps1.
Source: Coding guidelines
1 participant
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 + integration tests 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