X Tutup
Skip to content

Adding positional parameter for ScriptBlock when using Invoke-Command with SSH#10721

Merged
adityapatwardhan merged 4 commits intoPowerShell:masterfrom
machgo:invokecommandsshpos
Oct 10, 2019
Merged

Adding positional parameter for ScriptBlock when using Invoke-Command with SSH#10721
adityapatwardhan merged 4 commits intoPowerShell:masterfrom
machgo:invokecommandsshpos

Conversation

@machgo
Copy link
Contributor

@machgo machgo commented Oct 6, 2019

PR Summary

This PR adds the PositionalParameter Scriptblock when using Invoke-Command with the new -Hostname or -SSHConnection parameters.
This allows to use the same usage as when using WinRM with -Computername

examples:
{ Invoke-Command -HostName localhost { 'test' } }
{ Invoke-Command -SSHConnection @{ HostName = 'localhost' } { 'test' } }

PR Context

fixes #10708

PR Checklist

@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Oct 7, 2019
@iSazonov iSazonov added this to the 7.0.0-preview.5 milestone Oct 7, 2019
Copy link
Collaborator

@iSazonov iSazonov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@machgo Please add tests.

@machgo
Copy link
Contributor Author

machgo commented Oct 7, 2019

@machgo Please add tests.

I've added two simple tests which are checking for a System.ArguemenException when calling Invoke-Method with the positional parameter for ScriptBlock. Please let me know if this is enough or if we should verify with a real SSH-connection (if that's possible in during the CI-test).

Copy link
Contributor

@PaulHigin PaulHigin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Should -Not -Throw -ErrorId "System.ArgumentException,Microsoft.PowerShell.Commands.InvokeCommandCommand"
}

It "Invoke-Command should support poisitonal parameter ScriptBlock when using SSHConnection" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo in poisitonal -> positional

}

It "Invoke-Command should support positional parameter ScriptBlock when using HostName" {
{ Invoke-Command -HostName localhost { "test" } } |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of checking it in this way, i would prefer checking it using command metadata. I have a sample here:

Describe "sdfsf" {

    It "Invoke-Command should support positional parameter ScriptBlock when using parameter set '<ParameterSetName>'" -TestCases @{ParameterSetName = 'SSHHost'}, @{ParameterSetName = 'SSHHostHashParam'} {
        param ($ParameterSetName)
        $commandInfo = Get-Command -Name Invoke-Command 

        $sshHostParameters = $commandInfo.ParameterSets | Where-Object { $_.Name -eq $ParameterSetName } 
        
        $sshHostSBPostion = $sshHostParameters.Parameters | Where-Object { $_.name -eq 'ScriptBlock' } | Select-Object -ExpandProperty position
        
        $sshHostSBPostion | Should -Be 1        
    }
}

}

It "Invoke-Command should support poisitonal parameter ScriptBlock when using SSHConnection" {
{ Invoke-Command -SSHConnection @{ HostName = "localhost" } { "test" } } |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above

@ghost ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Oct 8, 2019
@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Oct 9, 2019
@adityapatwardhan adityapatwardhan merged commit 0f9ca32 into PowerShell:master Oct 10, 2019
@adityapatwardhan
Copy link
Member

@machgo Thank you for your contribution

@ghost
Copy link

ghost commented Oct 23, 2019

🎉v7.0.0-preview.5 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Invoke-Command -Hostname / -SSHConnection should accept the script block as the (first) positional argument

4 participants

X Tutup