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 Get-ShouldOperator #1116

Merged
merged 29 commits into from Dec 14, 2018

Conversation

Projects
None yet
2 participants
@brianbunke
Copy link
Contributor

brianbunke commented Oct 11, 2018

This PR would add a new function, Show-PesterAssertion, in an attempt to improve discoverability of Should parameter assertions.

Supercedes #1111.
Resolves #878. (See the issue for far more details.)

@brianbunke brianbunke changed the title WIP: Show-PesterAssertion, take two WIP: Get-ShouldOperator Oct 16, 2018

@brianbunke

This comment has been minimized.

Copy link
Contributor

brianbunke commented Oct 16, 2018

Per discussion in #878, changed name to Get-ShouldOperator. This PR will also add help for and rename all internal assertion operator functions (PesterBe --> Should-Be). I have already updated Be and BeExactly; need to get the rest of the built-ins, and then will remove the WIP label from this PR.

@nohwnd

This comment has been minimized.

Copy link
Member

nohwnd commented Oct 20, 2018

Maybe Add-AssertionOperator could take the command object, and keep reference to it, and take the name and definition from it. Just thinking out loud.

@brianbunke

This comment has been minimized.

Copy link
Contributor

brianbunke commented Oct 22, 2018

If I'm understanding correctly, that would be a breaking change to Add-AssertionOperator, which is now public.

I assume that change would look something like:

Add-AssertionOperator -Name Exist -Command (Get-Command Should-Exist)

And the internals could figure out the test script block, the help, and maybe even the SupportsArrayInput.

This PR is massive enough that I would love to not throw in a breaking change in on top of it. 🙂 But if that's correct, and you'd still like to see it done that way, let me know!

@brianbunke brianbunke changed the title WIP: Get-ShouldOperator Add Get-ShouldOperator Oct 22, 2018

@brianbunke

This comment has been minimized.

Copy link
Contributor

brianbunke commented Oct 22, 2018

If not pursuing the above, I think this PR is finally ready. I removed the WIP label from the PR title.

Summarizing:

  • Add Get-ShouldOperator
    • Returns name and alias of all registered operators
    • -Name is a dynamic parameter to allow for tab completion
    • If specifying one operator, returns the PowerShell examples help for that operator
  • Add optional -InternalName parameter to Add-AssertionOperator
    • Allows Get-ShouldOperator -Name Be to fetch help examples
  • Renames all Should operators' internal names to Should-xxx
    • From PesterBe to Should-Be, etc.
  • Adds help synopsis and examples to all Should operators
@nohwnd

This comment has been minimized.

Copy link
Member

nohwnd commented Oct 25, 2018

This PR is massive enough that I would love to not throw in a breaking change in on top of it. 🙂 But if that's correct, and you'd still like to see it done that way, let me know!

I was thinking two parameters sets, so the change would not be breaking, but fair enough. Adding that can be done later, or never.

Now I just need to test it.

@brianbunke

This comment has been minimized.

Copy link
Contributor

brianbunke commented Oct 26, 2018

Ah, right, I never think of the second parameter set. That could work, but yeah, I'll leave it alone for now.

@nohwnd nohwnd merged commit e04f19c into pester:master Dec 14, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@nohwnd

This comment has been minimized.

Copy link
Member

nohwnd commented Dec 14, 2018

@brianbunke I fixed the compatibility with PowerShell 2, and used both names and all aliases for the -Name parameter of Get-ShouldOperator. I added alias for Add-AssertionOperator and simplified the code a bit, mostly removed some formatting that was useless and the comments that just described what the next line does. Thanks for your contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment