Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(GH-148) Updates Tests to be compatible with Pester 5 #163

Merged
merged 7 commits into from
Jul 7, 2023

Conversation

JPRuskin
Copy link
Member

@JPRuskin JPRuskin commented Oct 25, 2021

Description

These commits update the tests to be compatible with Pester 5, as well as making some small maintenance commits to correct issues raised by the tests, e.g. trailing whitespace, unused parameters, etc. No tests were removed.

Related Issue

Fixes #148

Motivation and Context

This will allow us to use Pester 5 going forward. It should also result in easier addressing of PSSA test failures, as the new method (thanks to Jaykul) gives explicit failures with filepaths and line numbers.

How Has This Been Tested?

Primary testing was performed on a clean docker image running mcr.microsoft.com/powershell, without Chocolatey installed.
Tests were also run on a Windows 10 machine, with Chocolatey installed, and are passing on AppVeyor.
The tests now work with Chocolatey there or not, and other than the xDscResource tests work without elevation. This should be great for future users wanting to add to the tests!

Screenshots (if appropriate):

image

Example of old PSSA failures, taken from another PR:

[-] passes the PSScriptAnalyzer Rule PSShouldProcess 986ms
Expected 0, but got 1.
[-] passes the PSScriptAnalyzer Rule PSShouldProcess 986ms
Expected 0, but got 1.
34: (Invoke-ScriptAnalyzer -Path $module.FullName -IncludeRule $rule.RuleName ).Count | Should Be 0
at , C:\projects\cchoco\tests\cChoco_ScriptAnalyzerTests.ps1: line 3434: (Invoke-ScriptAnalyzer -Path $module.FullName -IncludeRule $rule.RuleName ).Count | Should Be 0
at , C:\projects\cchoco\tests\cChoco_ScriptAnalyzerTests.ps1: line 34

Example of new PSSA failures:
image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • [~] My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • [~] I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Notes: There's no current contributing document that I can see. I also think I've changed the code style of the tests a little, but I think this is in the spirit of the issue.

@JPRuskin JPRuskin force-pushed the 148-Pester5 branch 2 times, most recently from 4a11eec to 0317618 Compare October 25, 2021 15:15
@JPRuskin JPRuskin requested a review from pauby October 25, 2021 15:18
@JPRuskin JPRuskin self-assigned this Oct 25, 2021
Copy link
Member

@corbob corbob left a comment

Choose a reason for hiding this comment

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

These changes look good to me. I have taken them for a test in the Vagrant file I've been finessing for overall testing of cChoco. One caveat is that you either need to install cChoco from the PowerShell Gallery, or you need to put it on the PSModule Path. Otherwise you don't end up with all of the tests running.

JPRuskin added 7 commits July 6, 2023 20:09
Pester considers all files named *.Tests.ps1 to be test files. This is the default naming convention. This commit adjusts cChoco test filenames to match this.

See: https://pester-docs.netlify.app/docs/usage/file-placement-and-naming#common-convention
There were some compatibility issues when running the existing tests in Pester 5, and it was noted that it would be good to update them. This commit does so.

Tested using Pester 5.3.1.
The comment was possible copy-pasted from another file.
The following issues were raised with the updated tests, and needed to be fixed before builds could succeed.

