Add -WhatIf switch to Start-Process cmdlet#4735
Add -WhatIf switch to Start-Process cmdlet#4735iSazonov merged 7 commits intoPowerShell:masterPowerShell/PowerShell:masterfrom sarithsutha:start-process-enhancementssarithsutha/PowerShell:start-process-enhancementsCopy head branch name to clipboard
Conversation
|
@sarithsutha, |
|
|
||
| It "Should be able to use the whatif switch without error" { | ||
| { Start-Process $pingCommand -ArgumentList $pingParam -WhatIf } | Should Not Throw | ||
| } |
There was a problem hiding this comment.
Perhaps add a test that would normally perform an action and ensure that -WhatIf prevented the action?
For example, have a Start-Process command to write a string to a file and then test to ensure the file was not written to with -WhatIf is supplied.
There was a problem hiding this comment.
Maybe also a test that using the -PassThru switch, no object gets returned (which is also a good additional test that the command was not executed)?
It "returns null when using -WhatIf switch with `PassThru`" {
{ Start-Process $pingCommand -ArgumentList $pingParam -PassThru -WhatIf } | Should Be $null
}
There was a problem hiding this comment.
Thanks for your suggestion @bergmeister . I've added the test that you described.
… from being performed.
|
Thanks for your suggestion @markekraus . I've added that test. |
|
|
||
| It "Should be able to use the whatif switch without error" { | ||
| { Start-Process $pingCommand -ArgumentList $pingParam -WhatIf } | Should Not Throw | ||
| } |
There was a problem hiding this comment.
I think the test does not need - next do the check.
There was a problem hiding this comment.
I'm sorry, I did not get which - were you referring to.
| It "Should be able to use the whatif switch without performing the actual action" { | ||
| $pingOutput = Join-Path $TestDrive "pingOutput.txt" | ||
| Start-Process -Wait $pingCommand -ArgumentList $pingParam -RedirectStandardOutput $pingOutput -WhatIf | ||
| Test-Path $pingOutput | Should Be $false |
There was a problem hiding this comment.
Maybe:
$pingOutput | Should Not Exist | </data> | ||
| <data name="ProcessStartInfo" xml:space="preserve"> | ||
| <value>{0} {1}</value> | ||
| </data> |
There was a problem hiding this comment.
We re-use ProcessNameForConfirmation in Stop-Process and Debug-Process so I believe we can use the same in Start-process.
There was a problem hiding this comment.
The original request was to output the exact command being executed. By re-using the ProcessNameForConfirmation, the format of the output will be changed slightly, the arguments would appear in the parentheses. i.e. C:\WINDOWS\system32\PING.EXE (-n 2 localhost) instead of C:\WINDOWS\system32\PING.EXE -n 2 localhost.
If that is acceptable, I'm happy to make the change.
There was a problem hiding this comment.
Thanks for clarify.
Closed.
|
|
||
| It "Should be able to use the whatif switch without performing the actual action" { | ||
| $pingOutput = Join-Path $TestDrive "pingOutput.txt" | ||
| Start-Process -Wait $pingCommand -ArgumentList $pingParam -RedirectStandardOutput $pingOutput -WhatIf |
There was a problem hiding this comment.
We can remove next It and do the check here.
| It "Should be able to use the -WhatIf switch without performing the actual action" { | ||
| $pingOutput = Join-Path $TestDrive "pingOutput.txt" | ||
| Start-Process -Wait $pingCommand -ArgumentList $pingParam -RedirectStandardOutput $pingOutput -WhatIf | ||
| { Start-Process -Wait $pingCommand -ArgumentList $pingParam -RedirectStandardOutput $pingOutput -WhatIf} | Should Not Throw |
There was a problem hiding this comment.
I believe -ErrorAction Stop is required or it will never actually throw.
There was a problem hiding this comment.
Thanks @markekraus for pointing this out. Not sure how I missed it.
|
Thanks @iSazonov for reviewing and approving my changes. The minor comment is now incorporated. |
|
@daxian-dbw Could you please review and approve? |
| startInfo.WindowStyle = _windowstyle; | ||
| } | ||
|
|
||
| string targetMessage = StringUtil.Format(ProcessResources.ProcessStartInfo, startInfo.FileName, startInfo.Arguments.Trim()); |
There was a problem hiding this comment.
The resource string id ProcessStartInfo should be changed to StartProcessTarget.
There was a problem hiding this comment.
@daxian-dbw Renamed the resource string as suggested.
|
@sarithsutha Thanks for your contribution! |
Closes #4702
Add -WhatIf switch to Start-Process cmdlet