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

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

@@ -93,23 +93,23 @@ Describe "Get-Random DRT Unit Tests" -Tags "CI" {

 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

@@ -7,7 +7,7 @@ Describe "Get-RunspaceDebug" -Tags "CI" {
    It "Should return Microsoft.Powershell.Commands.PSRunspaceDebug as the return type" {
        $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

@@ -200,7 +200,7 @@ try
            param($Name, $Property, $Type)
            $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

@@ -4,7 +4,7 @@ Describe "New-TimeSpan DRT Unit Tests" -Tags "CI" {

     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

@@ -4,7 +4,7 @@ Describe "New-Guid" -Tags "CI" {

    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

@@ -11,7 +11,7 @@ Describe "Add-Member DRT Unit Tests" -Tags "CI" {
        }
        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

@@ -35,7 +35,7 @@ Describe "Clear-Variable DRT Unit Tests" -Tags "CI" {
 		Set-Variable foo bar
 		&{
 			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

    }

    It "Can compile C# files" {

-        { [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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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

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

Uh oh!

Conversation

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!

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!