pester / Pester

Pester is the ubiquitous test and mock framework for PowerShell.
https://pester.dev/
Other
3.08k stars 470 forks source link

v5: Discussion #1218

Closed nohwnd closed 3 years ago

nohwnd commented 5 years ago

πŸ™‹β€β™€οΈ

I am raising a few questions in v5 readme and this is the place to start discussion so you don't have to go through all the respective issues and ask there.

rdbartram commented 5 years ago

I like the idea of Add-Dependency to simplify importing scripts, but what about modules? I had some test classes I created for pester testing vmware PowerCLi but I had to mark them as .ps1 and use add-dependency in the beforeall in order that they worked. Previously, i had them as using module statements at the top of the .test.ps1 but that doesn't work anymore.

Could we do something with Add-Dependency so it could import modules/classes?

Where is the code for Add-Dependency? If I use get-command, I see its in the pester.runtime. Where is that?

Finally, I'm abit confused if we should be using Add-Dependency or not because here #1283, you say using beforeall and not add-dependency, but in the readme you say using Add-Dependency. Thanks for the clarification

nohwnd commented 5 years ago

@rdbartram when I wrote Add-Dependency I made it do the same thing as ., but then I realized that I am not using it as such. Instead I am using the Add-Dependency in the same manner as BeforeAll, so the goal of the issue is to use the same syntax so there is one less cmdlet to think about.

I don't know if I am able to do anything about using module, so that is still something I need to research.

All in all the readme is going a bit outdated, as I go and form how it will all work. :)

rdbartram commented 5 years ago

ok. so i'll stick to dot sourcing in the beforeall as in v4. Is a cool idea though, to simplify the dependencies. Could even make it something like frontmatter at the start of the tests.ps1

Stephanevg commented 4 years ago

After writing this, I am not 100% sur if this should be in this thread. Although the -Passthru object format is not mentionned in the readme document, I think it would make sense to discuss it a bit, as the change is pretty heavy compared to the v4 one. If you prefere having me open a issue of it own to track this just let me know.

Hi, I looked at the output of the -Passthru object a bit today, and there is really a very big difference with the v4 one.

Describe "simple test" {
    it 'Should be false' {
        $true | Should be $false
    }

    it 'Should be true' {
        $true | should be $true
    }
}

Currently, executing the following test in v4 will result in this output:

pester v4.X output


TagFilter         : 
ExcludeTagFilter  : 
TestNameFilter    : 
ScriptBlockFilter : 
TotalCount        : 2
PassedCount       : 1
FailedCount       : 1
SkippedCount      : 0
PendingCount      : 0
InconclusiveCount : 0
Time              : 00:00:00.2968309
TestResult        : {@{ErrorRecord=Expected $false, but got $true.; ParameterizedSuiteName=; Describe=simple test; Parameters=System.Collections.Specialized.OrderedDictionary; Passed=False; Show=All; 
                    FailureMessage=Expected $false, but got $true.; Time=00:00:00.0120719; Name=Should be false; Result=Failed; Context=; StackTrace=at <ScriptBlock>, 
                    C:\Users\taavast3\OneDrive\Repo\Projects\OpenSource\pesterPR\Pester\simpletest.ps1: line 3
                    3:         $true | Should be $false}, @{ErrorRecord=; ParameterizedSuiteName=; Describe=simple test; Parameters=System.Collections.Specialized.OrderedDictionary; Passed=True; Show=All; 
                    FailureMessage=; Time=00:00:00.0058684; Name=Should be true; Result=Passed; Context=; StackTrace=}}

pester v5 output


Content              : C:\Users\taavast3\OneDrive\Repo\Projects\OpenSource\pesterPR\Pester\simpletest.ps1
Type                 : File
PSTypename           : ExecutedBlockContainer
OneTimeTestSetup     : 
Focus                : False
DiscoveryDuration    : 00:00:00.2508997
ExecutedAt           : 9/28/2019 9:19:08 AM
EachTestTeardown     : 
Id                   : 
ShouldRun            : True
PluginData           : {}
ScriptBlock          : 
AggregatedPassed     : False
Tests                : {}
FrameworkData        : {PreviouslyGeneratedTests, PreviouslyGeneratedBlocks}
Passed               : True
IsRoot               : True
OwnDuration          : 00:00:00
BlockContainer       : @{Content=C:\Users\taavast3\OneDrive\Repo\Projects\OpenSource\pesterPR\Pester\simpletest.ps1; Type=File}
FrameworkDuration    : 00:00:00.5209437
Root                 : @{OneTimeTestSetup=; First=True; Focus=False; DiscoveryDuration=00:00:00.2508997; ExecutedAt=9/28/2019 9:19:08 AM; EachTestTeardown=; Id=; ShouldRun=True; 
                       PluginData=System.Collections.Hashtable; ScriptBlock=; AggregatedPassed=False; Tests=System.Collections.Generic.List`1[System.Object]; Name=Root; 
                       FrameworkData=System.Collections.Hashtable; Passed=True; IsRoot=True; OwnDuration=00:00:00; BlockContainer=; FrameworkDuration=00:00:00; Root=; OneTimeBlockTeardown=; 
                       Blocks=System.Collections.Generic.List`1[System.Object]; Exclude=False; OneTimeTestTeardown=; Executed=True; OneTimeBlockSetup=; EachBlockSetup=; Path=; EachTestSetup=; 
                       EachBlockTeardown=; Last=True; StandardOutput=; Tag=; Parent=; Duration=00:00:00; ErrorRecord=System.Collections.Generic.List`1[System.Object]}
OneTimeBlockTeardown : 
Blocks               : {@{OneTimeTestSetup=; First=True; Focus=False; DiscoveryDuration=00:00:00.0230244; ExecutedAt=9/28/2019 9:19:09 AM; EachTestTeardown=; Id=0; ShouldRun=True; 
                       PluginData=System.Collections.Hashtable; ScriptBlock=
                           it 'Should be false' {
                               $true | Should be $false
                           }

                           it 'Should be true' {
                               $true | should be $true
                           }
                       ; AggregatedPassed=True; Tests=System.Collections.Generic.List`1[System.Object]; Name=simple test; FrameworkData=System.Collections.Hashtable; Passed=True; IsRoot=False; 
                       OwnDuration=00:00:00.0137001; BlockContainer=; FrameworkDuration=00:00:00.3347260; Root=; OneTimeBlockTeardown=; Blocks=System.Collections.Generic.List`1[System.Object]; Exclude=False; 
                       OneTimeTestTeardown=; Executed=True; OneTimeBlockSetup=; EachBlockSetup=; Path=System.String[]; EachTestSetup=; EachBlockTeardown=; Last=True; StandardOutput=; Tag=System.String[]; 
                       Parent=; Duration=00:00:00.0137001; ErrorRecord=System.Collections.Generic.List`1[System.Management.Automation.ErrorRecord]}}
