Configuring when Should fails

[nohwnd]

Improvement for #1325

The Should assertion won't fail by default, instead it will store the error and continue executing, unless -ErrorAction Stop or $ErrorActionPreference = 'Stop' is specified.

Should ErrorActionPreference make the Should fail, or just the -ErrorAction and possibly ShouldActionPreference (and respective global Pester option) should make Should fail?

The reasoning here is that Eap will have impact on all the code that we are running and not just on the Assertions, and will also prevent the multiple assertions from working if eap Stop is used to force the rest of the code to fail.

I am inclined to do it this way:

• Add ShouldErrorAction option to Pester that will allow to opt-out from the new feature globally, to allow current test suites work as they did before (even though unless you use guard assertions they mostly will already)
• ShouldErrorActionPreference variable to opt-out in one file / scope. This can also be used to migrate by specifying $ShouldErrorActionPreference = 'Stop' or Pester option ShouldErrorAction 'Stop', and enabling it per file in BeforeAll { $ShouldErrorActionPreference = 'Continue' }.
• Respect -ErrorAction Stop on the assertion, but not the $ErrorActionPreference from the scope.

Thoughts?

[vexx32]

I think having the preference variable to opt out globally is good, but in terms of being more granular I'm not sure it necessarily makes sense to ignore $ErrorActionPreference. Since that variable is already widely know, introducing a new one will not be particularly discoverable. I suspect folx will only be very confused as to why Should is ignoring their set preference.

Perhaps we should just use $ErrorActionPreference -- and -ErrorAction. Best to stick with what is already known and used here rather than completely reinventing this same wheel, in my opinion.

[Jaykul]

I think the point, @vexx32 is that ErrorActionPreference is not respected by Should in Pester 4, so making it suddenly respect ErrorActionPreference would obviously be a breaking change.

Doing so without a Pester-level configuration option would mean updating everyone's CI builds, at a minimum, but for us, it would probably require adding $ErrorActionPreference = "Stop" to every existing test script, or setting $PSDefaultParameter values on every developer's profile -- either of which would mean that we would not be able to easily gracefully migrate to the new style.

Here's an example of the problem. We frequently write tests like this:

Describe "Set-Thing" {
    Mock GetConfiguration { }
    Mock GetConfiguration { "ImportantValues" }

    Context "It calls GetConfiguration and passes the output to the Configuration function" {
        It "Calls GetConfiguration" {
            Set-Thing Important "KeyNote"
            Assert-MockCalled GetConfiguration
        }

        It "Passes the output of GetConfiguration to the Configuration function" {
            Assert-MockCalled ImportantConfiguration -ParameterFilter {
                $RealKeyValue | Should -eq "ImportantValues"
                $true
            }
        }
    }
}

Specifically, we frequently test our mocks by having the ParameterFilter always return $true but after an assertion that the value is correct. Since Should always throws, if the assertion fails, we get a meaningful error message like:

    [-] Passes the output of GetConfiguration to the Configuration function 9ms
      ...
      Expected: 'ImportantValues'
      But was:  'IncorrectValues'

Which makes it a lot easier to hunt down the bug than if the failure is just:

    [-] Passes the output of GetConfiguration to the Configuration function 12ms
      Expected ImportantConfiguration to be called at least 1 times but was called 0 times

For us at work, if Should starts writing errors instead of throwing, it would break these mocks (the mock passes) even if Pester changed to consider errors as failures (which it does not, currently).

Further, although we might see some benefits on our oldest tests (where there may be lots of should statements in a single It), I think it would be rare that it would help with anything written in the last couple of years, because we have adapted to this poor behavior, and we tend to write every should in an it assertion -- so we can see all the failures already.

For what it's worth, if you want to play with that test, here's a dummy implementation:

function ImportantConfiguration {
  [CmdletBinding()]
  param(
    [string]$RealKeyValue
  )
  <# side effects that configure our environment #>
}

function GetConfiguration {
  [CmdletBinding()]
  param(
    [string]$Name,
    [string]$Value
  )
  <# look things up based on the current state of the environment and return #>
  switch ($Name) {
    # Lots of logic ... 
    default {
      throw "Unknown Name: $Name"
    }
  }
}

function Set-Thing {
  [CmdletBinding()]
  param(
    [Parameter(Mandatory)]
    [string]$Name,

    [Parameter(Mandatory)]
    [string]$Value
  )
  <# checking and lookup stuff #>
  $KeyValue = GetConfiguration -Name $Value -Value $Name
  if ($Name -like "*Important") {
    <# additional logic #>
    # Set the value internally
    ImportantConfiguration $KeyValue
  } <# else, a few other options #>
}

[vexx32]

... Yep, that seems to be exactly what @nohwnd was mentioning. That's the premise of this issue, not so much a "should we do that" as more of a "how should we let users change the new default behaviour".

Specifically, the main points of this issue seem to be that, in v5:

