pester / Pester

Pester is the ubiquitous test and mock framework for PowerShell.
https://pester.dev/
Other
3.11k stars 473 forks source link

Request for throw to be a case insensitive match #577

Closed pauby closed 7 years ago

pauby commented 8 years ago

The throw condition is a case sensitive match on any string you pass to it. For example:

{ throw "Luke Skywalker" } | Should throw "luke skywalker"

Fails. Whereas:

{ throw "Luke Skywalker" } | Should throw "Luke Skywalker"

Passes.

When I'm writing text to catch for throw's I just write in lowercase rather than having to make sure the case is exactly correct. The point would be to catch the error text rather than worrying about what case it is and it would be more convenient if Pester matched the text in the exception message regardless of case.

The problem seems to be in the Functions\Assertions\PesterThrow.ps1 file:

function Get-DoMessagesMatch($value, $expected) {
    if ($expected -eq "") { return $false }
    return $value.Contains($expected)
}

Which uses the case sensitive .Contains() which could be changed to -contains whcih is case insensitive:

function Get-DoMessagesMatch($value, $expected) {
    if ($expected -eq "") { return $false }
    return $value -contains $expected
}

I've verified that this works as I expected.

dlwyatt commented 8 years ago

The -contains operator doesn't quite do the same thing as String.Contains(), but there are other ways we can make that case insensitive. I'll work it into the v4 branch.

On Jul 13, 2016, at 1:50 PM, Paul Broadwith notifications@github.com wrote:

The throw condition is a case sensitive match on any string you pass to it. For example:

{ throw "Luke Skywalker" } | Should throw "luke skywalker" Fails. Whereas:

{ throw "Luke Skywalker" } | Should throw "Luke Skywalker" Passes.

When I'm writing text to catch for throw's I just write in lowercase rather than having to make sure the case is exactly correct. The point would be to catch the error text rather than worrying about what case it is and it would be more convenient if Pester matched the text in the exception message regardless of case.

The problem seems to be in the Functions\Assertions\PesterThrow.ps1 file:

function Get-DoMessagesMatch($value, $expected) { if ($expected -eq "") { return $false } return $value.Contains($expected) } Which uses the case sensitive .Contains() which could be changed to -contains whcih is case insensitive:

function Get-DoMessagesMatch($value, $expected) { if ($expected -eq "") { return $false } return $value -contains $expected } I've verified that this works as I expected.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

nohwnd commented 8 years ago

You scared me there a little, because I thought that all tests passed after you made your changes, but luckily the tests fail.

pauby commented 8 years ago

@dlwyatt In what way are they different?

I noticed you changed it to -like - in what way are they more the same? :)

nohwnd commented 8 years ago

@pauby Powershell -contains operator is used with collections not with strings. You use it to see if your item is present in an array of items. For strings you need to use -like operator.

"abc" -contains "a" # is false
"a", "b", "c" -contains "a" # is true

"abc" -like "*a*" # is true

(ehm disregard the commits that are not present in the PR, I got into some weird state and now my commits are orphaned. One should not force push.)

pauby commented 8 years ago

@nohwnd In my haste yesterday to find a case insensitive version of Contains() I fell upon an article by The Scripting Guys. Again, in my haste, I didn't bother reading the examples very clearly where they are using arrays and -contains. I saw STRINGS and I tried it... I'm a bloke. I tend not to read instructions and just use them as a guideline :)

-contains is an operator I've never had call to use yet!

So thanks for the clarification. And thanks for updating that so quickly.

nohwnd commented 8 years ago

@pauby Well it's not merged yet and it's going in the version 4 anyway, so you will have to wait for that change for a while.