pester / Pester

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

Should Not Match assertion across multiple lines. #1058

Closed ksivasenthil closed 6 years ago

ksivasenthil commented 6 years ago

1. Provide a general summary of the issue in the Title above

Should Not Match does not assert correctly across multiple lines. Please review https://github.com/pester/Pester/wiki/Contributing-to-Pester prior to submitting an issue. - [x] Reviewed

2. Describe Your Environment

Operating System, Pester version and PowerShell version:

$bugReport = &{
    $p = Get-Module -ListAvailable Pester
    "Pester version     : " + $p.Version + " " + $p.Path
    "PowerShell version : " + $PSVersionTable.PSVersion
    "OS version         : " + [System.Environment]::OSVersion.VersionString
}
$bugReport
$bugReport | clip

Pester version : 3.4.0 C:\Program Files\WindowsPowerShell\Modules\Pester\3.4.0\Pester.psm1 PowerShell version : 5.1.17134.48 OS version : Microsoft Windows NT 10.0.17134.0

3. Expected Behavior

If you're describing a bug, tell us what should happen. [x] Yes

I have put the code in Current Behavior. The assert "Should Not Match" should fail.

If you're suggesting a change/improvement, tell us how it should work. Especially what the proposed feature is, why it is useful, and what dependencies (if any) it has. It would also be great if you added one or two examples of real world usage, if you have any. [x] No

4.Current Behavior


$FileListWithMultipleLineCommented = @(
    @{
            "Path" = "HelloWorld.js";
            "CommentToken" = "/\*(.*)?\*/"
    }
);

$MultiLineCommentFileConent1=@"
                                /*This is Hello World comment;
                                Spanning multiple lines */
                                function HelloWorld() {
                                    //No ref;
                                }
"@;

    Describe -Name 'Remove comments' -Description "Validate removal of  comment blocks from the code." -Fixture {
    Context -Name 'Multiline comments' -Fixture {
        It 'Comments should be removed' -TestCase $FileListWithMultipleLineCommented {
            param($Path, $CommentToken)
            Mock Get-Content -Verifiable -MockWith { return $MultiLineCommentFileConent1;}

            <#The following assert should fail because the -replace operator does not have anything to replace with. Thus the original string will be returned and the Should Not Match with the same pattern should fail; since the string does still contain all the multi line comment block.#>

            (Get-Content -Path $Path) -replace $CommentToken | Select-Object -Property @{n="File";e={$Path}}, @{n="Content";e={$_}} | Select-Object -ExpandProperty Content | Should Not Match $CommentToken
        }
    }
}

5. Possible Solution

Have a solution in mind? [x]Yes

https://github.com/pester/Pester/wiki/Contributing-to-Pester has detailed instructions on how to contribute. [x] Reviewed

Suggestion

Functions/Assertions/Match.ps1@2 has -match operator. Instead of that operator can we use Select-String with AllMatches switch enabled?

6. Context

How has this issue affected you? What are you trying to accomplish?

I was trying to remove multiple lines of comment from a code base. While doing this I ventured into Pester to test my commands and bumped into this problem. So, actually all my tests will succeed because of the Should Not Match not looking across multiple lines.

it-praktyk commented 6 years ago

Please update Pester to version 4.3.1 and verify that an issue exists in the newest stable version also.

ksivasenthil commented 6 years ago

I have updated to the latest version of 4.3.1. I still have the same issue as narrated before. The tests report a pass in that specified scenario where they should have reported a fail.

ksivasenthil commented 6 years ago

I accidentally closed this. Pardon me :(

mikeclayton commented 6 years ago

@ksivasenthil - i modified your test slightly to output some temporary value as follows:

        It 'Comments should be removed' -TestCase $FileListWithMultipleLineCommented {
            param($Path, $CommentToken)

            Mock Get-Content -Verifiable -MockWith { return $MultiLineCommentFileConent1;}

            <#The following assert should fail because the -replace operator does not have anything to replace with. Thus the original string will be returned and the Should Not Match with the same pattern should fail; since the string does still contain all the multi line comment block.#>

            $result = (Get-Content -Path $Path) -replace $CommentToken `
                          | Select-Object -Property @{n="File";e={$Path}}, @{n="Content";e={$_}} `
                          | Select-Object -ExpandProperty Content

            write-host "result = '$result'";
            write-host "token = '$CommentToken'";

            $result | Should Not Match $CommentToken

        }

I got this output:

Executing script C:\src\scratch\pester\1058.ps1

  Describing Remove comments

    Context Multiline comments
result = '                                /*This is Hello World comment;
                                Spanning multiple lines */
                                function HelloWorld() {
                                    //No ref;
                                }'
token = '/\*(.*)?\*/'
      [+] Comments should be removed 103ms

which looks to be correct as "result" does not match "token".

You might want to double-check what your test is trying to assert...

ksivasenthil commented 6 years ago

@mikeclayton Thank you. I noticed a problem with the regex. The correct regex pattern should be "(?s)/*(.)?/" in my test. Since, this is not an issue with pester I am closing the issue.