1. Should will by default emit non-terminating errors, that Pester will consider as test failures. I could be misreading, but from the issue text it reads to me as "this has been done, so we need a way for users to change it if they need to."
   • Corollary: should Pester then consider all non-terminating errors as failures, or only Should-produced errors? -- Probably should be a configuration option. My first thought is to have it only consider Should-produced errors as failures by default.
2. Do we need a Pester-specific configuration variable for that? I don't think so, but I'd really have to write some tests with this behaviour and see how they behave, I don't think that's something we can easily determined without some field tests to see how it behaves in conjunction with other things that are affected by $ErrorActionPreference.
   • I think that it won't need its own variable for most use cases; the times we need Should to be halting, we can just use -ErrorAction Stop on that specific call. There may be cases that warrant wider usage, but even then... we can just make use of $PSDefaultParameterValues and this can be documented so users are aware of it. Adding another pref variable doesn't seem to add a lot of value here.
3. Yes, this will help simplify tests by allowing us to do multiple tests in a single It block. That's a plus, but can depend on the nature of the test. Tests that are sensitive need only add -ErrorAction Stop. Not a huge change imo.
   • So with your specific mocking example, just add -ErrorAction Stop to the Should calls in the mocks, nbd. Gotta say, not seem ParameterFilter used that way though, that strikes me as a bit odd tbh.

And lastly -- adaptations can be unlearned. Imo, this gives users of Pester the ability to determine how Should behaves, which gives us more granular control over our testing. Wins across the board there. I don't see a real need for specific preference variables for Pester functions; there is already functionality baked into PowerShell that lets us do the same thing such preference variables would allow us to do.

[KevinMarquette]

With 5.0 being a breaking change, it's the perfect time to make this happen. I commonly get questions about how to do this.

I would be happy with using -ErrorAction and ErrorActionPreference to have it stop on should failures. If we go that way, I suggest being able to specify the -ErrorAction on the It, Context, and Describe blocks too. I think it would be intuitive once we adjusted to it and I prefer it over the ErrorActionPreference because I can opt in and out on a case by case section of my tests.

I would also recommend a new helper function that asserts that all of the should assertions have passed in the current scope. It could be a low priority feature, but it would fit with the idea and give users full control of how they use it.

[nohwnd]

My original concern was a bit different from what you both mentioned, but you are raising some good points.

My original concern

Jane uses the new behavior by keeping the global Pester setting to 'Continue', but finds out that it causes some of hers tests to fail unexpectedly. She would like to revert to the old behavior in multiple files to have easier time migrating. She expects the assertions to work as they did before, but does not expect the setting to have any impact on the rest of the code

Here is some (contrieved) example of her code:

function a () {
    $f = Get-Item "A:\non-existing.jpeg"
    [PSCustomObject]@{
        Exists = $null -ne $f
        Name = $f.BaseName
        Path = $f.FullName
    }
}

Describe "a" { 
    It "continues to the last error" {
        $f = a
        $f.Exists | Should -BeTrue
        $f.Name = "file1.txt"
        $f.Path = "C:\file1.txt"
    }
}

She expects to get non-terminating error from Get-Item, and then failure on the first assertion, because that is how it works in Pester v4:

Describing a
Get-Item : Cannot find drive. A drive with the name 'A' does not exist.
At line:5 char:10
+     $f = Get-Item "A:\non-existing.jpeg"
+          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : ObjectNotFound: (A:String) [Get-Item], DriveNotFoundException
    + FullyQualifiedErrorId : DriveNotFound,Microsoft.PowerShell.Commands.GetItemCommand

  [-] continues to the last error 78ms
    Expected $true, but got $false.
    16:         $f.Exists | Should -BeTrue

BUT in Pester v5, specifying $ErrorActionPreference = 'Stop' on the top of the script, has impact on all commands that respect it and will terminate the test when Get-Item fails. Keeping $ErrorActionPreference = 'Continue' will not terminate at the first failed assertion. So there is no easy way to revert to the old behavior in the whole file without editing every test.

The solution to this would be to introduce a variable ShouldErrorActionPreference that would only impact Should, and that way she could specify it on the top of the file and get the old behavior without visiting every test in that file.

Makes sense?

Mocks

@Jaykul that is a valid concern and I am aware of that pattern. I think an improvement would be to implement error collecting in the scope of Mock, so that way the behavior would remain consistent, and this pattern would continue working.

Non-terminating errors

@vexx32 It actually does not work that way, Pester is not collecting all non terminating errors, instead the Should writes into a collection of errors when the assertion does not pass. I am not sure why I did not implement it just as "error stream", it probably did not occur to me at the time.

On one hand collecting the errors from Should directly and not making the test fail on all errors might be better, because then we can have scenarios as the one above, where non-terminating error is raised and the test still can pass (if we were testing that $f.Exists | Should -BeFalse).

