Path overhaul #272
Path overhaul #272
Conversation
This is a first crack at resolving issues pester#270 and pester#271 . I haven't written any tests around this new code yet, but it does successfully run all of Pester's existing tests (except for those looking specifically for the -Path parameter on New-PesterState; that's been removed since nothing but Invoke-Pester was actually looking at it.)
Testing the logic that's actually in Invoke-Pester is awkward, but I've manually verified that passing hashtables to the -Path parameter results in the proper splatting of the Arguments (for positional parameters) and/or Parameters (for named parameters) values to the script.
| @@ -221,6 +230,85 @@ about_pester | |||
| if ($EnableExit) { Exit-WithCode -FailedCount $pester.FailedCount } | |||
| } | |||
|
|
|||
| function ResolveTestScripts | |||
| { | |||
| param ([object[]] $Path) | |||
There was a problem hiding this comment.
Should we rename $Path to something different to represent that it may contain both path and arguments? Maybe create a separate Parameter set for it
There was a problem hiding this comment.
I'm okay with that, but we'd need to keep a Path alias to maintain backward compatibility. Maybe something like -Script or -TestScript?
|
What do you mean by "add line new line"? |
|
I'm talking about this: |
|
Ah, gotcha. I can check that as well. Just added some tests to make sure there's no trailing whitespace in any file... build blew up, as you might imagine. :P |
Build aborts for any file that doesn't end with a newline.
|
Yeah, we just need to fix all triling spaces, line ending once and never have this problem again :) |
|
Will do that later tonight as part of this PR. It'll mean there's a mountain of changes in the diff, but oh well. |
|
About teamcity github hook again: Gray cross usually indicates "aborted". For "failed" common practice is to use red cross. (see Azure/azure-powershell#102 for example). I don't know how to express it on github hook level, so I just talking in terms of user interface. :) It will be beneficial to align reporting mechanism with common practices, so new contributors will have a consistent experience. |
|
I might be able to change that on the TeamCity server; will see what I can do. |
|
Hmm, nevermind. I thought maybe I had set it up to "cancel build" if the dependencies failed, but it's actually set to "run build, but log problem." Beyond that, I don't seem to have any control over the GitHub hook. It's interesting that the X is red down by the Merge pull request button, but is gray on the line with the commit itself. |
|
Trying one thing... instead of reporting on build start / finish, I switched it to only report on finish. Right now the commits have both a Pending and a Failure status, and maybe that's somehow messing with the icons. |
Added missing newlines at the end of files, and removed all trailing whitespace from ps1 / psm1 files.
|
Reporting only on finish will probably mean, that it will not report 'in progress status'. |
|
That's probably fine. It's only a couple minutes from start to finish in our build anyway. |
|
Maybe preserving Also, we definitely need an example in |
|
I don't think I like the idea of having separate parameters. It's not consistent with the -CodeCoverage parameter (which already accepts strings or hashtables), as well as what people are already accustomed to in Select-Object and Format-*. |
|
Ok. Please, update Pester.psm1 help. |
Also added a check to make sure that explicit file paths at least end with a ps1 extension.
|
Done, but it's kind of wordy for a Parameter entry. By contrast, here's what Select-Object's help has to say about the Property parameter:
Not a single mention of calculated properties or hashtables until you get down to the examples. Would that be better? |
|
LGTM |
Proposed fix for issues #270 and #271 .
As a side effect of these changes, we lose a little bit of console output ("Executing all tests in $Path...") and the Path property is gone from the PesterState object. We intend to add path information about each test script that is executed into the PesterState object in v4 (as a test suite, similar to Describe / Context), which is a better place for that anyway.