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

Extend and export New-PesterConfiguration #1728

Merged
merged 5 commits into from
Apr 18, 2021

Conversation

fflaten
Copy link
Collaborator

@fflaten fflaten commented Oct 23, 2020

1. General summary of the pull request

Extends and exports a wrapper-function for creating [PesterConfiguration] objects used to run Invoke-Pester with advanced configurations. Binary classes like this aren't available until a module is imported, which may cause errors for users who depend on auto-loading as creating the configuration object is normally the first step before any call to Invoke-Pester or any other public functions.

Using this wrapper, a user will always be able to create a configuration-object as the first action (which triggers the auto-loading of Pester). The function is kept simple and has few parameters to avoid confusion for the user as the [PesterConfiguration] is intuitive, self-explanatory and might change over time.

Fix #1674

  • Extend and export existing New-PesterConfiguration
  • Add documentation to function
  • Add tests?
  • Update other documentation?

Replaces #1683

@fflaten
Copy link
Collaborator Author

fflaten commented Oct 23, 2020

New attempt 😅
Still not sure about the best parameter sets for this, see #1683 (comment), so current suggestion is to add this for now to solve the issue and extend later. Adding a future parameter set shouldn't cause a breaking change as the current sets feel natural to support and New-PesterConfiguration <no parameters> return a default configuration as expected.

@nohwnd
Copy link
Member

nohwnd commented Oct 24, 2020

Will get back to this once I clean up some more stuff in the 5.1 milestone. I am trying to get it to releasable state soon.

@nohwnd nohwnd added this to the 5.2 milestone Apr 17, 2021
src/Pester.RSpec.ps1 Outdated Show resolved Hide resolved
src/Pester.RSpec.ps1 Outdated Show resolved Hide resolved
@nohwnd
Copy link
Member

nohwnd commented Apr 17, 2021

This is what I envision the api might look like in the future, but I just want to avoid conflicts for now, I am open to feedback of course:

New-PesterConfiguration `
    -Run (New-PesterRunConfiguration -Path $PSScriptRoot -PassThru) `
    -CodeCoverage (New-PesterCodeCoverageConfiguration -Path $PSScriptRoot -OutputFormat "CoverageGutters") `
    -HashTable @{} # some settings to be used as base, and amended by the settings above
    -Default:$false # (maybe in future) do not inherit settings from profile?
    -Configuration # (maybe in future) path to pester configuration in a file?


# maybe some templates as well? But how would this handle multiple templates, e.g. CI with code coverage, 
# and are there other useful templates choices? 
New-PesterConfiguration -Template CI

@fflaten
Copy link
Collaborator Author

fflaten commented Apr 17, 2021

This is what I envision the api might look like in the future, but I just want to avoid conflicts for now, I am open to feedback of course:

New-PesterConfiguration `
    -Run (New-PesterRunConfiguration -Path $PSScriptRoot -PassThru) `
    -CodeCoverage (New-PesterCodeCoverageConfiguration -Path $PSScriptRoot -OutputFormat "CoverageGutters") `
    -HashTable @{} # some settings to be used as base, and amended by the settings above
    -Default:$false # (maybe in future) do not inherit settings from profile?
    -Configuration # (maybe in future) path to pester configuration in a file?


# maybe some templates as well? But how would this handle multiple templates, e.g. CI with code coverage, 
# and are there other useful templates choices? 
New-PesterConfiguration -Template CI

Can you elaborate on the -Default:$false idea? Is that like a -IgnoreProfile/IgnoreGlobal/IgnorePreference switch to ignore $global:PesterPreference"?

I guess my main question is: Do you see these functions as the primary method of configuration going forward? Considering how interactive the PesterConfiguration-object already is, will this compete or compliment it? I believe giving the users too many options can be confusing (+ more code + more documentation), so I thought this function had two possible directions:

  • A tool to generate/import a new PesterConfiguration-object which the user would customize further like today. In this scenario New-PesterConfiguration would have parametersets for different import-options + a common ignorepreference switch if I understood the idea behind -Defaults:$false:
SYNTAX
    # This is default config with optional template-option
    New-PesterConfiguration [-Template <string>] [-IgnorePreference] [<CommonParameters>]
    
    New-PesterConfiguration [-HashTable <IDictionary>] [-IgnorePreference] [<CommonParameters>]
    
    New-PesterConfiguration [-Path <string>] [-IgnorePreference] [<CommonParameters>]
    # Basically a cloning-option
    New-PesterConfiguration [-Configuration <PesterConfiguration>] [-IgnorePreference] [<CommonParameters>]
  • Full Powershell-interface for PesterConfiguration including New-Pester*Configuration wrapper-functions for each section (maybe except Debug due to name conflict and only used for advanced scenarios?):