Exclude              : False
OneTimeTestTeardown  : 
Executed             : True
OneTimeBlockSetup    : 
EachBlockSetup       : 
EachTestSetup        : 
EachBlockTeardown    : 
Duration             : 00:00:00.0137001
ErrorRecord          : {}

I have written quite some automation around the object that -Passthru outputs in v3 / v4 and I was wondering if the object currently returned in v5 would be definitive one?

as much as I love all that extra information we get on the details of every run, I miss some of the information I have been used to through the different versions of pester. Although that going up a major version we allow our selfs to introduce some breaking changes, I am a bit afraid of how much of my automation and CI checks I will need to change.

Also, it seems like that the information contained in the v5 object contains the details of the internal PesterRun.

Discussion

The part that would be really missing I think is the header of the v4 object.

TagFilter         : 
ExcludeTagFilter  : 
TestNameFilter    : 
ScriptBlockFilter : 
TotalCount        : 2
PassedCount       : 1
FailedCount       : 1
SkippedCount      : 0
PendingCount      : 0
InconclusiveCount : 0
Time              : 00:00:00.2968309
TestResult: 

I know that my self (and quite a lot of people I work with) use something like this to see if all tests passed.

$PesterRun = Invoke-Pester .\SimpleTest.ps1 -PassThru

If($PesterRun.FailedCount -ge 1){
  # Query failed pestertests from $PesterRun.TestsResults
}

This might be a naΓ―ve thought, but would it make sense to simply add a property to the existing v4 object, and put the new object of v5 in that one? Something like RunDetails perhaps ?

Implementing the -Passthru as described above, could result in a -Passthruobject that would look like this


TagFilter         : 
ExcludeTagFilter  : 
TestNameFilter    : 
ScriptBlockFilter : 
TotalCount        : 2
PassedCount       : 1
FailedCount       : 1
SkippedCount      : 0
PendingCount      : 0
InconclusiveCount : 0
Time              : 00:00:00.2642866
TestResult        : {@{ErrorRecord=Expected $false, but got $true.; ParameterizedSuiteName=; Describe=simple test; Parameters=System.Collections.Specialized.OrderedDictionary; Passed=False; Show=All; 
                    FailureMessage=Expected $false, but got $true.; Time=00:00:00.0105793; Name=Should be false; Result=Failed; Context=; StackTrace=at <ScriptBlock>, 
                    C:\Users\taavast3\OneDrive\Repo\Projects\OpenSource\pesterPR\Pester\simpletest.ps1: line 3
                    3:         $true | Should be $false}, @{ErrorRecord=; ParameterizedSuiteName=; Describe=simple test; Parameters=System.Collections.Specialized.OrderedDictionary; Passed=True; Show=All; 
                    FailureMessage=; Time=00:00:00.0055245; Name=Should be true; Result=Passed; Context=; StackTrace=}}
RunDetails        : 

                    Content              : C:\Users\taavast3\OneDrive\Repo\Projects\OpenSource\pesterPR\Pester\simpletest.ps1
                    Type                 : File
                    PSTypename           : ExecutedBlockContainer
                    OneTimeTestSetup     : 
                    Focus                : False
                    DiscoveryDuration    : 00:00:00.2698095
                    ExecutedAt           : 9/28/2019 9:25:02 AM
                    EachTestTeardown     : 
                    Id                   : 
                    ShouldRun            : True
                    PluginData           : {}
                    ScriptBlock          : 
                    AggregatedPassed     : False
                    Tests                : {}
                    FrameworkData        : {PreviouslyGeneratedTests, PreviouslyGeneratedBlocks}
                    Passed               : True
                    IsRoot               : True
                    OwnDuration          : 00:00:00
                    BlockContainer       : @{Content=C:\Users\taavast3\OneDrive\Repo\Projects\OpenSource\pesterPR\Pester\simpletest.ps1; Type=File}
                    FrameworkDuration    : 00:00:00.4905946
                    Root                 : @{OneTimeTestSetup=; First=True; Focus=False; DiscoveryDuration=00:00:00.2698095; ExecutedAt=9/28/2019 9:25:02 AM; EachTestTeardown=; Id=; ShouldRun=True; 
                                           PluginData=System.Collections.Hashtable; ScriptBlock=; AggregatedPassed=False; Tests=System.Collections.Generic.List1[System.Object]; Name=Root; 
                                           FrameworkData=System.Collections.Hashtable; Passed=True; IsRoot=True; OwnDuration=00:00:00; BlockContainer=; FrameworkDuration=00:00:00; Root=; OneTimeBlockTeardown=; 
                                           Blocks=System.Collections.Generic.List1[System.Object]; Exclude=False; OneTimeTestTeardown=; Executed=True; OneTimeBlockSetup=; EachBlockSetup=; Path=; 
                    EachTestSetup=; 
                                           EachBlockTeardown=; Last=True; StandardOutput=; Tag=; Parent=; Duration=00:00:00; ErrorRecord=System.Collections.Generic.List1[System.Object]}
                    OneTimeBlockTeardown : 
                    Blocks               : {@{OneTimeTestSetup=; First=True; Focus=False; DiscoveryDuration=00:00:00.0299721; ExecutedAt=9/28/2019 9:25:03 AM; EachTestTeardown=; Id=0; ShouldRun=True; 
                                           PluginData=System.Collections.Hashtable; ScriptBlock=
                                               it 'Should be false' {
                                                   True | Should be False
                                               }

                                               it 'Should be true' {
                                                   True | should be True
                                               }
                                           ; AggregatedPassed=True; Tests=System.Collections.Generic.List1[System.Object]; Name=simple test; FrameworkData=System.Collections.Hashtable; Passed=True; 
                    IsRoot=False; 
                                           OwnDuration=00:00:00.0153029; BlockContainer=; FrameworkDuration=00:00:00.3050340; Root=; OneTimeBlockTeardown=; 
                    Blocks=System.Collections.Generic.List1[System.Object]; Exclude=False; 
                                           OneTimeTestTeardown=; Executed=True; OneTimeBlockSetup=; EachBlockSetup=; Path=System.String[]; EachTestSetup=; EachBlockTeardown=; Last=True; StandardOutput=; 
                    Tag=System.String[]; 
                                           Parent=; Duration=00:00:00.0153029; ErrorRecord=System.Collections.Generic.List1[System.Management.Automation.ErrorRecord]}}
                    Exclude              : False
                    OneTimeTestTeardown  : 
                    Executed             : True
                    OneTimeBlockSetup    : 
                    EachBlockSetup       : 
                    EachTestSetup        : 
                    EachBlockTeardown    : 
                    Duration             : 00:00:00.0153029
                    ErrorRecord          : {}