On the other hand sometimes you raise non-terminating errors by accident and you'd rather want terminating errors. Then the question is how to handle that, should Pester detect that non-terminating error was raised and do something? Should it just save it into some collection and do nothing, should it fail the test? Imho there are many different scenarios to consider and neither is a clear winner.

Verify assertion pass

I would also recommend a new helper function that asserts that all of the should assertions have passed in the current scope.

@KevinMarquette Could you expand on this please? What would be the benefit of having that? Right now this is what the It block does, it looks at the ErrorRecord collection on ScriptBlock invocation result and if there are any errors the test is considered to be Failed. The error can get to the collection in three ways:

• it is thrown by the code
• it is thrown by Should
• it is added there by Should

In fact it should not be just the It block, it should be all code that uses Invoke-ScriptBlock, and that should be all code that invokes user provided scriptblocks. Also Should internally detects if it runs inside of Pester.Runtime. If it doesn't it always throws the error on failure, this way the error is not lost when we are not able to collect it.

[KevinMarquette]

Looks like I only got half my thought on the page. I was thinking of the scenario where I have a large test with lots of the new should operators and I may want to have the ability to check half way in if anything has failed, then stop execution of the test if so. Like check twenty things and if all is good, execute the next hundred.

I'm sure I would just break it into two tests. It's just a thought I had while reading this thread, not sure if there is a real need for it.

[Jaykul]

What @nohwnd is describing sounds OK to me. It's what I was expecting (although I may not have explained myself well). It's worth pointing out that we could avoid the extra parameters by using default parameters:

$PSDefaultParameters["Should:ErrorAction"] = "Stop"

What @vexx32 is describing sounds like a new test tool. I'd be freaking out. Changing Pester to fail on all errors, but have Should not throw would be compounding breaking changes. We can't start saying "oh, it's a breaking change, let's break everything" -- the only good news about that would be if I could use the chaos and pain to justify rewriting everything in Gherkin syntax.

[vexx32]

Yep, I mentioned $PSDefaultParameterValues above as well. I agree, that pretty much makes an additional parameter / preference variable not really necessary in my opinion. 🙂

I wasn't sure exactly what the scope of this change would be, so appreciate the clarification @nohwnd. But it sounds like everyone's on the same page now. 👍

[nohwnd]

to check half way in if anything has failed

@KevinMarquette This seems like an edge case, and seeing how confused people are about Assert-VerifiableMocks I will let you implement this yourself if you ever need it. Once every test has access to the state it would be as easy as if ($Test.ErrorAction.Count -gt 0) { throw }.

@Jaykul @vexx32 the default parameter is a nice idea, let's go that way. It removes the need for custom variable and is supported from PowerShell 3 above, which is what Pester targets. ( At least that is whay they say here: https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_parameters_default_values?view=powershell-6 )

To make it work I will still have to remove the $ErrorActionPreference checking in Should, and explain it in documentation.

We can't start saying "oh, it's a breaking change, let's break everything"

@Jaykul agree here. I am trying to do that within reason. I think that the change in Should is a rather big breaking change, but useful one. Luckily for all tests that have one assertion at the end of test the breaking change is transparent to the user. And for many tests where there are multiple assertions at the end of the test the new behavior is more desired anyway. Where the problems start are guard assertions. But the test will fail anyway, it's just that more work will be done than necessary.

[nohwnd]

Okay trying it out, and the $PSDefaultParameterValues usage is a bit more nuanced that I'd like, but I will still stick with it...

The problem is that the variable holds a hashtable which is a ref type, so we get different scoping when we do $PSDefaultParameterValues['Should:ErrorAction'] = 'Stop' and $PSDefaultParameterValues = @{ 'Should:ErrorAction' = 'Stop' }.

The first call modifies a the table and is then used in every scope, the second one assignes a new hashtable to a new local variable and is applied only in the current scope. Clever, and confusing...

# scoped
$PSDefaultParameterValues.Clear()

&{ # one extra scope
    $PSDefaultParameterValues = @{ 'Should:ErrorAction' = 'Stop' }
}

$PSDefaultParameterValues # remains empty

# not scoped
$PSDefaultParameterValues.Clear()

&{ # one extra scope
    $PSDefaultParameterValues['Should:ErrorAction'] = 'Stop'
}

$PSDefaultParameterValues # remains Should:ErrorAction = 'Stop'

[vexx32]

@nohwnd yeah there is a bit of nuance there, and it may be worth mentioning how that plays out for Should since Pester does its own scoping in particular ways that may not be familiar to everyone (to complement docs on Describe and It, etc., And how they scope things).

[nohwnd]

@vexx32 The scoping is a bit easier to understand in v5. What you will want in this case is probably one BeforeAll on the top of the file with the code in it. That will set it for the whole file, and just that file. But I will make sure to explain it.

[jhoneill]

Late, since this is closed. I get really worried by changes which say "Previously when something was 'wrong' subsequent code did not run. Now we will run that code."

For example