SYNTAX
    New-PesterConfiguration [-Run <RunConfiguration>] [-Filter <FilterConfiguration>] [-CodeCoverage <CodeCoverageConfiguration>] [-TestResult <TestResultConfiguration>] [-Should <ShouldConfiguration>] [-DebugConfiguration <DebugConfiguration>] [-Output <OutputConfiguration>] 
    [-IgnorePreference] [-Template <string>] [<CommonParameters>]
    
    New-PesterConfiguration [-Run <RunConfiguration>] [-Filter <FilterConfiguration>] [-CodeCoverage <CodeCoverageConfiguration>] [-TestResult <TestResultConfiguration>] [-Should <ShouldConfiguration>] [-DebugConfiguration <DebugConfiguration>] [-Output <OutputConfiguration>] 
    [-IgnorePreference] [-HashTable <IDictionary>] [<CommonParameters>]
    
    New-PesterConfiguration [-Run <RunConfiguration>] [-Filter <FilterConfiguration>] [-CodeCoverage <CodeCoverageConfiguration>] [-TestResult <TestResultConfiguration>] [-Should <ShouldConfiguration>] [-DebugConfiguration <DebugConfiguration>] [-Output <OutputConfiguration>] 
    [-IgnorePreference] [-Path <string>] [<CommonParameters>]
    
    # Could possibly be dropped. Covered by Default (first set) with each parameter used.
    New-PesterConfiguration [-Run <RunConfiguration>] [-Filter <FilterConfiguration>] [-CodeCoverage <CodeCoverageConfiguration>] [-TestResult <TestResultConfiguration>] [-Should <ShouldConfiguration>] [-DebugConfiguration <DebugConfiguration>] [-Output <OutputConfiguration>] 
    [-IgnorePreference] [-Configuration <PesterConfiguration>] [<CommonParameters>]

@nohwnd
Copy link
Member

nohwnd commented Apr 18, 2021

-default $false. -> yes one possible syntax to ignore some default configuration, most likey the one inherited from pester preference. Maybe I am just overthinking this.

I think the form where we have the separate functions is nicer, but it also means there would be quite a few, which probably does not matter.

@fflaten
Copy link
Collaborator Author

fflaten commented Apr 18, 2021

-default $false. -> yes one possible syntax to ignore some default configuration, most likey the one inherited from pester preference. Maybe I am just overthinking this.

Not necessarily. I just couldn't think of any other default-kind of values except PesterPreference atm. I'd personally recommend a switch-parameter over bool every time to disable/ignore that.

The second option can feel a bit verbose and maybe complex, especially for Should and Output which currently only has one option each, but at least it won't explode with hundred parameters (and aliases) over time while providing intellisense and early input-validation.

$container = New-PesterContainer -ScriptBlock { param($first) ... } -Data @{ first = "1st" }
$run = New-PesterRunConfiguration -Path $PSScriptRoot -Container $container -PassThru
$output = New-PesterOutputConfiguration -Verbosity 'Detailed'
$conf = New-PesterConfiguration -Run $run -Output $output -Template CI -IgnoreDefaults

Invoke-Pester -Configuration $conf

Users familiar with the options can use hashtable (or another serialized file in the future) to keep their scripts short either wway. And there's always the Simple-interface of Invoke-Pester for the quick, interactive runs.

For now, should I update according to review comments so this PR can go forward as a start (only default and hashtable-conversion as options)? Or put this on hold until there's more input on the end-game from the #1265 ? I'd like more discussion before extending with 8 New-Pester*Configuration-functions. 🙂

@nohwnd
Copy link
Member

nohwnd commented Apr 18, 2021

Believe it or not I actually meant -Default to be a switch, I forgot to put : there. Yes please just the -HashTable, and no -Default for now. Let's not add the Pester*Configuration cmdlets for now.

@nohwnd
Copy link
Member

nohwnd commented Apr 18, 2021

Nice and clean.

@nohwnd nohwnd merged commit 734ccef into pester:v5.0 Apr 18, 2021
@fflaten fflaten deleted the pesterconfiguration branch April 18, 2021 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't find type [PesterConfiguration] though Pester 5.0.1
2 participants