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

Preference variable to suppress Mandatory prompts #2408

Closed
JohnLudlow opened this issue Oct 3, 2016 · 11 comments
Closed

Preference variable to suppress Mandatory prompts #2408

JohnLudlow opened this issue Oct 3, 2016 · 11 comments
Labels
Issue-Discussion the issue may not have a clear classification yet. The issue may generate an RFC or may be reclassif Resolution-Answered The question is answered.

Comments

@JohnLudlow
Copy link

The ability to mark parameters as Mandatory is great, but there is one critical omission - the ability to suppress prompts for Mandatory variables for a session.

The main reason this is important is because of unit testing. We want to set certain parameters as Mandatory=$true and then unit test (using Pester) that the parameters are in fact enforced as mandatory.

Unfortunately, this actually causes Pester to pause waiting for input. Our temporary solution is to not use Mandatory.

What we'd like to do is set something like $MandatoryPreference = 'Continue' so that if we miss something, we get an actual error thrown back

@lzybkr
Copy link
Member

lzybkr commented Oct 3, 2016

The PowerShell team has generally provided guidance that it's not important for cmdlet unit tests to test this behavior - a cmdlet unit test should simply verify that Mandatory is set as expected, e.g.

((Get-Command Get-Command).Parameters['Name'].Attributes | ? { $_ -is [parameter] }).Mandatory | Should Be $false

This way, you leave the responsibility of testing prompting behavior on Mandatory to the core PowerShell parameter binder tests.

@lzybkr lzybkr added the Issue-Discussion the issue may not have a clear classification yet. The issue may generate an RFC or may be reclassif label Oct 3, 2016
@JohnLudlow
Copy link
Author

I guess that kind of makes sense. I'll feed this back to the rest of my team. HelpMessage should be testable in the same way, right?

I still argue that it would be a good idea to provide this capability, if only to cover situations where you can't control whether the powershell session has been launched with powershell.exe -NonInteractive, or the session wasn't launched with powershell.exe -NonInteractive but you want to change that without restarting it.

Thanks

@vors
Copy link
Collaborator

vors commented Oct 3, 2016

I think it's a good request. Here is a scenario.

In a big non-interactive script, we currently hang, when such mistake slips in. Current behavior makes troubleshooting harder. It would be much better to have an option to error-out immediately when script knows that it's not intended for the interactive use.

@lzybkr
Copy link
Member

lzybkr commented Oct 3, 2016

HelpMessage could be tested in the same way, but we don't generally test the exact message because we rely on code reviews to ensure the messsage is good plus localization makes the test only work on specific locales.

I disagree that this functionality is needed. If PowerShell is not interactive, the parameter binder throws an exception if a mandatory parameter is missing - and this is exactly what you want - report an error, not to ignore it.

@lzybkr
Copy link
Member

lzybkr commented Oct 3, 2016

Here is an example of a non-interactive runspace throwing an exception:

#3 PS> [powershell]::Create().AddCommand("Import-Module").Invoke()
Exception calling "Invoke" with "0" argument(s): "Cannot process command because of one or more missing mandatory
parameters: Name."
At line:1 char:1
+ [powershell]::Create().AddCommand("Import-Module").Invoke()
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodInvocationException
    + FullyQualifiedErrorId : ParameterBindingException

@vors
Copy link
Collaborator

vors commented Oct 3, 2016

Indeed, then if somebody wants such check she can write a helper function to start script in non-interactive runspace.

@JohnLudlow
Copy link
Author

I'm not sure what that example was showing. It doesn't look like anything I'm trying to do.

There's a disconnect here between something which is 'needed' (a poisonous word, IMHO) and something that seems like a good idea. This is definitely in the latter group, given the guidance @lzybkr posted above.

In any case, you're the guys in charge so if you think it's not a good suggestion then we can close it.

@lzybkr
Copy link
Member

lzybkr commented Oct 3, 2016

I wanted to demonstrate how the PowerShell team would test this behavior (non-interactive) in our tests. A new process works similarly but is much slower:

#4 PS> powershell -noninteractive -noprofile -command "ipmo"
Import-Module : Cannot process command because of one or more missing mandatory parameters: Name.
At line:1 char:1
+ ipmo
+ ~~~~
    + CategoryInfo          : InvalidArgument: (:) [Import-Module], ParameterBindingException
    + FullyQualifiedErrorId : MissingMandatoryParameter,Microsoft.PowerShell.Commands.ImportModuleCommand

In my opinion, adding a preference variable to control this behavior is neither necessary nor a good idea. The variable introduces new complexity that needs to be documented and explained.

I'm closing the issue as I believe I've suggested reasonable solutions to testing the desired behavior. If there are situations I haven't covered, feel free to reopen.

@lzybkr lzybkr closed this as completed Oct 3, 2016
@lzybkr lzybkr added the Resolution-Answered The question is answered. label Oct 3, 2016
@JohnLudlow
Copy link
Author

Yeah I'm not sure the second example would achieve what we're after. We'll start looking into using the first example since right now we're not using the Mandatory parameter for a parameter which is mandatory. Instead, we're letting other validation handle that situation which is not necessarily ideal.

Thanks for looking into it anyway

@DarkLite1
Copy link

DarkLite1 commented Aug 29, 2017

For anyone else having this issue in case you use a Param() block at the beginning of your script, it can be solved like this:

$here = Split-Path -Parent $MyInvocation.MyCommand.Path
$sut = (Split-Path -Leaf $MyInvocation.MyCommand.Path) -replace '\.Tests\.', '.'

Describe 'input' {
    Context 'mandatory parameters' {
            it 'ScriptName' {
                (Get-Command "$here\$sut").Parameters['ScriptName'].Attributes.Mandatory | Should be $true
            }
            it 'CountryCode' {
                (Get-Command "$here\$sut").Parameters['CountryCode'].Attributes.Mandatory | Should be $true
            }
        }
}

@gmckeown
Copy link

I'm writing tests for various scripts that have multiple parameter sets. The above checks for the Mandatory attribute on a parameter aren't quite what I need. For example:

    Param(
        [Parameter(ParameterSetName = 'MatchName', Mandatory)][Parameter(ParameterSetName = 'MatchPattern', Mandatory)][SessionObject] $Session,
        [Parameter(ParameterSetName = 'MatchName')][String] $Name,
        [Parameter(ParameterSetName = 'MatchPattern', Mandatory)][String] $Pattern,
        [Parameter(ParameterSetName = 'MatchPattern', Mandatory)][String] $MatchOn
    )

I want to be able to check that Pattern is passed if MatchOn is passed. If Name is passed, I don't care about that pattern match being mandatory. So I could write some logic that tries to emulate what PowerShell itself is doing, but it would seem more useful to be able to just run the command and see how PowerShell reacts (and without the risk of it sitting waiting for input in the case of a missing param).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Discussion the issue may not have a clear classification yet. The issue may generate an RFC or may be reclassif Resolution-Answered The question is answered.
Projects
None yet
Development

No branches or pull requests

5 participants