Like that we won't break code that is strongly based the structure of the v4 object, and still offer new functionality to the Pester end users.

nohwnd commented 4 years ago

There is a function that summarizes the new object to the old one. It does not do everything exactly yet but it should in the future. It’s probably not published right now but it is used internally to get the summary at the end of the run. That is imho the way to go. Just put the adapter function in the pipeline after Invoke-Pester and before your existing build infrastructure and you are good to go.

BoSorensen commented 4 years ago

Could you also mention the breaking change to Should -Throw, it was introduced in #1003 and means that the substring match has been replaced with the -like operator: link to the v4 docs https://pester.dev/docs/usage/assertions for reference.

I guess it means that it is not possible to use Should -Throw that works both for v4 and v5, if you want to do substring matching.

Perhaps #675 could be reconsidered ?

nohwnd commented 4 years ago

@BoSorensen Okay gotcha. I think that #675 or something similar would be useful for migration where you can run your test suite in a mixed mode. It would probably be difficult to merge the test results, but I have a Legacy result converter, so maybe not. πŸ™‚

nohwnd commented 4 years ago

@Stephanevg Coverter to legacy object will be merged shortly. https://github.com/pester/Pester/pull/1472

sdecker commented 4 years ago

Are all the things listed in "Breaking changes" expected to be that way in the final release? It feels like many are "known issues". But some are actual breaking changes. If so could you please separate this section into two?

For example, not an exhaustive list True breaking changes: these seems like they are expected not to work going forward

Really should be known issues: And these seem like known issues expected to be resolved or improved before final release

As it stands it's hard to make a business case of the value of moving to Pester 5 without knowing which are permanent.

nohwnd commented 4 years ago

@sdecker good point! I expect some of the known issues to be solved before 5.0 and some before 5.1, will, take that into consideration as well :)

nohwnd commented 4 years ago

@sdecker done πŸ™‚

dbafromthecold commented 4 years ago

@nohwnd this may be me missing something, but has the -Script parameter for Invoke-Pester been removed?

Just tried updating the module to RC 5 and am getting...

A parameter cannot be found that matches parameter name 'Script'

When running something like...

Invoke-Pester -Script @{Path = "C:\pestertest\pestertest1.ps1"; Parameters = @{Param1 = $Value1;}} -TestName 'MyPesterTest' -PassThru -Show None

I checked the breaking features of the docs and didn't see it mentioned there. The function itself has -Script in the examples but it's not declared.

nohwnd commented 4 years ago

@dbafromthecold you are right the -Script does not exist anymore. It did too much stuff. It is called -Path now and accepts paths. The parametrized script running is not implemented and is documented here. https://github.com/pester/Pester/issues/1485

I added -Script to breaking changes list.

dbafromthecold commented 4 years ago

@nohwnd thanks! will get updating my scripts and testing.

bergmeister commented 4 years ago

@nohwnd I find the guidance to put everything that used to not be controlled into a BeforeAll block confusing because of the following scenario: At the start a module with test helper functions gets imported (not within Pester controlled blocks initially). At first I just moved all the existing test initialization into a BeforeAll block but then inside the Describe block, the imported function is not available any more, therefore I had to move the bit with the module import outside of the BeforeAll block for it to work again. What do you think?

nohwnd commented 4 years ago

@bergmeister yeah that one is confusing. I saw you do that in PSScriptAnalyzer (worked recently a bit more on moving it to v5, not sure if I pushed into my branch). The original readme for alpha2 had guidance around this. And I did not come up with a good pattern for this yet. I experimented with BeforeDiscovery { }, and had Anywhere { } and Add-Dependency {} blocks before. All with different timings when they will run. But the way I ran the tests also changed two times.

Anyway, describing it in an article is probably the best thing to do now, because there are more situations where you might want to put your code outside of pester blocks.

Stephanevg commented 4 years ago

Ok, sorry in advance for the HUGE spam:

Hi,

I wanted to share some feedback regarding the experience of migrating to Pester v5. I took a simple test of PSTHML https://github.com/Stephanevg/PSHTML/blob/master/Tests/link.tests.ps1

And tried to migrate it.

I had basically two things I needed to do:

1) Adding all the 'Arrange' code into the a beforeAll 2) I had to add the parameter names to the IT block

Old version

Describe "Testing link" {

        $Class = "MyClass"
        $Id = "MyID"
        $Style = "Background:green"
        $href = "woop"
        $rel = "author"
        $CustomAtt = @{"MyAttribute1"='MyValue1';"MyAttribute2"="MyValue2"}
        $string = link -href $href -rel $rel  -Attributes $CustomAtt -Style $Style -Class $class -id $id

        if($string -is [array]){
            $string = $String -join ""
        }

        it "Should contain opening and closing tags" {
            $string -match '^<link.*>' | should be $true
            #$string -match '.*</link>$' | should be $true

        }

# More it blocks...
} # End Describe block

Output

PS /Users/stephanevg/Code/PSHTML/Tests> $e = invoke-pester -Path ./link.tests.ps1

Starting test discovery in 1 files.
Discovering tests in /Users/stephanevg/Code/PSHTML/Tests/link.tests.ps1.
Found 5 tests. 64ms
Test discovery finished. 79ms

Running tests from '/Users/stephanevg/Code/PSHTML/Tests/link.tests.ps1'
Context Testing PSHTML
 Describing Testing link
   [-] Should contain opening and closing tags 10ms (2ms|7ms)
    ParameterBindingException: Parameter set cannot be resolved using the specified named parameters. One or more parameters issued cannot be used together or an insufficient number of parameters were provided.
    at <ScriptBlock>, /Users/stephanevg/Code/PSHTML/Tests/link.tests.ps1:35
   [-] Testing common parameters: Class 6ms (2ms|4ms)
    ParameterBindingException: Parameter set cannot be resolved using the specified named parameters. One or more parameters issued cannot be used together or an insufficient number of parameters were provided.
    at <ScriptBlock>, /Users/stephanevg/Code/PSHTML/Tests/link.tests.ps1:42
   [-] Testing common parameters: ID 6ms (2ms|4ms)
    ParameterBindingException: Parameter set cannot be resolved using the specified named parameters. One or more parameters issued cannot be used together or an insufficient number of parameters were provided.
    at <ScriptBlock>, /Users/stephanevg/Code/PSHTML/Tests/link.tests.ps1:45
   [-] Testing common parameters: Style 6ms (3ms|4ms)
    ParameterBindingException: Parameter set cannot be resolved using the specified named parameters. One or more parameters issued cannot be used together or an insufficient number of parameters were provided.
    at <ScriptBlock>, /Users/stephanevg/Code/PSHTML/Tests/link.tests.ps1:48
   [-] Testing Attributes parameters 8ms (4ms|5ms)
    ParameterBindingException: Parameter set cannot be resolved using the specified named parameters. One or more parameters issued cannot be used together or an insufficient number of parameters were provided.
    at <ScriptBlock>, /Users/stephanevg/Code/PSHTML/Tests/link.tests.ps1:56
Tests completed in 176ms
Tests Passed: 0, Failed: 5, Skipped: 0 NotRun: 0

New version Result

Describe "Testing link" {
BeforeAll -Scriptblock {

            $Class = "MyClass"
            $Id = "MyID"
            $Style = "Background:green"
            $href = "woop"
            $rel = "author"
            $CustomAtt = @{"MyAttribute1"='MyValue1';"MyAttribute2"="MyValue2"}
            $string = link -href $href -rel $rel  -Attributes $CustomAtt -Style $Style -Class $class -id $id

            if($string -is [array]){
                $string = $String -join ""
            }
        }

        it -Name "Should contain opening and closing tags" -test {
            $string -match '^<link.*>' | should -be $true
            #$string -match '.*</link>$' | should be $true

        }
# more IT blocks

} # End describe block

It fixed my test.

I wrote a lot (but like, really A LOT!) of tests without specifying the parameter names.

Pester Configuration Object

I LOVE IT!

Up until I have tested, that is really something that could make us think of migrating to v5.

One small suggestion, is maybe to expose the constructors to the end users via Functions (using a 'hybrid' approach). so that the creation of the Configuration is abstracted from the user.

example:


Function New-PesterConfiguration {
[CmdletBinding()]
Param(
  [System.Io.FileInfo]$Path,
  [String[]]$Tag,
  [String[]]$ExcludeTag
  [Switch]$CodeCoverage
)

If($CodeCoverage){
      $ccstate = $true
   }else{
       $ccstate = $False
    }
$configuration = [PesterConfiguration]@{
    Run = @{
        Path = $Path
    }
    Filter = @{
        Tag = $Tag
        ExcludeTag = $ExcludeTag
    }
    Should = @{
        ErrorAction = $PsBoundParameter.ErrorAction
    }

    CodeCoverage = @{
        Enable = $ccstate
    }
}

Return $Configuration

}

Like this the users will be able to benefit from all the guidance a Powershell function can offer when creation a new pester configuration (Comment based help, validate sets etc...).

Logging

I couldn't find how to enable the logging in the readme file. I think it would be great to have the possibility to save the logs to file.

ConvertTo-Pester4Object

Thanks for adding that @nohwnd πŸ‘ I tested it, and it works like a charm.

ConvertTo-NunitReport

it works really well. I like the new -AsString parameter. ExportTo-NunitReport works as I would expect as well.

Note: I couldn't find a ConvertTo-JunitReport. It is probably on it's way, but we do rely on the JunitXML document as we use Gitlab, and it only supports that format. IF there would a new cmdlet ConvertTo-JunitReportto come, I guess a Export-JunitReport should also be implemented. Wouldn't it make sense to Have a generic 'Export-pesterReport' function (or something around these lines) which could persist the following to file:

Internally, that function could either have it's own logic, or simply call the other ConverTo-J/NUnitReport functions.

I am just raising the thought here, simply to have the discussion now, and avoid having to rename the cmdlets once v5 is officially out.

Overall impression of RC1

I really really really like the Configuration Object. That will definitely be something very usefull. Especially for framework authors I think.

I have to admit that it confuses me a little bit that we have the that small pester message 'Amount of failed / passed tests etc...' AND the pester object that is returned at each run.

Saw this discussion -> https://github.com/pester/Pester/issues/1480 I think adding -Passthru back would make it more clear. It is a pattern that people already know, and less scripts will need to be updated.

I have just tested for regular Unit tests. Moving things to BeforeAllblocks seems to be the right migration path. (Also, It would be great if the migration can be smoothed a little bit with some Migration help scripts.)

