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

Way forward for Should? #1423

Open
nohwnd opened this issue Jan 12, 2020 · 11 comments · May be fixed by #2428
Open

Way forward for Should? #1423

nohwnd opened this issue Jan 12, 2020 · 11 comments · May be fixed by #2428
Labels
Milestone

Comments

@nohwnd
Copy link
Member

nohwnd commented Jan 12, 2020

Should assertions have multiple quirks and pain points, here is a not-complete list:

  • -Be does too much, so it is hard to understand what is happening
  • Enumeration via pipeline complicates a lot of things and -Be has special logic to revert this, which does not always work
  • It is difficult to apply seperate special behavior for different types of data (e.g. Should -BeFalse -AllowNull -AllowTypeCast -FalseStringAsFalse, Should -BeString -CaseSensitive, Should -BeCollection -All { $_ -like "*file.ps1" }) because parameter sets limited to 32 parameter sets per function

I researched the various limitations of Should by writing a separate module Assert which has nice functionality but is not used often because people are not aware of it. I think it would be wort it to merge the functionality into Pester and fix some of the quirks finally. But I am wondering what ways forward I actually have.

There are few more limitations:

  • How to export the Should-* functions without triggering Verb warning? (Pester is getting around this by using single word functions at the moment).
  • How to ensure backward compatibility? (probably Should -Be has to remain in place while Should-Be is added)
  • Should this change be done in v5? (if it is parallel then we can do it as non-breaking change).
@asears
Copy link
Contributor

asears commented Jan 17, 2020

Should Be that community use Assert module rather than Should? Seems much friendlier, some Rules to Better Pester Tests would be good.

Could this be done by aliasing the commands? By a configuration or feature toggle to enable in Pester v5?

Some of the features are great in Assert (eg. compare), some are a bit odd naming and could be leveraging some built-in commands (eg. Format-Nicely?) but are useful too!

Is it possible for the assert module to be added to a Microsoft Trusted Repository rather than PSGallery? Or perhaps a wiki page could be added for Azure Devops integration of Pester + Assert.

https://docs.microsoft.com/en-us/azure/devops/artifacts/tutorials/private-powershell-library?view=azure-devops

@nohwnd
Copy link
Member Author

nohwnd commented Jan 19, 2020

@asears Thanks for the suggestions. Assert was my playground to find what is possible, and to look at assertions from a different angle. In some cases I went too far, and then returned back (e.g. -Be which was different per type, which get's pretty annoying especially for numeric types). I think that the biggest problem is getting it to people, because I presented about it multiple times, and I always get asked if it will become part of Pester, as if installing it from the gallery would be difficult. I would also like to see it in Pester, because the current assertions are a bit difficult sometimes.

I think that aliasing is a good option, and it was suggested on slack, I am just not sure what to do with the actual commands that will be exported. Sooner or later someone will use them somewhere and then they will need to be supported. They will also pop-up at the top of the list in ExportedCommands on module object because they are Assert-*.

It would be nice to keep it consistent as well. In Pester v5 I am already working with New-Test -> It, New-Block -> Describe / Context. I could export those as well, but then it would get complicated because Mock allows you to differ scopes based on Context / Describe and those new names for blocks would have to be reflected there as well. And it would be similarly dificult for aliases, Describe and Context would map to the same function but the function would have to be aware of which alias called it so we can keep the same behavior. I could keep Describe and Context a function, but then it would be inconsistent again.

Is it possible for the assert module to be added to a Microsoft Trusted Repository rather than PSGallery? Or perhaps a wiki page could be added for Azure Devops integration of Pester + Assert.

What is that, and what would it bring?

@asears
Copy link
Contributor

asears commented Jan 19, 2020

Thanks for answering my questions. It sounds like Assert should be merged into Pester, with some refactoring done on the Assert repo prior to this / during this. Maybe an interim fork with two options for running Pester with optional Assert? Changing apis while remaining backwards compatible is hard.

When I install Assert it's showing up as "untrusted" as all modules coming from PSGallery seem to be by default. Setting up a trusted artifacts library in Azure Devops is one way around this. Publishing / promoting more info on Assert within Pester readme.md and wiki might help.

Will see if I can bring the discussion to Slack.

@nohwnd
Copy link
Member Author

nohwnd commented Jan 19, 2020

@asears I am open to suggestions on the refactorings, if you want to share 🙂

@fsackur
Copy link

fsackur commented Sep 9, 2021

About the module warnings on non-canonical verbs, try this:

# .psd1
FunctionsToExport = @('Do-Stuff')


# .psm1
# Don't dot-source the command directly
$SomeModuleVariable = 42
Set-Item Function:\Global:Do-Stuff $(& {. $PSScriptRoot/Public/Do-Stuff.ps1; Get-Command Do-Stuff})


# Do-Stuff.ps1
function Do-Stuff
{
    "What is six times nine?"
    [pscustomobject]@{Answer = $SomeModuleVariable} | ft
    $MyInvocation.MyCommand.Module | ft
}

@nohwnd
Copy link
Member Author

nohwnd commented Sep 10, 2021

@fsackur thanks, I will try it :)

@nohwnd
Copy link
Member Author

nohwnd commented Nov 22, 2021

I think the plan could be this:

  1. Publish assertion operators as separate functions with Should-Be syntax.

  2. Keep the current assertion operators working as is.

  3. Add new operators using Add-AssertionOperator to both parameter set, and as a function.

  4. Add option to disable adding to parameter set, and (much) later make it the default.

  5. Add all the new operators I want by porting them from Assert, and consider which breaking changes I want to keep.

@nohwnd
Copy link
Member Author

nohwnd commented Nov 22, 2021

  1. Deprecate the Exactly assertions, because it is very unclear what the "Exactly" means.

@nohwnd
Copy link
Member Author

nohwnd commented Apr 2, 2024

@fflaten FYI I am working on porting Assert assertions to Pester. To ship them side-by-side with the current Should -Be. My plan is to publish them as regular functions with approved verbs Assert-Equal, Assert-StringEqual, and alias them as Should-Be, Should-BeString and similar. I would like to make as few compromises on the existing functionality of Assert, e.g. there are separate assertions for singular and collection item. I don't know how soon there will be PR for the port, but after that I would like to add some more general assertions, e.g. for time, ranges.

@nohwnd nohwnd linked a pull request Apr 2, 2024 that will close this issue
5 tasks
@fflaten
Copy link
Collaborator

fflaten commented Apr 2, 2024

Thanks for the update 🙂

  1. Add new operators using Add-AssertionOperator to both parameter set, and as a function.
  2. Add option to disable adding to parameter set, and (much) later make it the default.

Is this part of the plan scrapped? Future custom operators should be standalone functions only to keep things simple?

@nohwnd
Copy link
Member Author

nohwnd commented Apr 3, 2024

Currently I am more inclined to doing 1 migration to the (hopefully) better solution which is Assert assertions. My plan here is to:

  1. Keep the current assertion operators working as is.
  2. Add Assert assertions as standalone functions
  3. Port assertions that exist only in Pester to also use the function (e.g. Should-Invoke) syntax
  4. bother some people like dba checks to move to the new assertions and see how much breaks
  5. eventually deprecate Should + parameter sets

Future custom operators should be standalone functions

yes

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 a pull request may close this issue.

4 participants