$selection | should not beNullOrEmpty 
{get-stuff | where Property -match $selection | delete stuff} |  should not throw. 

It's very easy to change the script to work but anyone who just installs the new version could be in trouble. Especially if they are using Pester to verify correct operation of a production environment (which is most of what I have used it for).

The reason for posting is this a "Can / should / Must" split. Must is non-negotiable Up to this point pester "Shoulds" are really "musts" should is ... there are might be other ways, this might be optional, but "better if it is the case than not" And can or may is optional, not really better or worse (optional and much better is should, and optional but much worse is should not) .]

So I would prefer to see a SHOULD which with the old behavior and a MAY/MIGHT/CAN which is optional .

Just my 2% of a currency unit

[vexx32]

@jhoneill I hear you, but I think this is a better move than an additional cmdlet. With proper documentation this shouldn't pose any particular problem. Sure, it means you might need to update a few of your more sensitive tests, but on the whole I think it makes the most sense.

I don't think it's wise to try to maintain a perfect backwards compatibility scene by duplicating cmdlets. Some backwards compatibility is necessary, but trying to retain the exact experience will be an exercise in frustration for both maintainers and users of Pester. In my opinion, the clearer and more visible the break is, the easier it is to document and understand, and thus adapt old tests for.

Besides, until tests have been rewritten, the 4.9.x branch of Pester can still be used without issue. As far as major version changes go, this is pretty minor. 🙂

[jhoneill]

@vexx Image in a new machine arrives. IT Pro fires up powershell for the first time

>Install-module pester
>copy U:\myscripts\*.*
>Test_AD_Add_Replicate_remove.ps1 

Phone rings ... "Yeah ... uh-huh. So none of you can log in? OK Just let me ... oh holy .... "

Same OS. Same PowerShell version. Same step to install the module. Same script we've run 1000 times before. The IT pro didn't know that since they last installed Pester a change has been made which will end their career ?

It's not a call for 100% backwards compatibility because sometimes you have to break stuff to make progress. But "Breaking changes" which mean existing code stops running, are annoying, but not running does no active harm. That's failing safe (and things should go to or stay in a safe state if the script doesn't run.) We get a delay while things are put right. Code changes which remove a block on something running may do any amount of harm, because no one can say where safety relied on that block. Such changes must be opt in. . Again fail safe, not fail "terminate employment"

Having spent the holidays working with Pester and building on @adamdriscol 's selenium stuff I have a headful of syntax I'd like to see having thought about can/should/must it is fair to say that if this were a brand new project I'd be arguing that the word SHOULD needs to be "MUST"

$x  | must  not beNullorEmpty 
$y  | must be exactly "Christmas" 

etc. and

$a | might not beNullorEmpty 
$b | might be "Christmas" 

Must would stop dead in its tracks on a fail; should would continue but fail the whole test, and might would flag a warning but not fail the test. That's not where we are starting from. So it needs a way to make progress with safety and reasonable compatibility

Three predicates doesn't mean triplicate code (yes, having the same code multiple times ... never good. D.R.Y and all that.) I wrote this doing the selenium stuff

function Clear-SeAlert {
    [Alias('SeAccept','SeDismiss')]
    param (
    ... [ValidateSet('Accept','Dismiss')]
        $Action = 'Dismiss'  ... 
    )
    if (-not $PSBoundParameters.ContainsKey('Action') -and
        $MyInvocation.InvocationName -match 'Accept') {$Action = 'Accept'}
    ...
}

So I have one command with 3 names. Call it as SeAccept it accepts the alert, call it as Clear-SeAlert and it will assume Dismiss but you can write that explicitly or write -Action Accept to accept. And if you want to have something similar to SeAccept for dismissing an alert, SeDismiss is there just for completeness.

So one could get the syntax and behaviour above without tripling the code, which I agree is something to avoid.

[vexx32]

Once again, nobody is saying it's perfect, but nor would I recommend anyone update everything in production to a new major version without first verifying tests. Frankly, that's just not really doing due diligence?

Versions 4.9.x and below of pester don't suddenly stop being available. I'm afraid I don't follow your hypothetical where everything falls apart, unless you're positing that a majority of IT professionals don't do their due diligence and upgrade production code before verifying it works as expected. This change isn't going to suddenly stop tests working at all. If your "tests" are modifying your production environment... that's shooting yourself in the foot to begin with, surely?

I'm really not following how you find this harmful in the least. Inconvenient in some niche circumstances and requiring you to update some tests, sure. But not much more than that.

[jhoneill]

@vexx32
You are coming at this backwards when you say

This change isn't going to suddenly stop tests working at all. No it isn't , test which run now will still run. That isn't the problem. The problem is that things which prevented from running will, instead, run in the future

If You don't follow how this could be harmful Copy & paste this

describe test {
    it "does stuff" {
    $unitializedVar | should not benullorempty
    Write-Host "This >>$unitializedVar<< must contain something"
    }
}

Do we ever see "This >><< must contain something" ? Now take this code.

describe test {
    it "Got the selection criteria and was able to delete selected items" {
    $Selection | should not benullorempty
    Get-childitem | where name -match $selection | del 
    Get-childitem | where name -match $selection | should be nulloremoty 
    }
}

Currently if $selection isn't set , delete doesn't run. What is proposed is if selection isn't set, instead of stopping, all files will be deleted. People have learned you can have an assertion which fails to exit from a block which might otherwise do damage. They also think a throw will always break out (but haven't tested setting errorAction) but that's another story