I do have another test case I would like to takle, and that is for Framekworks that are written around Pester itself. At work we have a framwork that works 'kind of' like DBAChecks.

I do have the impression that these frameworks will benefit a lot of the refactoring of Pester.

When reading through the v5 readme with my "boss's vision" I don't see that much that can appeal us to migration from v4 to v5. Of course, there is logging, the discovery, the new structure of the object, the decoupling of commands etc... But in the end, If I just want to use Pester's basic functionality to test my code (written with v4 syntax) v5 doesn't seem to offer something that will want us to really really migration to it, and go through the hassle of migrating ALL our tests (belive, we have a LOT of tests) to v5.

For now, the Pester v5 migration sounds like a super lot of work for me, without any direct benefits (outside of use the latest version of pester).

I can image that we would end up having a double approach: 1) leave our existing Tests written in v4 as is, and call Invoke-Pester from the v4 module . 2) when writing new tests, we would embrace Pester v5 recommendations (beforeAll etc...) , and use Invoke-Pester from the v5 module.

We would need to know in advance for which pester version a Tests.ps1 file is written. Not sure how to get that one to work.

nohwnd commented 4 years ago

@Stephanevg

I had to add the parameter names to the IT block Why was that necessary? That should not be necessary. Were there errors?

One small suggestion, is maybe to expose the constructors to the end users via Functions (using a 'hybrid' approach). so that the creation of the Configuration is abstracted from the user.

That will come in the future on way or another. Right now each section can be created via a hashtable now, or by creating the whole object using $config = [PesterConfiguration]::Default and then assigning to the different properties. I understand that this is not ideal, but my immediate goal was to expose a simple api that can be used interactively, and then have a complete "api" into which everything else will be translated. So internally I can represent all the configuration via a single object, not matter in which way it is created.

I am planning to add a more intermediate api, which will have more parameters on Invoke-Pester, or possible functions to create the sections of the config. The thing is that I need to think about it a lot to make it usable. And I don't have the bandwidth to do that before 5.0.

That is why I am exposing the "raw" config object because it allows to config everything that is there to configure.

Like this the users will be able to benefit from all the guidance a Powershell function can offer when creation a new pester configuration (Comment based help, validate sets etc...).

Did you see the descriptions on each of the properties? Or that is not discoverable?

ConvertTo-JunitReport.

Is that in v4?

Export-pesterReport

That is what I wanted to avoid. Generic functions that do many things. You are also IMHO unlikely to have both NUnit and JUnit reports in one CI pipeline.

I couldn't find how to enable the logging in the readme file. I think it would be great to have the possibility to save the logs to file.

That would be in the debug section of config. https://github.com/pester/Pester/tree/v5.0#advanced-interface WriteDebugMessages = $true and WriteDebugMessagesFrom = "Filter"

It currently uses Write-Host to write to the screen, so it does not save into a file. And there is a rather huge perf penalty for enabling it. Imho adding -Log param with the basics like "Skip", "Mock", "Discovery", "Filter" would make it way more usable. Again, not enough time to polish all this out :)

I have to admit that it confuses me a little bit that we have the that small pester message 'Amount of failed / passed tests etc...' AND the pester object that is returned at each run.

That is already reverted and in rc2 there is -PassThru again. πŸ™‚

migration can be smoothed a little bit with some Migration help scripts.

There is a script linked in the readme that puts all file setups to BeforeAll, I will try to update it to do the same for the code after Describe but before It

When reading through the v5 readme with my "boss's vision" I don't see that much that can appeal us to migration from v4 to v5.

Give it a try on a new project, and take advantage of stuff like skipping whole blocks based on the platform variables that PowerShell 6 has, or tagging on everything, even child Describes and tests, and mocks that almost never require you to specify the scope.

And then when you go back to v4 you will probably feel annoyed by all of this missing. As I was yesterday 😁

And there is more stuff coming, I have an idea how to make the mocks debuggable, will probably make parallel test running more native and so on. v5 is where new stuff will be happening.

We would need to know in advance for which pester version a Tests.ps1 file is written. Not sure how to get that one to work.

In the config you could define that all your pester5 tests have .Tests5.ps1 extension, there is an option for that in the Run section. That way Pester5 will pick up it's tests, and v4 will pickup the legacy tests.

nohwnd commented 4 years ago

@Stephanevg tagging you one more time, I posted the previous answer too soon. It was not a spam at all, thanks for sharing your experience πŸ‘

bergmeister commented 4 years ago

Around -TestCases, which is now also evaluated at discovery time: I have a test suite here with multiple, long test cases for different Context blocks. The first solution was to declare variables for the test case arrays but for long and multiple test case objects meaning that test and test case are far apart. I then though of inlining the test case array, but that would meaning that the test name and test scriptblock are far apart. The final solution that I came up with is using the named parameters of It:

It -TestCases @(
    @{
        foo = 'bar'
    }
    @{
        foo = 'baz'
    }
) -Name 'ItName' -Test {
    $foo | Should -Match 'ba'
}

or

It 'ItName' -Test {
    $foo | Should -Match 'ba'
} -TestCases @(
    @{
        foo = 'bar'
    }
    @{
        foo = 'baz'
    }
)

I think I slightly prefer the last version. Just posting as seeing this pattern might help others

nohwnd commented 4 years ago

Around -TestCases, which is now also evaluated at discovery time:

Does this have any impact on this? I don't see anything that would actually run in your case, it's just a bunch of strings.

The final solution that I came up with is using the named parameters of It:

That looks okay to me, especially if the test is just a couple of lines long, then you can easily see that testcases are specified below.


If the problem actually was that the evaluation takes long time, then I think -LazySkip and -LazyTestCases which would take scriptblocks could be introduced.

bergmeister commented 4 years ago

Does this have any impact on this?

It was more around finding the most readable version, especially when touching a lot of code now, not performance

On a related note: Is there an easier/lazier way of using TestCases with just an array of strings instead of an array of hashtables if I have a simple test that only takes an array of test values All I could think of is doing something like this

$testValues= 'foo', 'bar', 'baz'
$testCases = $Instances.ForEach{ @{ _ = $_ } }

