pester / Pester

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

Should Be/BeExactly do not assert type #328

Closed dfch closed 9 years ago

dfch commented 9 years ago

When using should to assert values, the data type is not checked leading to false test results.

False Negative

Describe -Tags "Error in Comparisons" "Tests that should fail, but succeed" {

    # not the same, but still succeeds
    It "ShouldBe-IntegerString" {
        3 | Should Be '3';
    }

    # not the same, but still succeeds
    It "ShouldExactlyBe-IntegerString" {
        3 | Should BeExactly '3';
    }

    # not the same, but still succeeds
    It "ShouldBe-StringInteger" {
        '3' | Should Be 3;
    }

    # not the same, but still succeeds
    It "ShouldExactlyBe-StringInteger" {
        '3' | Should BeExactly 3;
    }

}

Describing Tests that should fail, but succeed
 [+] ShouldBe-IntegerString 60ms
 [+] ShouldExactlyBe-IntegerString 16ms
 [+] ShouldBe-StringInteger 16ms
 [+] ShouldExactlyBe-StringInteger 36ms

False Positive

In addition, these test fail, though they should actually succeed:

Describe -Tags "Error in Comparisons" "Tests that should succeed, but fail" {

    # not the same, but still fails
    It "ShouldNotBe-IntegerString" {
        3 | Should Not Be '3';
    }

    # not the same, but still fails
    It "ShouldNotExactlyBe-IntegerString" {
        3 | Should Not BeExactly '3';
    }

    # not the same, but still fails
    It "ShouldNotBe-StringInteger" {
        '3' | Should Not Be 3;
    }

    # not the same, but still fails
    It "ShouldNotExactlyBe-StringInteger" {
        '3' | Should Not BeExactly 3;
    }

}

Describing Tests that should succeed, but fail
 [-] ShouldNotBe-IntegerString 78ms
   Expected: value was {3}, but should not have been the same
   at line: 29 in C:\Github\Comparison.Tests.ps1
   29:                  3 | Should Not Be '3';
 [-] ShouldNotExactlyBe-IntegerString 21ms
   Expected: value was {3}, but should not have been exactly the same
   at line: 34 in C:\Github\Comparison.Tests.ps1
   34:                  3 | Should Not BeExactly '3';
 [-] ShouldNotBe-StringInteger 18ms
   Expected: value was {3}, but should not have been the same
   at line: 39 in C:\Github\Comparison.Tests.ps1
   39:                  '3' | Should Not Be 3;
 [-] ShouldNotExactlyBe-StringInteger 23ms
   Expected: value was {3}, but should not have been exactly the same
   at line: 44 in C:\Github\Comparison.Tests.ps1
   44:                  '3' | Should Not BeExactly 3;

Correct comparison

Only when you specifically supply the type, the comparison is done correctly (i.e. the tests fail):

Describe -Tags "Correct Comparisons" "Tests that fail as expected" {

    # not the same, fails as expected
    It "ShouldBe-TypedIntegerString" {
        {[int] 3} | Should Be ([string] '3');
    }

    # not the same, fails as expected
    It "ShouldBe-TypedStringInteger" {
        ([string] '3') | Should Be {[int] 3};
    }

}

Describing Tests that fail as expected
 [-] ShouldBe-TypedIntegerString 104ms
   Expected: {3}
   But was:  {[int] 3}
   at line: 53 in C:\Github\Comparison.Tests.ps1
   53:                  {[int] 3} | Should Be ([string] '3');
 [-] ShouldBe-TypedStringInteger 53ms
   Expected: {[int] 3}
   But was:  {3}
   at line: 58 in C:\Github\Comparison.Tests.ps1
   58:                  ([string] '3') | Should Be {[int] 3};

In my opinion the behaviour should be similar to the behaviour of the [Microsoft.VisualStudio.TestTools.UnitTesting.Assert] class where you have specific AreEqual methods that also compare the given data type:

PS > [Microsoft.VisualStudio.TestTools.UnitTesting.Assert]::AreEqual

OverloadDefinitions
-------------------
static void AreEqual(System.Object expected, System.Object actual)
static void AreEqual[T](T expected, T actual)
static void AreEqual(double expected, double actual, double delta)
static void AreEqual(float expected, float actual, float delta)
static void AreEqual(System.Object expected, System.Object actual, string message)
static void AreEqual(string expected, string actual, bool ignoreCase)
static void AreEqual[T](T expected, T actual, string message)
static void AreEqual(double expected, double actual, double delta, string message)
static void AreEqual(float expected, float actual, float delta, string message)
static void AreEqual(System.Object expected, System.Object actual, string message, Params System.Object[] parameters)
static void AreEqual(string expected, string actual, bool ignoreCase, cultureinfo culture)
static void AreEqual(string expected, string actual, bool ignoreCase, string message)
static void AreEqual[T](T expected, T actual, string message, Params System.Object[] parameters)
static void AreEqual(double expected, double actual, double delta, string message, Params System.Object[] parameters)
static void AreEqual(float expected, float actual, float delta, string message, Params System.Object[] parameters)
static void AreEqual(string expected, string actual, bool ignoreCase, cultureinfo culture, string message)
static void AreEqual(string expected, string actual, bool ignoreCase, string message, Params System.Object[] parameters)
static void AreEqual(string expected, string actual, bool ignoreCase, cultureinfo culture, string message, Params System.Object[] parameters)
nohwnd commented 9 years ago

The Should Be assertion uses -eq operator internally and inherits all it's behavior including everything that might seem like a quirk. PowerShell (and it's users) usually do not care about the types much, and there are well defined rules to which the operators (and hence the assertions) comply. So the assertions work as designed.

Your examples of "false positive" and "false negative" show what I would expect to see: comparing string 3 to integer 3 results in them being equal.

In the rest of your examples you use ScriptBlocks to specify the type, but you use it incorrectly and what is actually happening is that instead of integer 3 you get back string [int] 3 as in this example:

3 -eq "[int] 3" 

which is false. To correct the code you'd have to call it as such:

([string] '3') -eq ([int] 3); #use () instead of {} 

But that again results in true.

Do you have a real life use case where you need to differentiate between the two types of comparison?

Maybe #327 might help you?

dfch commented 9 years ago

Hi @nohwnd thanks for the clarification on the implementation and correct usage of the script block.

I had the use case when encapsulating a third party REST API where the return data came back as JSON which we processed and returned. There it made sense to return correct data types and properly check for them (in the end this is what unit tests are made for in my opinion).

At least for BeExactly I would have expected an exact comparison, otherwise it is a little bit misleading and BeCaseSensitive (or something else) might be more appropriate.

And yes, you are probably right, that not all PowerShell users might be too interested in this. However, all the testing frameworks I used before always supported this (maybe that's why I am caring about this at all).

But if you think this is not adding a value to the framework, feel free to close the issue. Regards, Ronald

Jaykul commented 9 years ago

It would, of course, be a total disaster to change this behavior now.

https://github.com/pester/Pester/issues/334