Inconvenient is when code which was expect to run fails and you have to mend it. This is code which runs when it was NOT expected to run which might be totally harmless or might be career ending.

I have been telling IT pros that they should use Pester to test their infrastructure for years (I wrote https://jamesone111.wordpress.com/2016/06/01/a-different-pitch-for-pester/ 3 1/2 years ago, and I'd been saying it well before that). It isn't just used for unit testing software. I spoke to an old consultancy client a few weeks ago and they're using a script I wrote in 2017 to turn DSC changes into steps in per-machine pester tests. If they think something is wrong they run the test to see if the machine IS set the way they told DSC to set it. (Since some of the DSC classes are in house this is a seen as a better check than asking DSC)

If your "tests" are modifying your production environment... that's shooting yourself in the foot to begin with, surely?

Did you know in WWI soldiers shot themselves in the foot to get hospitalized and avoid fighting? That aside how do you test that a production environment is running normally ? How do you check that A is reachable, the B has replicated, that you can sign in to C, create, read update and delete a row on D. Or Send and receive messages on E, Or Read, Write and Delete files on F. Some are "look but don't touch", others verifications need a change. Either way pester is a good tool for it

Your faith in IT pros is touching but as I near the end of a long career in support, training, consulting, evangelism, and consulting again which has taken me into and out of more places than I can count I can say: (a) despite having smart people in our community "IT pros" include at least their share of the slow-witted and lazy, in fact (b) In some organizations that describes the majority of IT staff - which is the root of many problems. (c) even smart ones assume what worked last time will work next time, and safe scripts don't suddenly become dangerous. So I've no idea if it is a majority, or a large minority but in the organizations which have needed to hire-in my skills over the last 30+ years, just installing the current version and expecting it to work like the old one is normal. install-module pester (or whatever module) doesn't even say which version has been installed. Hence my scenario - new machine without the modules, script needs to run, but a module is missing, install module, run script, job done. Who inserts "check version" and if it isn't what they got last time, who reads the release notes, and checks the script they intended to run.
In the 4 weeks or so since Doug Finke thought I made big enough changes to import Excel to call it V7. 11,000 people have installed it. More than 99% of downloads take whatever happens to be current. If people thought that today's version might have a change which costs them their job, a lot of repeat downloaders would be asking for a known-safe version. Every downloader of 7 has ignored the request for donations, so how many checked what had changed? I'd put it close to zero. Of those, how many preemptively checked their code in case they need to change something. Even closer to to zero.
And no, it isn't acceptable to say "Well if they didn't know better it's their fault if they screw things up and get fired". We who know better have a duty of care to those who don't

[vexx32]

I think your expectation that all code from every possible new version work identically is simply unrealistic. There will always be changes in an update. Anyone running code in prod should be pinning versions and only upgrading production modules after verifying that nothing critical has changed. Yes, maybe some folx aren't. I don't think it's realistic to walk on eggshells just to enable these folx to continue to be careless. It seems highly unlikely that the one thing that trips them up would happen to be Pester if that's how they're handling their dependencies.

I'm not sure where you get this idea that the tool should be assuming its users are incompetent. If we assumed that, Remove-Item wouldn't even exist as a cmdlet. It would be too dangerous.

There is some level of simplifying things, of course, that's why tools exist, but Pester isn't designed to think for it's users. And shouldn't be, in my opinion. I think going that route is just an exercise in frustration for everyone involved in Pester's development.

Please don't just hamper improvement to modules based on a devil's advocate worst-case scenario which assumes the person running production code is handling their dependencies completely carelessly. If that were the case, it would almost certainly break long before Pester changed something.

Documentation and informing users is the answer here in my opinion, not catering to and encouraging poor coding practices. :slightly_smiling_face:

I appreciate you may think differently, but at the end of the day I think this is a step in the right direction for Pester. ^^

[KevinMarquette]

This release is a major revision to Pester and I am already strongly advocating everyone to pin their version of Pester now before it gets released (if they are not already doing so). This release is already going to require me to carefully review every project before I move them to this version. This feels like the right release for this change.

The core issue is more that PowerShell has poor dependency management. I don't want to go into that rant here, but that does contribute to the problem. Module owners should have no fear of introducing breaking changes if they bump the major version.

One important question back on topic, does this change also apply to use of should outside of Pester test scripts?

@jhoneill is not wrong in that People do use Pester in ways it was never intended. I am no longer surprised when I find a process that included the execution of a Pester tests and found it making production changes. Something that I would see as a validation, others see as a chance to validate and correct. If people are mixing the use of Should into their non-test scripts, then this change would have a dangerous impact to them.

[nohwnd]

@jhoneill I understand your arguments, and Pester is keeping backwards compatibility where possible (v4 still supports PowerShell v2 for example.) I am also afraid of making breaking changes and that is why I am using semantic versioning, maybe not strictly, but definitely for a big breaking changes.

I even had a discussion at Powershell conference Europe about what I am preparing for Pester v5 and whether or not I should make breaking changes or not. The voting was overwhelmingly for breaking changes.

https://github.com/pester/Pester/issues/1218 https://github.com/pester/Pester/issues/1319

I am also very open about v5 having a lot of breaking changes, talk about it online and in person, open threads where people have chance to express their opinion, and will make sure that I will announce new version coming for a quite some time before putting it into powershell gallery.

That said I am still sure that it will come to many people as a surprise and there will be many threads looking for reasoning for the changes, and calling for them to be reverted. But that is sadly the life of a project maintainer, calling for beta testers will give you 10 people actually trying the release out, and releasing it will give you 100 complainers, just checkout the numbers on pre-releases :)