It would be nice if Pester shipped with something like an array to hashtable converter with the hashtable key being _ so that one can use the intuitive $_ in the test itself (not sure what to do about the angle syntax in the test name itself)

nohwnd commented 4 years ago

It would be nice if Pester shipped with something like an array to hashtableConverter with the hashtable key being so that one can use the intuitive $ in the test itself (not sure what to do about the angle syntax in the test name itself)

And that is why it does not yet ship with such function, because people want to use it in different ways πŸ™‚

nohwnd commented 4 years ago

Anyways I will start cutting Gherkin from Pester 5, and start renaming to __ where necessary.

bergmeister commented 4 years ago

And that is why it does not yet ship with such function, because people want to use it in different ways πŸ™‚

I only partially agree: I agree that people need the freedom to do it in their way but at the moment, if one supplies e.g. an array of strings to the -TestCases parameter, then Pester fails with a parameter binding error that it cannot convert it to an IDictionary. Therefore having at least one default value of using an object array would be useful. Anyone who want to do some custom logic can still use their own array to hashtable converter. This way I don't see how adding support for arrays in -TestCases would break any existing usage or constrain usage.

How about having a -Whatif on Invoke-Pester to run only the test discovery? This would be useful as a simple build validation that there are no syntax errors, etc. without having to run the entire test suite?

nohwnd commented 4 years ago

having at least one default value of using an object array would be useful

You are right. The problem is that consuming arrays of any type is hard to make practical without inventing a lot of arbitrary rules that are hard to come up with, and then hard to define in code. So sticking with the least amount of ways to provide the test cases which still allow you to do everything, even though you sometimes need to adapt array to hashtable is what I do now. We can continue discussing it here https://github.com/pester/Pester/issues/832 there are some nice ideas in that thread. πŸ™‚

How about having a -Whatif on Invoke-Pester to run only the test discovery?

Internally there is Find-Test. And publishing it as a separate cmdlet or as additional parameter on Invoke-Pester is a decision that prevents me from publishing it. I also want to first get the functionality that current Pester has, without adding a lot of new stuff, because the smaller the api is at the start the better.

nohwnd commented 4 years ago

Anyways I will start cutting Gherkin from Pester 5, and start renaming to __ where necessary.

Wrong thread 😁

Stephanevg commented 4 years ago

Hi @nohwnd , I do have a last issue / suggestion / comment regarding Pesterv5, and since it is not in official release yet, I hope we can find a way to inegrate that.

At work I use pester to do Infrastructure validation tests. In (very short) What we do, is we 'dehydrate' (Export-CliXML) the PesterObject, which we rehydrate to do some global parsing afterwards and some other stuff. This is done each time using the -PassThru parameter.

The object in Pester5 is very different than the one from v4. It is a good thing we have ConvertTo-Pester4Result (And btw, was that object introduced in v4 only? Was is not present before v3 ? If so, the name might be missleading then.).

In a scenario where I have already a lot of dehydrated pester 4 objects, and than I export new ones using pester 5, I realize that I need to somehow make a difference between the types of objects as the parsing logic I need to use is different according to the version of pester in which it got generated.

Wouldn't it make sense to add a property at the root of the pesterobject which can help to identify the major version of pester that was used to generate the object?

Adding the same property (with a different value of course) on the v4 object in a version 4.11 for example could help solve this issue, and open the door to a standardisation towards the pester object as such.

