Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ jobs:

- name: Run Pester Tests
shell: pwsh
env:
SCRIPTS_ROOT: ${{ github.workspace }}
run: |
$result = Invoke-Pester -Path 'tests/unit/PowerShell' -PassThru
if ($result.FailedCount -gt 0) {
Expand Down
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ repos:
hooks:
- id: psscriptanalyzer
name: psscriptanalyzer
entry: pwsh -Command "Invoke-ScriptAnalyzer -Path PowerShell/ -Recurse -Exclude 'PowerShell/*/testing/*','PowerShell/*/templates/*' -ExcludeRule PSAvoidUsingWriteHost"
entry: pwsh -Command "Invoke-ScriptAnalyzer -Path PowerShell/ -Recurse -ExcludeRule PSAvoidUsingWriteHost"
language: system
types: [powershell]
pass_filenames: false
3 changes: 3 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,9 @@ Before submitting, ensure your tests cover the following scenarios:
- [ ] **Dependencies:** How does the script behave if a required dependency (module, command-line tool) is missing?
- [ ] **Concurrency:** If applicable, is the script safe to run multiple times simultaneously? Does it handle file locking?

**Example:** See `tests/unit/PowerShell/system-maintenance.Tests.ps1` for a comprehensive example that demonstrates testing
for all these scenarios including invalid inputs, edge cases, permissions, dependencies, and error handling.

### Basic Testing

All scripts must be tested with:
Expand Down
126 changes: 126 additions & 0 deletions tests/unit/PowerShell/system-maintenance.Tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,130 @@ Describe "system-maintenance.ps1" {
{ & $localPath -WhatIf } | Should -Not -Throw
}
}

Context "Invalid Inputs" {
It "should reject MaxTempFileAgeDays below minimum (negative values)" {
$localPath = $scriptPath
{ & $localPath -MaxTempFileAgeDays -1 -WhatIf } | Should -Throw
}

It "should reject MaxTempFileAgeDays above maximum (> 3650)" {
$localPath = $scriptPath
{ & $localPath -MaxTempFileAgeDays 9999 -WhatIf } | Should -Throw
}

It "should reject non-numeric MaxTempFileAgeDays" {
$localPath = $scriptPath
{ & $localPath -MaxTempFileAgeDays "invalid" -WhatIf } | Should -Throw
}
}

Context "Edge Cases" {
It "should handle MaxTempFileAgeDays = 0 (delete all temp files)" {
$localPath = $scriptPath
{ & $localPath -MaxTempFileAgeDays 0 -WhatIf } | Should -Not -Throw
}

It "should handle MaxTempFileAgeDays at upper boundary (3650 days)" {
$localPath = $scriptPath
{ & $localPath -MaxTempFileAgeDays 3650 -WhatIf } | Should -Not -Throw
}

It "should handle MaxTempFileAgeDays = 1 (minimum practical value)" {
$localPath = $scriptPath
{ & $localPath -MaxTempFileAgeDays 1 -WhatIf } | Should -Not -Throw
}

It "should handle RunWindowsUpdate switch with WhatIf" {
$localPath = $scriptPath
# WhatIf prevents actual Windows Update operations
{ & $localPath -RunWindowsUpdate -WhatIf } | Should -Not -Throw
}
}

Context "Permissions and Prerequisites" {
It "should have #Requires -RunAsAdministrator directive" {
$content = Get-Content -Path $scriptPath -Raw
$content | Should -Match '#Requires\s+-RunAsAdministrator'
}

# Note: Testing actual permission failures requires running in a non-admin context,
# which cannot be easily tested in a typical test suite that requires admin rights.
# This test verifies the script declares the requirement; runtime enforcement is
# handled by PowerShell itself.
}

Context "Dependencies" {
It "should gracefully handle missing PSWindowsUpdate module when not requested" {
$localPath = $scriptPath
# When RunWindowsUpdate is not specified, the script should not attempt to use the module
{ & $localPath -WhatIf } | Should -Not -Throw
}

# Note: Testing the RunWindowsUpdate path would require either:
# 1. Installing PSWindowsUpdate (which the script does automatically if missing)
# 2. Mocking the module import (complex in Pester 5 for external scripts)
# This demonstrates the dependency is optional and only loaded when needed
}

Context "Parameter Validation" {
It "should accept valid boolean switch parameters" {
$localPath = $scriptPath
# PowerShell automatically converts -RunWindowsUpdate:$false to proper switch handling
{ & $localPath -RunWindowsUpdate:$false -WhatIf } | Should -Not -Throw
}

It "should use default value when MaxTempFileAgeDays not specified" {
# This is validated by the smoke test - default is 7 days
$command = Get-Command -Name $scriptPath
$command.Parameters['MaxTempFileAgeDays'].Attributes.Where({$_ -is [System.Management.Automation.ParameterAttribute]}).Count | Should -BeGreaterThan 0
}
}

Context "WhatIf Support (Confirming Non-Destructive Preview)" {
It "should support -WhatIf for all destructive operations" {
$localPath = $scriptPath
# WhatIf should prevent any actual changes from being made
{ & $localPath -MaxTempFileAgeDays 0 -RunWindowsUpdate -WhatIf } | Should -Not -Throw
}

It "should have ConfirmImpact set appropriately" {
$command = Get-Command -Name $scriptPath
$cmdletBinding = $command.ScriptBlock.Attributes | Where-Object { $_ -is [System.Management.Automation.CmdletBindingAttribute] }
$cmdletBinding.ConfirmImpact | Should -Not -BeNullOrEmpty
}
}

Context "Logging and Output" {
It "should create log file path using Get-LogFilePath function" {
$content = Get-Content -Path $scriptPath -Raw
$content | Should -Match 'function Get-LogFilePath'
}

It "should handle environment where MyDocuments is not available" {
# The script has fallback logic for when MyDocuments is null or empty
# This is tested by examining the Get-LogFilePath function logic
$content = Get-Content -Path $scriptPath -Raw
$content | Should -Match 'IsNullOrWhiteSpace.*userDocs'
$content | Should -Match 'GetTempPath\(\)'
}
}

Context "Error Handling" {
It "should use StrictMode" {
$content = Get-Content -Path $scriptPath -Raw
$content | Should -Match 'Set-StrictMode\s+-Version\s+Latest'
}

It "should set ErrorActionPreference appropriately" {
$content = Get-Content -Path $scriptPath -Raw
$content | Should -Match '\$ErrorActionPreference\s*=\s*[''"]Stop[''"]'
}

It "should include try-catch blocks for error handling" {
$content = Get-Content -Path $scriptPath -Raw
# Check that the script uses try-catch for error handling
($content -split 'try\s*\{').Count | Should -BeGreaterThan 5
}
Comment on lines +185 to +189
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is fragile because it relies on counting string splits of 'try\s*{' patterns. The magic number "5" is unexplained and could easily break with script refactoring that doesn't meaningfully change error handling behavior. For example, adding or removing a single try-catch block would fail this test even if error handling remains adequate.

Consider either:

  1. Removing this test entirely since the presence of try-catch blocks doesn't guarantee proper error handling
  2. Testing actual error handling behavior by invoking the script in error scenarios
  3. If testing for try-catch presence is important, document why "5" is the threshold and what it represents
Suggested change
It "should include try-catch blocks for error handling" {
$content = Get-Content -Path $scriptPath -Raw
# Check that the script uses try-catch for error handling
($content -split 'try\s*\{').Count | Should -BeGreaterThan 5
}

Copilot uses AI. Check for mistakes.
}
}