@vexx32 thanks for advocating for this change, I also think this is a step in the right direction.

@KevinMarquette the expected behavior is to throw when not in Pester context. Implemented here:

https://github.com/pester/Pester/blob/v5.0/Functions/Assertions/Should.ps1#L89

I should add an explicit test for it though. Filed in #1410 and the improvement @Jaykul suggested in #1409, both for 5.0 milestone so it is in the initial release.

[jhoneill]

I think your expectation that all code from every possible new version work identically is simply unrealistic.

OK Stop there That is not my expectation. Before you were born* someone told me you can have progress, compatibility or freedom of choice. If you are very lucky you might get two. All 3 is impossible. Much progress, and involves changes which means things stop working. That is a breaking change and the reason why we have a change in the major version number is a flag that says "Some things might not work".

This is the complete opposite. Code which was prevented from running if an error occured now runs. A change which results in NOT RUNNING the code which tells an IT pro everything is fine is OK. That is failing safe. A change which STARTS RUNNING code and destroys peoples data is negligent. @nohwnd I'd advise you to make sure you are insured against claims in countries where a contract can not excuse a negligent party from liability.

Anyone running code in prod should be pinning versions and only upgrading production modules after verifying that nothing critical has changed.

Your youth, idealism and nativity and inexperience are showing*. SHOULD is the operative word. Yes they SHOULD. But you know what ? Things work if they don't. 30 years and hundreds of clients tells me where something is optional is usually not done. What you're saying is We are entitled to destroy anything if anyone working with it does not follow all best practices (as we defined them) at all times.

I'm not sure where you get this idea that the tool should be assuming its users are incompetent. If we assumed that, Remove-Item wouldn't even exist as a cmdlet. It would be too dangerous.

(a) You know Barnum's famous “No one ever went broke underestimating the intelligence of the American public.” ? We would like to think that choosing our product was positively correlated with smartness, and thinking our users are dumbasses (negative correlation) is horrible and disrespectful. The [sometimes bitter] experience of 32 years full in IT* , and 3 years part time before that tells me the correlation is zero, not positive or negative. In addition people highly competent in one area sometimes work in areas where they are less competent. And even smart, competent people do stupid things. The worlds worst air accident was the fault of the most highly qualified training-rated pilot in one of the words safest airlines (he actually appear on posters for his airline). In Scuba (where I'm a lapsed instructor) highly competent experienced divers get killed by breaking basic safety rules - the more experienced you are, the worse risk you become. Intelligent, competent and rational people don't always behave that way, and two such people can combine to give a stupid/ incompetent/irrational result.

Earlier this year I deleted a customers data because one of their employees wrote a script which did "Delete from Table where Name -like 'Parameter'" He was the main user of the script and "didn't need to" validate his inputs.
I ran "alice", "bob", "chris". "george", "Phil" | hisScript and everything was deleted. Can you see I've typed a . instead of , after chris. So instead of "chris", "george" I got the george property of the string "chris", which is null. His script became "Delete from Table where Name -like '' ". This guy was in the smartest 5% of customers staff I meet. I've got 13 years of powershell with books, lectures and who knows what behind me, plus >40 years of typing. We should not combine destructively... and yet there we were.