Something like PesterObjectVersion or perhaps PesterVersion or anything in that direction would help to make a difference easily and quickly. (I'd be happy to implement it also)

nohwnd commented 4 years ago

Wouldn't it make sense to add a property at the root of the pesterobject which can help to identify the major version of pester that was used to generate the object?

Totally, that's a great idea :) I also need to work on actually make the object serializable. It is serializable now I think, but there are a lot of recursive properties on the unfiltered object. And that will be problematic with clixml (or any other serializer for that matter).

Stephanevg commented 4 years ago

I'll switch to the #1501 to continue the discussion to continue the discussion

inammathe commented 4 years ago

Sorry if this is a dumb question.. but what happened to the -OutputFile parameter of Invoke-Pester?

PS> Get-Help Invoke-pester -Parameter OutputFile
Get-Help: No parameter matches criteria OutputFile.
bergmeister commented 4 years ago

@inammathe When using the -CI, it will set the TestResult.Enabled property of the configuration object to $true and the file name will default to testResults.xml (but can be overriden via the TestResult.OutputPath property on the configuration object). See the v5 readme for more details around the interfaces and the configuration object: https://github.com/pester/Pester/tree/v5.0#simple-and-advanced-interface

nohwnd commented 4 years ago

@inammathe https://github.com/pester/Pester/pull/1578 this PR will add them back together, and will be soon released in 5.0.1 to make the compatibility better for now. They are deprecated, and will be removed at some point later when the intermediate api has a comfortable replacement.

vojtech-kasny commented 4 years ago

Hello all. Has anybody problems with variables not imported to session in 1st run? Pester v5.0.1

Code

BeforeAll {
    # Call module loader
    $LoaderPath = (Resolve-Path -Path (Join-Path $PSScriptRoot -ChildPath "..\..\..\common\ModuleLoader.ps1")).Path
    . $LoaderPath -PSModuleNames @('Vsts.Functions', 'Utility.Functions') -AdditionalFolderNamesWithModules '\Tipsweb\DownloadBuildArtifact\Modules'
    # Module to test
    $ModuleName = 'VstsBuild.Functions'
    # Module commands to test
    $ExportedCommands = Get-Command -Module $ModuleName
    # PSScriptAnalyzer rules setup
    $Severity = 'error' # Error|Warning
    $ScritpAnalyzerRules = Get-ScriptAnalyzerRule -Severity $Severity
    # PSScriptAnalyzer call
    $ScriptAnalyzerResult = Invoke-ScriptAnalyzer -Path (Get-Module $ModuleName).Path -Severity $Severity
}

Describe "Module '$ModuleName' PSScriptAnalyzer Tests" {
    It "Has at least 1 function" {
        $ExportedCommands.Count | Should -BeGreaterThan 0
    }
    foreach ($Rule in $ScritpAnalyzerRules) {
        It "Should pass rule '$Rule' on severity '$Severity'" {
            if ($ScriptAnalyzerResult.RuleName -contains $Rule) {
                ($ScriptAnalyzerResult | Where-Object {$_.RuleName -eq $Rule} -OutVariable Failures).Count | Should -Be 0
            }
        }
    }
    foreach ($cmd in $ExportedCommands) {
        $ErrorCount = $null
        [System.Management.Automation.PSParser]::Tokenize((Get-Command $cmd).Definition, [ref]$ErrorCount) | Out-Null
        It "'$cmd' has valid PowerShell code syntax" {
            $ErrorCount.Count | Should -Be 0
        }
    }
}

Run 1

Invoke-Pester C:\Dev\Repos\toolbox\Build\01_scripts\tipsweb\DownloadBuildArtifact\Tests -Output Detailed

Starting discovery in 1 files.
Discovering in VstsBuild.Functions.Tests.ps1.
Found 1 tests. 475ms
Discovery finished in 927ms.

Running tests from 'VstsBuild.Functions.Tests.ps1'
Describing Module '' PSScriptAnalyzer Tests
  [+] Has at least 1 function 472ms (350ms|122ms)
Tests completed in 3.33s
Tests Passed: 1, Failed: 0, Skipped: 0 NotRun: 0

Run 2

Invoke-Pester C:\Dev\Repos\toolbox\Build\01_scripts\tipsweb\DownloadBuildArtifact\Tests -Output Detailed

Starting discovery in 1 files.
Discovering in VstsBuild.Functions.Tests.ps1.
Found 13 tests. 38ms
Discovery finished in 51ms.

Running tests from 'VstsBuild.Functions.Tests.ps1'
Describing Module 'VstsBuild.Functions' PSScriptAnalyzer Tests
  [+] Has at least 1 function 4ms (2ms|2ms)
  [+] Should pass rule 'PSAvoidUsingUserNameAndPassWordParams' on severity 'error' 190ms (188ms|2ms)
  [+] Should pass rule 'PSAvoidUsingComputerNameHardcoded' on severity 'error' 7ms (5ms|3ms)
  [+] Should pass rule 'PSAvoidUsingConvertToSecureStringWithPlainText' on severity 'error' 11ms (7ms|4ms)
  [+] Should pass rule 'PSDSCUseIdenticalMandatoryParametersForDSC' on severity 'error' 10ms (8ms|2ms)
  [+] Should pass rule 'PSDSCUseIdenticalParametersForDSC' on severity 'error' 5ms (3ms|2ms)
  [+] Should pass rule 'PSDSCStandardDSCFunctionsInResource' on severity 'error' 6ms (4ms|2ms)
  [+] 'Get-VstsBranchBuild' has valid PowerShell code syntax 7ms (6ms|2ms)
  [+] 'Get-VstsSprintBuild' has valid PowerShell code syntax 11ms (8ms|3ms)
  [+] 'Invoke-GetBuildArtifact' has valid PowerShell code syntax 7ms (4ms|2ms)
  [+] 'Save-VstsBuildArtifact' has valid PowerShell code syntax 7ms (5ms|2ms)
  [+] 'Test-VstsBuildArtifact' has valid PowerShell code syntax 6ms (4ms|2ms)
  [+] 'Test-VstsBuildIsRunning' has valid PowerShell code syntax 6ms (4ms|2ms)
Tests completed in 720ms
Tests Passed: 13, Failed: 0, Skipped: 0 NotRun: 0
nohwnd commented 4 years ago

@vojtech-kasny in your example the code in BeforeAll which defines $ScritpAnalyzerRules will run after your foreach loop. BUT because there is a bug https://github.com/pester/Pester/issues/1579 in scoping when running via Invoke-Pester the variable will stay in scope even after the file is executed, and you will use it on the second run.

This will get fixed and your example will stop working.

To make this work every time, you would have to move the code that you need to generate the tests outside of BeforeAll and attach the variables that you need in a test using -TestCases. In your case that would be the iterator variable $Rule, like this It "asdfsafd" -TestCases { Rule = $Rule }` , this will attach the current value of $Rule to the It block, and will later define it automatically, because all Keys from TestCases are defined as variables to avoid manual writing of param block.

vojtech-kasny commented 4 years ago

@nohwnd Thanks Jakub. Adjusted code based on your recommendations and it works

plastikfan commented 4 years ago

I am new to Pester. I have created some plasters (derived from Adentures In Plaster and invoking pester with parameters -Path, -OutputFile and -OutputFormat. The blog article is quite old so I suppose it is probably out of date (finding it really hard to find something that is up to date), but this invocation is causing the following warning to appear:

WARNING: You are using Legacy parameter set that adapts Pester 5 syntax to Pester 4 syntax. This parameter set is deprecated, and does not work 100%. The -Strict and -PesterOption parameters are ignored, and providing advanced configuration to -Path (-Script), and -CodeCoverage via a hash table does not work. Please refer to https://github.com/pester/Pester/releases/tag/5.0.1#legacy-parameter-set for more information.

I am trying to not use a "Legacy" parameter set and the ... Please refer to ... and not sure what parameters should be used for version 5 of Pester.

It looks like I should either be using Simple or Advanced parameter set. I am trying to work my way through this description, but there isn't mapping between v4 parameters and v5.

As I said before I'm trying to figure out what parameters I should replace to be v5 compliant

It's not clear to me how to adapt this call to invoke-pester for v5 compliancy:

Invoke-Pester -Path <String[]> -ExcludePath <String[]> -Tag <String[]> -ExcludeTag <String[]> -Output -CI

The only one that is clear is -Path. I thought that the old -Output was the same as the new, but they are not (how do I now specify the output file?) and how do I specify the -OutputFormat, or do I not need to?

Thanks for any help

EDIT: I'm using a configuration object and this runs without warnings. However, I no longer have a test results xml file, what used to be specified by -OutputFile.

EDIT2: I have discovered that a lot of the old parameters have moved to configuration.TestResult. So, where we used to specify -OutputFormat NUnitxml and -Output 'testResults.xml', we can now populate these on configuration.TestResult. Hope this helps somebody else that might struggle with this as its not yet documented.

nohwnd commented 4 years ago

@plastikfan thanks for the question. The Legacy parameter set was added in hurry, and you are right there is no table. But you can go to the code here, and see the parameter set: https://github.com/pester/Pester/blob/816c964ea85bb39898ca5180c405cdc8beaa41c9/src/Pester.ps1#L259

All of the parameters are in the Legacy parameter set, and most of them define extra aliases. Except for ExcludeTag, that relies on the feature of PowerShell that allows you to specify just enough of the name to not be ambiguous.

The mapping from the parameters to the values on the configuration object is done below in:

https://github.com/pester/Pester/blob/816c964ea85bb39898ca5180c405cdc8beaa41c9/src/Pester.ps1#L420

It would be nice if you could make PR that adds Legacy section into the readme with a table like the one in the Simple and Advanced parameter set πŸ™‚

plastikfan commented 4 years ago

Hi, are there some strange scoping rules that apply to variables declared (or just set as we can't explicitly declare a variable with say a 'var' statement) inside pester Describe/Context/It blocks, which are not particularly intuitive?

Consider this test:

[System.Collections.Hashtable]$DefaultKrayolaTheme = @{
  'FORMAT'             = 'def:"<%KEY%>"="<%VALUE%>"';
  'KEY-PLACE-HOLDER'   = '<%KEY%>';
  'VALUE-PLACE-HOLDER' = '<%VALUE%>';
  'KEY-COLOURS'        = @('DarkCyan');
  'VALUE-COLOURS'      = @('DarkBlue');
  'OPEN'               = 'β€’β€’β€’ (';
  'CLOSE'              = ') β€’β€’β€’';
  'SEPARATOR'          = ' @@ ';
  'META-COLOURS'       = @('Yellow');
  'MESSAGE-COLOURS'    = @('Cyan');
  'MESSAGE-SUFFIX'     = ' ~~ '
}

Describe 'Get-KrayolaTheme' {
  Context 'Dark Theme, "KRAYOLA-THEME-NAME" not defined in Environment' {
    Context 'Given: Theme not specified' {
      it 'Should: return default valid theme' {
        Mock -ModuleName Elizium.Krayola Get-IsKrayolaLightTerminal { return $false }
        Mock -ModuleName Elizium.Krayola Get-EnvironmentVariable { return $null }

        $result = Get-KrayolaTheme -DefaultTheme $DefaultKrayolaTheme
        $result | Should -Not -BeNullOrEmpty
        $result | Should -BeOfType System.Collections.Hashtable
      }
    }
...

This is failing for a strange reason because, on entry into the function under test Get-KrayolaTheme, the argument DefaultTheme is $null even though it has been set to the file scoped DefaultKrayolaTheme.

However, when I change the test, so that DefaultKrayolaTheme is declared inside the It block, then it passes as expected because DefaultTheme is no longer $null and is correctly set to the local variable value.

So what am I missing here? DefaultKrayolaTheme is a global and should be applied to multiple tests, hence that is the reason why I want to set it once at the top of the file for it to be shared.

And setting the value inside of a BeforeAll block does not help, that block's scope doesn't apply to the It scope, so what-ever is set there isn't persistent.

What's happening here isn't intuitive and I can't find anything relevant in the Describe/Context & It documents referenced above.

Thanks for anyone's insight.

plastikfan commented 4 years ago

I have discovered that if I use the Global scope specifier, this works

[System.Collections.Hashtable]$Global:DefaultKrayolaTheme = @{ ...

I have also found that another technique (issue #330) is to use hash tables and populate them inside BeforeEach. However I would prefer to use Global, unless there is a good reason not to and it is much more concise.

This is not a Pester issue as it turns out, it's just one of the many frustrating unintuitive gotchyas of PowerShell.

nohwnd commented 4 years ago

$DefaultKrayolaTheme is defined during discovery and those variables are not guaranteed to be available during run. If you'd put that into a BeforeAll block it would work correctly because Pester would run the BeforeAll block before running It block. BeforeAll is a setup for multiple tests.

plastikfan commented 4 years ago

Thanks @nohwnd, I just tested your theory and can confirm that you're right. I didn't think this would have worked because I previously moved that code into the BeforeAll block, but the script analyser flags this as an unreferenced variable, so I just assumed it wouldn't work, without running it!

Your point about the discovery phase is noted, cheers.

BoSorensen commented 4 years ago

Was there a resolution to the pattern where you move setup code to a BeforeAll block and end up with warnings from PsScriptAnalyzer ?

See https://github.com/PowerShell/PSScriptAnalyzer/issues/711#issuecomment-476532100

plastikfan commented 4 years ago

Hi @BoSorensen, actually, the script analyzer is ok with the variable if you mark it as Global, so in my BeforeAll it's declared as follows:

[System.Collections.Hashtable]$Global:DefaultKrayolaTheme = @{ ...

Previously, I wasnt using $Global, which resulted in the script analyzer warning.

BoSorensen commented 4 years ago

Good tip, but has the drawback of polluting the $Global namespace with these variables.

plastikfan commented 4 years ago

Yeah you're right, but I'm tackling one problem at a time. I will resolve this properly in due course. And actually, I'm not dogmatic about these things, sometimes a global variable is ok, depends how you do it

plastikfan commented 4 years ago

Actually, I just realised, this is only a test, so there is no consequence to using a global here. This is an example I would say, the use of the global is ok.

BoSorensen commented 4 years ago

If you run Invoke-Pester from the PowerShell console you will have a lot of new variables in the $Global namespace, so it has consequences.

plastikfan commented 4 years ago

Ok, you're probably right, but I'm not going to waste time on it. I've got far bigger issues to be dealing with.

BoSorensen commented 4 years ago

Fair enough, the issue is also with PSScriptAnalyzer where the real bugfix should be applied.