Pester icon indicating copy to clipboard operation
Pester copied to clipboard

Way forward for Should?

Open nohwnd opened this issue 5 years ago • 11 comments

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).

nohwnd avatar Jan 12 '20 16:01 nohwnd

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

asears avatar Jan 17 '20 20:01 asears

@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?

nohwnd avatar Jan 19 '20 10:01 nohwnd

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.

asears avatar Jan 19 '20 12:01 asears

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

nohwnd avatar Jan 19 '20 20:01 nohwnd

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
}

fsackur avatar Sep 09 '21 00:09 fsackur

@fsackur thanks, I will try it :)

nohwnd avatar Sep 10 '21 06:09 nohwnd

I think the plan could be this:

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

    • I am not sure about the limitations, but it seems easiest to just publish them as global functions. We can try to grab a session state in during import-module, but it has edge cases. So I would consider this only when running pester in pester https://github.com/PowerShell/PowerShell/issues/6147#issuecomment-365132293
  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 avatar Nov 22 '21 08:11 nohwnd

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

nohwnd avatar Nov 22 '21 08:11 nohwnd

@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 avatar Apr 02 '24 08:04 nohwnd

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?

fflaten avatar Apr 02 '24 20:04 fflaten

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

nohwnd avatar Apr 03 '24 10:04 nohwnd