Use new Pester syntax: -Parameter for Pester tests in Utility by KevinMarquette · Pull Request #6359 · PowerShell/PowerShell

KevinMarquette · 2018-03-11T05:09:50Z

PR Summary

Use new Pester syntax: -Parameter for Pester tests in Utility. #6245

Because of the number of tests in Utility, this PR only contains the updated Pester syntax. Issues, functional, or test style changes will be handled in separate PRs.

Items identified so far that need issues created:

Get-Random.Tests.ps1: Code Cleanup: Pester tests in Utility/Get-Random.Tests.ps1 #6370
- use of Should -Be (1 -or 2 -or 3 -or 5 -or 8 -or 13) needs to be refactored
- use of Should -Be @secondRandomNumber should be Should -Be @secondRandomNumber and tested
Remove-Variable.Tests.ps1 Code Cleanup: Pester tests in Utility/Remove-Variable.Tests.ps1 need reviewed #6371
- use of Should -Be #nothing. it should be Nothing at all. should be Should -BeNullOrEmpty
Implicit.Remoting.Tests.ps1 Code Cleanup: Pester tests in Utility/Implicit.Remoting.Tests.ps1 need reviewed #6372
- use of Should -Not -Be NullOrEmpty should be Should -Not -BeNullOrEmpty and tested because of functional change

PR Checklist

PR has a meaningful title
- Use the present tense and imperative mood when describing your changes
Summarized changes
Change is not breaking
[NA] Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
This PR is ready to merge and is not Work in Progress.
- If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready.
User-facing changes
- Not Applicable
- OR
- [NA] User-facing Documentation needed
  - [NA] Issue filed - Issue link:
Testing - New and feature
- Not Applicable or can only be tested interactively
- OR
- [NA] Make sure you've added a new test if existing tests do not effectively test the code changed
  - [NA] Add [feature] if the change is significant or affects feature tests

iSazonov · 2018-03-23T13:15:25Z