Tests which were mistaken have been overridden at the lowest scope possible. Unused parameters have, in general, been ignored rather than removed to make the smallest amount of change to the actual running code for this.
```
[-] Testing cChoco DSC Resources against PSScriptAnalyzer rule-set.Testing cChocoFeature against default PSScriptAnalyzer rules.Passes 'PSAvoidTrailingWhitespace' 2ms (2ms|0ms)
 Exception: cChocoFeature.psm1:80 Line has trailing whitespace
 cChocoFeature.psm1:134 Line has trailing whitespace
 at <ScriptBlock>, C:\tests\Tests\cChoco_ScriptAnalyzer.Tests.ps1:42
 at <ScriptBlock>, C:\tests\Tests\cChoco_ScriptAnalyzer.Tests.ps1:41
[-] Testing cChoco DSC Resources against PSScriptAnalyzer rule-set.Testing cChocoFeature against default PSScriptAnalyzer rules.Passes 'PSReviewUnusedParameter' 2ms (1ms|0ms)
 Exception: cChocoFeature.psm1:139 The parameter 'FeatureName' has been declared but not used. 
 at <ScriptBlock>, C:\tests\Tests\cChoco_ScriptAnalyzer.Tests.ps1:42
 at <ScriptBlock>, C:\tests\Tests\cChoco_ScriptAnalyzer.Tests.ps1:41
[-] Testing cChoco DSC Resources against PSScriptAnalyzer rule-set.Testing cChocoInstaller against default PSScriptAnalyzer rules.Passes 'PSAvoidGlobalFunctions' 2ms (1ms|0ms)
 Exception: cChocoInstaller.psm1:125 Avoid creating functions with a Global scope.
 at <ScriptBlock>, C:\tests\Tests\cChoco_ScriptAnalyzer.Tests.ps1:42
 at <ScriptBlock>, C:\tests\Tests\cChoco_ScriptAnalyzer.Tests.ps1:41
[-] Testing cChoco DSC Resources against PSScriptAnalyzer rule-set.Testing cChocoInstaller against default PSScriptAnalyzer rules.Passes 'PSReviewUnusedParameter' 2ms (1ms|0ms)
 Exception: cChocoInstaller.psm1:24 The parameter 'InstallDir' has been declared but not used. 
 cChocoInstaller.psm1:72 The parameter 'ChocoInstallScriptUrl' has been declared but not used.
 cChocoInstaller.psm1:131 The parameter 'NoNewLine' has been declared but not used.
 cChocoInstaller.psm1:133 The parameter 'ForegroundColor' has been declared but not used.
 cChocoInstaller.psm1:135 The parameter 'BackgroundColor' has been declared but not used.
 at <ScriptBlock>, C:\tests\Tests\cChoco_ScriptAnalyzer.Tests.ps1:42
 at <ScriptBlock>, C:\tests\Tests\cChoco_ScriptAnalyzer.Tests.ps1:41
[-] Testing cChoco DSC Resources against PSScriptAnalyzer rule-set.Testing cChocoPackageInstall against default PSScriptAnalyzer rules.Passes 'PSAvoidGlobalFunctions' 2ms (1ms|0ms)
 Exception: cChocoPackageInstall.psm1:427 Avoid creating functions with a Global scope.
 at <ScriptBlock>, C:\tests\Tests\cChoco_ScriptAnalyzer.Tests.ps1:42
 at <ScriptBlock>, C:\tests\Tests\cChoco_ScriptAnalyzer.Tests.ps1:41
[-] Testing cChoco DSC Resources against PSScriptAnalyzer rule-set.Testing cChocoPackageInstall against default PSScriptAnalyzer rules.Passes 'PSAvoidTrailingWhitespace' 2ms (1ms|0ms)
 Exception: cChocoPackageInstall.psm1:286 Line has trailing whitespace
 cChocoPackageInstall.psm1:322 Line has trailing whitespace
 cChocoPackageInstall.psm1:347 Line has trailing whitespace
 cChocoPackageInstall.psm1:350 Line has trailing whitespace
 cChocoPackageInstall.psm1:473 Line has trailing whitespace
 at <ScriptBlock>, C:\tests\Tests\cChoco_ScriptAnalyzer.Tests.ps1:42
 at <ScriptBlock>, C:\tests\Tests\cChoco_ScriptAnalyzer.Tests.ps1:41
[-] Testing cChoco DSC Resources against PSScriptAnalyzer rule-set.Testing cChocoPackageInstall against default PSScriptAnalyzer rules.Passes 'PSReviewUnusedParameter' 2ms (1ms|0ms)
 Exception: cChocoPackageInstall.psm1:33 The parameter 'MinimumVersion' has been declared but not used. 
 cChocoPackageInstall.psm1:434 The parameter 'NoNewLine' has been declared but not used.
 cChocoPackageInstall.psm1:436 The parameter 'ForegroundColor' has been declared but not used.
 cChocoPackageInstall.psm1:438 The parameter 'BackgroundColor' has been declared but not used.
 cChocoPackageInstall.psm1:528 The parameter 'NoCache' has been declared but not used.
 at <ScriptBlock>, C:\tests\Tests\cChoco_ScriptAnalyzer.Tests.ps1:42
 at <ScriptBlock>, C:\tests\Tests\cChoco_ScriptAnalyzer.Tests.ps1:41
```
The xDscResources test for admin before running. This ensures Pester doesn't waste time on the file when running it in non-elevated sessions.
Tests were failing on systems without Chocolatey installed. This was embarrassing due to the test for Chocolatey not being installed being the bit that failed. 

This should fix that.
AppVeyor's version was behind. This has been tested.
@corbob
Copy link
Member

corbob commented Jul 7, 2023

Thanks for getting this added @JPRuskin 👍

@JPRuskin JPRuskin deleted the 148-Pester5 branch July 13, 2023 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update to work with Pester 5+
3 participants