(b) Why do you think Remove-Item with no parameters does not delete everything ? Had you thought about why commands support should process and some default have impactlevel to high? Why do you think guns have safety catches or trains have "dead man's" handles ? Why do think letting go of the controls of a plane means it just keeps going, but taking your foot off the Gas Pedal causes the car to coast to a halt, but . You know the original form of Murphy's law "If it can be done wrongly someone will do it wrongly". Back in the 1980s Schools here in England had little computers called the "BBC Micro". The OS on them had command to delete files del. filename. I worked for the main company replacing these with the MS-DOS based machines. One customer discovered the commands no longer began with a so used Del. filename. DEL. is treated as Del . (i.e. current directory) and later parameters are ignored. I can't remember now if Dos 3.1 didn't asks "Are you sure" or if the BBC micro always asked for confirmation of deleting a file so the customer replied Yes he was sure. Either way he deleted one key directory and then another to confirm what he had just seen.

Pester isn't designed to think for it's users. It is not entitled to think against them. And that is saying that it exists with total disregard for the people who use it. @nohwnd Do you have total disregard for all users ?

This isn't about Pester thinking for users or not. It exists it does things, people decide if it meets their needs or not. The moment it went on line (like any other shared software) its creators lose control of who uses it and how. Whether it guides the user a bit better or bit worse isn't actually the point at issue. The point is are people relying an existing behaviour as a safety mechanism. Yes they are. Neither you nor I know how many are, or what damage might result from removing that safety mechanism. Remember at all times please, this is not a script which should run failing to run, it is runaway code doing things it should have been stopped from doing.

Please don't just hamper improvement to modules based on a devil's advocate worst-case scenario which assumes [people don't always do stuff right] ....

I'm not hampering anything. There is a beneficial, desirable change. It needs to be introduced in a safe way. What you see as "devil's advocate, worst-case scenario" is what those with a bit more experience call 'the real world' . Your plea not to be hampered is actually a plea to be allowed to abdicate any responsibility. You would presumably design a gun with no safety catch because everyone knows not to go the near the trigger (which has a guard on it) unless you mean to fire the weapon. Ultimately Stuff should be done. Doing Stuff has risks. Risks should be mitigated.

• Sorry to play the old man card, but twitter says you were born in 1995. I was certified in NT in 1993. I'll probably have retired by the time you really get some of this stuff.

[vexx32]

@jhoneill As I said, I recognise your opinion is different. Let's not derail this topic further; the point of whether or not breaking changes were going to be made seems to have been decided well before this topic was opened. 🙂

I'm sorry to hear that you've had some difficult experiences. I simply don't agree that those bear any relation to how a project like Pester should be maintained. Ultimately, the only way to really ensure nothing changes is to pin your versions. Otherwise, something will always break.

I very much prefer @nohwnd's approach here, being communicative and interacting with the community. The alternative is oftentimes to make smaller changes over a period of time, which will break just as many things, but is much harder to communicate.

I'll ask that you not derail the discussion with overly personal arguments, if you don't mind. I've no desire to see things get ugly here. 😊

[jhoneill]

@jhoneill I understand your arguments, and Pester is keeping backwards compatibility where possible (v4 still supports PowerShell v2 for example.) I am also afraid of making breaking changes and that is why I am using semantic versioning, maybe not strictly, but definitely for a big breaking changes.

That's all good. Breaking changes are necessary, tell people they can have progress, compatibility or freedom of choice and to be thankful if they get two. But please make a distinction between "My code does not run after this change" which has the answer "Fix it like this, or install the old version from " and "My code ran something extra after this change and now I need a new job" which does not have a good answer.

That said I am still sure that it will come to many people as a surprise and there will be many threads looking for reasoning for the changes, and calling for them to be reverted. But that is sadly the life of a project maintainer ...

Yes ! :-) You know the acronym "WYSIWYG" (what you see is what you get) a friend of mine coined "WIKIWIL" What I know is what I like. Whatever you change someone will say "I preferred it before" or "What idiot is in charge who changed X". Somewhere you added the Pester logo when invoke-pester runs I don't much care for. If you take it away again to please me, we both know you would get more complainers than happy people.
In the end you have to trust your own idea of what is right, and introduce it in the best way for your user base. This case is unusual because the change can result in extra things running which should not; therefore it should have one-time opt-in to the new behaviour. Since you provide an opt-out anyway this is quite easy to do. If the user has never opted in, or if they have opted out for this run, use the old behaviour. If the user has opted-in at any point and didn't opt out for this run , use the new behaviour. That way you get the benefits in safe way and those who didn't know they needed the old behaviour are protected.

I've have additional ideas about Should (and using MUST or MIGHT), and (of course) I think those are good ideas, but I also know that you have more knowledge of Pester, its users, and what they do than anyone else - especially me, I have my use and the use of a handful of clients which is tiny by comparison. I will back your right to make decisions on what to introduce. But I'll also weigh-in doing it safely.

[jhoneill]

@vexx32
Assuming your twitter age is right, I think the difference in opinion is down to the difference in age. I hope it's not become personal. Reading your words as calling me some kind of ancient Luddite, blocking all change would not help anything and I've chosen not to. Reading mine as calling you whatever the opposite is ... equally possible and I hope you have not.