test/powershell/Modules/Microsoft.PowerShell.Utility/Get-Alias.Tests.ps1

        else
        {
-            ,$val1 | Should BeOfType "System.Array"
+            ,$val1 | Should -BeOfType   "System.Array"


Extra spaces. Below in the file too.

iSazonov · 2018-03-23T13:16:43Z

test/powershell/Modules/Microsoft.PowerShell.Utility/Get-Random.Tests.ps1

-        $result | Should BeOfType $type
+        $result | Should -BeGreaterThan  $greaterThan
+        $result | Should -BeLessThan  $lessThan
+        $result | Should -BeOfType  $type


Extra space in three lines.

iSazonov · 2018-03-23T13:17:14Z

test/powershell/Modules/Microsoft.PowerShell.Utility/Get-Random.Tests.ps1

 Describe "Get-Random" -Tags "CI" {
    It "Should return a random number greater than -1 " {
-	Get-Random | Should BeGreaterThan -1
+	Get-Random | Should -BeGreaterThan  -1


Extra space. Below in the file too.

iSazonov · 2018-03-23T13:17:36Z

test/powershell/Modules/Microsoft.PowerShell.Utility/Get-RunspaceDebug.Tests.ps1

        $rs = Get-RunspaceDebug
        $rs | Should -Not -BeNullOrEmpty
-        $rs[0] | Should BeOfType "Microsoft.PowerShell.Commands.PSRunspaceDebug"
+        $rs[0] | Should -BeOfType     "Microsoft.PowerShell.Commands.PSRunspaceDebug"


Extra space.

iSazonov · 2018-03-23T13:18:06Z

test/powershell/Modules/Microsoft.PowerShell.Utility/New-Object.Tests.ps1

            $comObject = New-Object -ComObject $name
            $comObject.$Property | Should -Not -BeNullOrEmpty
-            $comObject.$Property | should beoftype $Type
+            $comObject.$Property | Should -beoftype  $Type


Extra space and case.

case is corrected

iSazonov · 2018-03-23T13:18:55Z

test/powershell/Modules/Microsoft.PowerShell.Utility/New-TimeSpan.Tests.ps1

     It "Should works proper with new-timespan"{
         $results =  New-TimeSpan -Days 10 -Hours 10 -Minutes 10 -Seconds 10
-         $results | Should BeOfType "System.Timespan"
+         $results | Should -BeOfType  "System.Timespan"


Extra space. Below in the file too.

iSazonov · 2018-03-23T13:19:11Z

test/powershell/Modules/Microsoft.PowerShell.Utility/NewGuid.Tests.ps1

    It "returns a new guid" {
        $guid = New-Guid
-        $guid | Should BeOfType System.Guid
+        $guid | Should -BeOfType     System.Guid


Extra spaces.

iSazonov · 2018-03-23T13:20:16Z

test/powershell/Modules/Microsoft.PowerShell.Utility/Wait-Event.Tests.ps1

-$stopwatch.ElapsedMilliseconds | Should BeGreaterThan 500
-$stopwatch.ElapsedMilliseconds | Should BeLessThan 1500
+$stopwatch.ElapsedMilliseconds | Should -BeGreaterThan  500
+$stopwatch.ElapsedMilliseconds | Should -BeLessThan  1500


Extra spaces in two lines.

…lity

KevinMarquette · 2018-03-23T16:53:37Z

I cleaned up the extra spaces across all files using \| Should -(\w+)\s\s+ to find them all.

SteveL-MSFT · 2018-03-23T19:47:49Z

test/powershell/Modules/Microsoft.PowerShell.Utility/Add-Member.Tests.ps1

        catch
        {
-            $_.FullyQualifiedErrorId | Should Be "ParameterArgumentValidationErrorNullNotAllowed,Microsoft.PowerShell.Commands.AddMemberCommand"
+            $_.FullyQualifiedErrorId | Should -Be "ParameterArgumentValidationErrorNullNotAllowed,Microsoft.PowerShell.Commands.AddMemberCommand"


We should update these tests to use:

{ ...scriptblock... } | Should -Throw -ErrorId "...fully qualified error id..."

SteveL-MSFT · 2018-03-23T19:48:38Z

test/powershell/Modules/Microsoft.PowerShell.Utility/Clear-Variable.Tests.ps1

 		&{
 			Set-Variable foo baz
-			$foo | should be baz
+			$foo | Should -Be baz


For strings, unless we explicitly want case insensitive matching, we should use -BeExactly

SteveL-MSFT · 2018-03-23T19:49:59Z

test/powershell/Modules/Microsoft.PowerShell.Utility/Add-Type.Tests.ps1


-        { [Test.AddType.BasicTest1]::Add1(1, 2) } | Should Throw
-        { [Test.AddType.BasicTest2]::Add2(3, 4) } | Should Throw
+        { [Test.AddType.BasicTest1]::Add1(1, 2) } | Should -Throw


We should never have just Should -Throw, please get the error being thrown and add -ErrorId. We want to prevent the test from passing if it throws for a different reason than what is being tested (like out of memory, for example).

SteveL-MSFT · 2018-03-23T19:51:05Z

I think @kalgiz already took care of these tests with her PR #6366

KevinMarquette · 2018-03-24T18:14:42Z

Sorry about that. I didn't see that other pull request then I did the work and submitted this PR. I reviewed the merge conflicts that now exist and this PR has nothing new to add any more.

iSazonov · 2018-03-24T19:00:52Z

@KevinMarquette Thanks for your contribution!
Sorry, we must explicitly write in the original Issue who grab the job.

KevinMarquette added 19 commits March 10, 2018 17:09

Should -Throw

8ce4229

Should -Not -Throw

88d4b28

Should -BeTrue

faee08a

Should -BeFalse

ee37dab

Should Not BeNullOrEmpty

807cd5c

Should -BeNullOrEmpty

0afa546

Should Not Be $Null -> Should -Not -BeNullOrEmpty

ae8dc08

Should Be $null -> Should -BeNullOrEmpty

fc13f42

Should Match

b51012c

Should -Not -Match

e8d83c8

Should -Not -Be variable

e110fbf

Should -Be variable

f5c5b23

Should Be "" -> Should -BeNullOrEmpty

a423fd9

Should Not Be "" -> Should -Not -BeNullOrEmpty

a2b6b48

Should -Not -<Command>

5f8568b

Should -Throw

2d47f00

Should -Be

034821d

Should -<command>

0de5174

Should -Exist

af92862

KevinMarquette changed the title ~~WIP: Use new Pester syntax: -Parameter for Pester tests in Utility~~ Use new Pester syntax: -Parameter for Pester tests in Utility Mar 13, 2018

KevinMarquette added 2 commits March 19, 2018 22:30

Merge branch Master

aa9ed16

resolve merge conflict

453ae82

iSazonov requested review from JamesWTruher, SteveL-MSFT and adityapatwardhan March 23, 2018 12:46

iSazonov reviewed Mar 23, 2018

View reviewed changes

KevinMarquette added 3 commits March 23, 2018 09:36

Merge remote-tracking branch 'origin/master' into kmarquette/6245-uti…

2edcb2e

…lity

remove extra spaces after pester parameters

e70de93

correct case

b07ebe6

iSazonov approved these changes Mar 23, 2018

View reviewed changes

SteveL-MSFT requested changes Mar 23, 2018

View reviewed changes

KevinMarquette closed this Mar 24, 2018

Conversation

KevinMarquette commented Mar 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iSazonov Mar 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iSazonov Mar 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KevinMarquette commented Mar 23, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SteveL-MSFT commented Mar 23, 2018

Uh oh!

KevinMarquette commented Mar 24, 2018

Uh oh!

iSazonov commented Mar 24, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

KevinMarquette commented Mar 11, 2018 •

edited

Loading

iSazonov Mar 23, 2018 •

edited

Loading

iSazonov Mar 23, 2018 •

edited

Loading