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

Add -InParameterSet in Should -HaveParameter #2273

Merged
merged 1 commit into from May 6, 2023

Conversation

fflaten
Copy link
Collaborator

@fflaten fflaten commented Nov 26, 2022

PR Summary

Adds support for asserting parameter in a specific parameter set.
This only affects whether parameter exists (or not) and if exists in set and mandatory.

Fix #2227

PR Checklist

  • PR has meaningful title
  • Summary describes changes
  • PR is ready to be merged
    • If not, use the arrow next to Create Pull Request to mark it as a draft. PR can be marked Ready for review when it's ready.
  • Tests are added/update (if required)
  • Documentation is updated/added (if required)

@fflaten
Copy link
Collaborator Author

fflaten commented Nov 26, 2022

Just to be clear, Should -Not -HaveParameter abc -InParameterSet 'foo' -Mandatory will still fail if parameter exists regardless of mandatory-requirement as -Not, see #2105

@fflaten
Copy link
Collaborator Author

fflaten commented Nov 26, 2022

Pending #2267 to fix CI

Copy link
Contributor

@johlju johlju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. The tests seems to cover all (that I can see) possible variants.

@fflaten
Copy link
Collaborator Author

fflaten commented Nov 26, 2022

Not sure about the parameter name. Maybe just -ParameterSet?

@johlju
Copy link
Contributor

johlju commented Nov 27, 2022

Not sure about the parameter name. Maybe just -ParameterSet?

I think -InParameterSet is good enough for now. It makes sense when used together with -HaveParameter. It could be HaveParameterSet if it it should be able to be used standalone, but if it never should be used alone then as-is is good enough. 🙂

Come to think of it we did not have a test case for what happens (should happen) when using -InParameterSet without -HaveParameter, did we?

@fflaten
Copy link
Collaborator Author

fflaten commented Nov 27, 2022

Thanks.

It could be HaveParameterSet if it it should be able to be used standalone

Come to think of it we did not have a test case for what happens (should happen) when using -InParameterSet without -HaveParameter, did we?

-HaveParameter is the assertion/operator name so it's mandatory (like -BeTrue etc).

The reason I'm unsure is that we normally use -Mandatory (not -IsMandatory) and -DefaultValue (not -HasDefaultValue). On the other hand, this operator already use -HasArgumentCompleter so it's already special. 🤷‍♂️ 🙂

-InParameterSet felt more intuitive since it's used as a single-value filter for the operator. -ParameterSet could read like it accepts an array of set names (as an assertion).

@johlju
Copy link
Contributor

johlju commented Nov 27, 2022

-InParameterSet felt more intuitive since it's used as a single-value filter for the operator. -ParameterSet could read like it accepts an array of set names (as an assertion).

I agree.

@fflaten
Copy link
Collaborator Author

fflaten commented Dec 4, 2022

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 1 pipeline(s).

@nohwnd nohwnd merged commit 08998af into pester:main May 6, 2023
@fflaten fflaten deleted the haveparameter-inparameterset branch May 6, 2023 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Evaluate parameter in a parameter set
3 participants