Suggested fix for #1173 with suitable tests

[gnuechtel]

Fixes issue #1173 and adds tests for expected step results

[Jaykul]

I really dislike code that generates tests -- it's extremely hard to tell what these tests prove, if anything...

[nohwnd]

I think we forgot to add test exclusions to the appveyor build. The style/help/whitespace checks should run only when I try releasing. Can’t check right now.

[nohwnd]

I am reacting to the other comment :/

[Jaykul]

What?

[gnuechtel]

I really dislike code that generates tests -- it's extremely hard to tell what these tests prove, if anything...

You are right.
That code is a little bit too dynamic.
The test logic must be simpler.
I will replace the function by simple hash tables later...

My goal was to create a data-driven test

[gnuechtel]

@Jaykul I changed test code and think now it is more readable

[Jaykul]

Yep, that looks fine now. Honestly, you could probably shorten it to just count the PassedScenarios, the way the tests at the top of the file do, but I don't mind either way. This is clear and looks correct to me.

[nohwnd]

@cgnuechtel I upgraded your branch to the latest master because I changed how builds are done. It failed on PowerShell 2 because PowerShell 2 does not automatically expand properties on arrays. Calling .Result on an array fails because array does not have a property called Result (unlike in PowerShell 3+, where it looks for .Result on all items in the array). (There is now fail on using PSJob after it was disposed, it happens occasionally, I will be fixing that in a different branch and update this to current master when I am done.)

[gnuechtel]

@nohwnd Thanks, but Set-ItResult does not yet have a File parameter!?

[nohwnd]

@cgnuechtel I see it builds now, is this still work in progress or done?

[gnuechtel]

@nohwnd I am ready here, but please look at Set-ItResult.ps1 as I added some parameters. However, it should be a non-breaking change.

[nohwnd]

@cgnuechtel thanks for the heads up, I think using the Set-ItResult directly was not a good idea (I know I changed your code to do that, so it's my bad.) I will refactor to make it call an internal function that creates the error. The public Set-ItResult should not be able to define anything except for status and reason.

[gnuechtel]

@nohwnd Please look on this code. I am not sure, if this change is correct.

I am also not sure if there are missing tests here. The Gherkin tests I have been added check the NUnit XML output and the inconclusive message, but also some error stuff. If this is too confusing, feel free to remove these tests.

[nohwnd]

@cgnuechtel it was the correct change to make I think, but not for the Set-ItResult there when the failure has a known type I want to show a custom message build from the because string. There are no tests for it because it's very close to the output I guess. Now it should be working as expected.

PS C:\projects\pester_main> get-module pester | remove-module ; import-module .\Pester.psd1; describe "a"{ it "b" { set-itresult -inconclusive  }}

Describing a

Hit Line breakpoint on 'C:\projects\pester_main\Functions\Output.ps1:286'

[DBG]: PS C:\projects\pester_main>
  [?] b, is inconclusive 67ms
PS C:\projects\pester_main> get-module pester | remove-module ; import-module .\Pester.psd1; describe "a"{ it "b" { set-itresult -inconclusive -Because "abc" }}

Describing a

[DBG]: PS C:\projects\pester_main>
  [?] b, is inconclusive, because abc 73ms

Hope I did not break anything else.

[nohwnd]

All tests pass, merging, if it does not work we should prove it wrong with broken tests.