fix formatting for Nunit parameterrized tests

[alexbuit]

1. General summary of the pull request

Fix #1573
Fix #1612

It will replace the Parameter tokens in the test names with the parameter values if provided. If no replacement has taken place, the current behavior will take place (put all parameters cast into [string] after the test name.

The current behavior breaks on test builds we are running with parametersets containing a huge array of hash tables, as they will produce test names with "System.Collections.Hashtable System.Collections.Hashtable ..." at the end, making both the result file very big and break the parser eventually. (apart from the fact it makes the results unreadable)

[nohwnd]

I think there already is a property on the test object that has the expanded name, I think it's called DisplayName

[alexbuit]

If I run the following, I do not see the parameterized SomeItem and the current problem we face is that the nunit output that is now generated actually blows up our bamboo build (as it uses the description field to name the tests in it's output, and having test names that are a 23.528 characters blows up completely (and also in a very unpleasant way, the build actually gets stuck without error) Although the latter is a bug in Bamboo, we are causing it by submitting test results in a unclean way, so I would rather fix the input then the parser. (I kept the about of hashtables in the example below limited to 11, but you can imagine what it would look like when applying this to our production build running on way bigger data sets.

$testparam = @(
    @{
        SomeItem = 'bla'
        SomeItemArray = 0..10 | ForEach-Object { @{SomeKey = "bla_$_"} }
    }
)
Describe 'a' {
    It 'great <SomeItem>' {
        Param (
            [string]$SomeItem,
            [hashtable[]]$SomeItemArray
        )
        $SomeItemArray[0].SomeKey | Should -be 'bla_0'
        $SomeItem | Should -be 'bla'
    } -TestCases $testParam
}

Results in:

<test-results xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="nunit_schema_2.5.xsd" name="Pester" total="1" errors="0" failures="0" not-run="0" inconclusive="0" ignored="0" skipped="0" invalid="0" date="2020-07-20" time="10:34:57">
  <environment clr-version="4.0.30319.42000" user-domain="DESKTOP-9UMGMK5" cwd="C:\repo\Pester\src\functions" platform="Microsoft Windows 10 Pro|C:\WINDOWS|\Device\Harddisk0\Partition4" machine-name="DESKTOP-9UMGMK5" nunit-version="2.5.8.0" os-version="10.0.18363" user="alexb" />
  <culture-info current-culture="en-NL" current-uiculture="en-US" />
  <test-suite type="TestFixture" name="Pester" executed="True" result="Success" success="True" time="0.0655" asserts="0" description="Pester">
    <results>
      <test-suite type="TestFixture" name="C:\temp\bla2.tests.ps1" executed="True" result="Success" success="True" time="0.0655" asserts="0" description="C:\temp\bla2.tests.ps1">
        <results>
          <test-suite type="TestFixture" name="a" executed="True" result="Success" success="True" time="0.0394" asserts="0" description="a">
            <results>
              <test-suite type="ParameterizedTest" name="a.great &lt;SomeItem&gt;" executed="True" result="Success" success="True" time="0.0173" asserts="0" description="great &lt;SomeItem&gt;">
                <results>
                  <test-case description="great &lt;SomeItem&gt;" name="a.great &lt;SomeItem&gt;(&quot;bla&quot;,System.Collections.Hashtable System.Collections.Hashtable System.Collections.Hashtable System.Collections.Hashtable System.Collections.Hashtable System.Collections.Hashtable System.Collections.Hashtable System.Collections.Hashtable System.Collecti
ons.Hashtable System.Collections.Hashtable System.Collections.Hashtable)" time="0.0173" asserts="0" success="True" result="Success" executed="True" />
                </results>
              </test-suite>
            </results>
          </test-suite>
        </results>
      </test-suite>
    </results>
  </test-suite>
</test-results>

[nohwnd]

Okay, I just want to make sure that the output to screen matches the output to file, so we shoukd only convert the name to DisplayName once, and use it in all the places where it is reported. Maybe the display name converter needs fixing instead?

[alexbuit]

committed a new version where the $test.ExpandedName and (new generated property) $test.ExpandedPath are used, is this the direction you were thinking?

[alexbuit]

Looks like one of the builds had a bad timing issue:
https://nohwnd.visualstudio.com/Pester/_build/results?buildId=984&view=logs&j=ff75994e-ad5b-5649-588c-530e2c306b35&t=bb64baaf-a3d6-5598-2b44-c9bedcc4901a&l=1670
Reading the logs it should just have taken only a few seconds, but it took way longer, about 7 seconds, (also it's discovery phase, which took 11 seconds)

I will push the code with a minor change to re-trigger the builds to verify if a second run will fix the issue...

[nohwnd]

To me it feels like we should drop the entire part where the parameters are beeing parsed into the description, but I did not yet do that, as that will change behaviour for tests that do not have parameter tags in their test names

which part would be that? and what would be the impact?

[alexbuit]

I left the code below in the NUnit output generation, but it will only kick in when there are no tags in the testname, removing this would mean parameterized tests that do not have tags will all just be named exactly the same.

The alternative is in PR #1089 where the parameters are passed in as a seperate XML property into the NUnit file, thus the information is still available, but it will not change the testname.

starting from this line:
https://github.com/pester/Pester/pull/1630/files#diff-9d06394664e89fba6b48c05ff5001458R597

 # todo: this comparison would fail if the test name would contain $(Get-Date) or something similar that changes all the time
    if ($testName -eq $ParameterizedSuiteName) {
        $paramString = ''
        if ($null -ne $TestResult.Data) {
            $params = @(
                foreach ($value in $TestResult.Data.Values) {
                    if ($null -eq $value) {
                        'null'
            $paramsUsedInTestName =$false

            if (-not $paramsUsedInTestName) {
                $params = @(
                    foreach ($value in $TestResult.Data.Values) {
                        if ($null -eq $value) {
                            'null'
                        }
                        elseif ($value -is [string]) {
                            '"{0}"' -f $value
                        }
                        else {
                            #do not use .ToString() it uses the current culture settings
                            #and we need to use en-US culture, which [string] or .ToString([Globalization.CultureInfo]'en-us') uses
                            [string]$value
                        }
                    }
                    elseif ($value -is [string]) {
                        '"{0}"' -f $value
                    }
                    else {
                        #do not use .ToString() it uses the current culture settings
                        #and we need to use en-US culture, which [string] or .ToString([Globalization.CultureInfo]'en-us') uses
                        [string]$value
                    }
                }
            )
                )

            $paramString = "($($params -join ','))"
                $paramString = "($($params -join ','))"
                $testName = "$testName$paramString"
            }
        }
    }

[alexbuit]

We can merge this PR as is, I will create a seperate PR to get this idea clarified, maybe that will be easier

[alexbuit]

I still need to write tests for that change, but the draft looks like this (compared to this PR's version)
https://github.com/alexbuit/Pester/compare/nunitTestcases...alexbuit:NunitTestParameters?expand=1

[nohwnd]

Thanks, please do create another PR for that :)

[nohwnd]

The code looks good, is it ready to merge or is there more work to be done?

[bielawb]

I spoke with @alexbuit before he went on vacation and got confirmation the code is ready. Thanks!