Improve BeTrue assertion

t3mi commented 5 years ago

1. General summary of the issue

I understand that examples specified below is how PowerShell works but it's not so obvious what assertion to use to make a proper check against the $true value even from the Should page. It would be helpful at least for BeTrue assertion to have a proper compare check.

Describe "Check if Truthy" {

    It "False as a string" {
        'False' | Should -Be $true
    }

    It "False as a string" {
        'False' | Should -BeTrue
    }

    It "Any string" {
        'Whatever' | Should -Be $true
    }

    It "Any string" {
        'Whatever' | Should -BeExactly $true
    }

    It "Any string" {
        'Whatever' | Should -BeTrue
    }
}

2. Describe Your Environment

Pester version     : 4.8.1 C:\Program Files\WindowsPowerShell\Modules\Pester\4.8.1\Pester.psd1
PowerShell version : 5.1.17763.316
OS version         : Microsoft Windows NT 10.0.17763.0

3. Expected Behavior

Tests must fail.

4.Current Behavior

Tests passed.

renehernandez commented 5 years ago

-BeTrue assertion will check for truthy equality, which as described here works not only for boolean values, but also for:

Strings, as long as they aren't zero length string
Any number different than 0
Any non-zero length array
Any non-null object

That left us with the 3 remaining cases that we may need to analyze:

Describe "Check if Truthy" {

    It "False as a string" {
        'False' | Should -Be $true
    }

    It "Any string" {
        'Whatever' | Should -Be $true
    }

    It "Any string" {
        'Whatever' | Should -BeExactly $true
    }
}

By looking at the code, it seems to be that this is due to the way the comparison are being performed within the -Be and -BeExactly assertions, which use -eq \ -ceq operators and it is comparing the expected value ($true) on the left-hand side against the received value (strings objects) on the right-hand side. Due to this, -eq \ -ceq will coerce the strings into a boolean value and since they aren't empty it will coerce them to the $true value, thus ending up performing a truthy comparison as well :).

renehernandez commented 5 years ago

Fixing this may affect people that are already relying on this unexpected behavior, so this could possible be a breaking change.

@nohwnd Any comments?

nohwnd commented 5 years ago

Yes, it would be a breaking change so it would need to be done in Pester v5. I want to re-visit the asseritons in v5 and I am now deciding if going in the direction of many breaking changes and better future behavior is better than keeping v5 more compatible to allow people to migrate to it.

renehernandez commented 5 years ago

For what it matters, I would vote to improve the future behavior and not be too concerned with the breaking changes. We are already doing breaking changes in v5, so I think we should leverage it as a relatively clean slate to revisit, fix and change as much as possible. Within reason of course :)

nohwnd commented 5 years ago

I think that as well. I will be talking about exactly this with the community on psconf and post summary afterwards so others can speak to it as well :)

nohwnd commented 4 years ago

Described here a possible way forward for should, would keep the new functionality specification for that change #1423

marko-stanojevic commented 3 years ago

Is this going to be addressed? Still present in 5.2.2

nohwnd commented 3 years ago

@marko-stanojevic I hope at some point. I wanted to move my Assert module into Pester for a long time and do more assertion related stuff, but unfortunately there were other things to focus on in 5.3 and previous releases, and there is also a blocking issue with dynamic parameters being limited to 32 parameter sets only in powershell.

nohwnd commented 6 months ago

Will be fixed by #2428 where we have separate assertions for False, Falsy, True and Truthy.

pester / Pester