I've been young (and known, with certainty, that there were Right Things To Do and all the reasons for not doing things that way were Bad Reasons), but you haven't yet been old (with the the doubt that any way is much less wrong, and experience of seeing bad reasons behind more decisions good ones). Which makes it easier for me to understand the p.o.v you express (I can remember having it) than for you to understand mine.

For clarity. I've come think of my typical user as someone moderately smart, under time-pressure and prone to human error. If I make possible for to do something wrong, sooner or later someone will; and as the person releasing software knows more about it than the person using it some responsibility rests on the me, to foresee negative consequences and make the grosser ones hard to get to. Remove-Item could run without a path and assume "everything", but elementary design says don't do that. Asking "Are you sure" every time just means people automatically say "yes" even when they tab completed the wrong name. So we get to the design we have. Someone can change the meaning by typing a . for a , which creates null which combines with a * to delete every and there isn't much the language authors can do...

For, I hope the last time: This is not a breaking change. What ran before will run afterwards. The problem is what was prevented from running will now run. Anything might happen, but neither you nor I nor @nohwnd has much idea what will. I worry about code which says "Exit if variable is not initialized" will not now exit and a later step selects everything for a change as a result - it's the kind of code I may have left behind me. (It's OK I carry £2 million in liability insurance if a court doesn't agree that the client should have seen the version change and what would result :-) ).

On the safety of introducing specific changes in specific ways we do seem to represent the two poles. And that's OK. One voice saying "This is great - you/we should do it" and another saying "But be careful of..." is a way to make good progress - enthusiasm without experience is dangerous, experience without enthusiasm gets nothing done. Strong project groups have people who bring different perspectives - diversity of age or anything else isn't just about ideas of fairness, it brings results. I don't mind being at one pole and finding people don't agree with me - they have something more to interesting to say than "I agree", although that isn't always helpful

[nohwnd]

This is a good discussion, maybe we can have it opt-in rather than opt-out. That will give us nice transition to v5 without yet another breaking change, and the new behavior will be easy to enable via configuration. I am sure that people who would find it beneficial will be able to do that, while opting-in will be less destructive when others will let v5 hit them unprepared.

Right now I am structuring the configuration to be provided in multiple ways, as $PesterPreference variable, as -Configuration parameter to Invoke-Pester and in the future maybe $PesterPreference per file, or even per Describe / It, not sure, I don't want to introduce too much overhead right now.

The migration per file will be still possible, you can either enable it per file (or even per scope) using the default parameters (but with 'Continue' instead of 'Stop'), or you can enable it everywhere and only disable it where needed.

---

@jhoneill I am not trying to take cheap shots to shut down your arguments, but you should stop using the legacy syntax for your code, it will not be present in Pester v5 and will throw, see your code won't run aftewards and you are protected! 😁 Here is a migration guide if you are interested: https://pester.dev/docs/migrations/v3-to-v4#update-to-the-new-assertions-syntax

[jhoneill]

@jhoneill I am not trying to take cheap shots to shut down your arguments, but you should stop using the legacy syntax for your code, it will not be present in Pester v5 and will throw, see your code won't run aftewards and you are protected! 😁 Here is a migration guide if you are interested: https://pester.dev/docs/migrations/v3-to-v4#update-to-the-new-assertions-syntax

You see what I mean about not everyone keeping up to date ? :-) Certainly a lot of the test we use for import-excel pre-date V4 and won't have been updated, and I think doug and I have just kept writing in the same old way. I'm now thinking did I ever read the notes when the version changed, or did I do that harassed IT-Pro thing of "Of course the new version will work like the old one". I admired the original form and hated for not being "proper" PowerShell. Eventually I embraced it, now I'm building up technical debt around it.... But not wishing to beat myself up too much most things I download are also using the old syntax.

[jhoneill]

@nohwnd I have to share this which makes me laugh, although since it is heading for 2 in the morning, I'm not really laughing. I made sure I was using the new syntax, Runs fine on my machine, And what is installed in Azure Devops. ?

Based on the path that's 3.4.0 . So using the new syntax I also need to be sure to update the pester version. If they skip 4 and upgrade to 5 anyone they won't get a transitional version. For people who aren't requesting a version this will be a problem -so ADO may never upgrade.

[vexx32]

@jhoneill if they're never upgrading, they can't possibly be factored into any decisions about new code by definition. 😂

[nohwnd]

@jhoneill that's most likely because Pester was shipped as part of windows 10, and still is? They won't upgrade that version and won't take it away either (because of compatibility). What host are you running this on?

[jhoneill]

@nohwnd What Azure devops host - " vmImage: 'windows-latest' "

@jhoneill if they're never upgrading, they can't possibly be factored into any decisions about new code by definition. 😂

Indeed.
But you forget the reverse is also true. If you don't factor them into your decision making, they won't upgrade.

[nohwnd]

@Jaykul FYI the default behavior for Should is now back to Stop but I am forcing Should in Mock parameter filter to fail irrespective of the settings in here #1421.