8000 Fix importing module remote using version filters and added tests by SteveL-MSFT · Pull Request #4900 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]
Skip to content

Fix importing module remote using version filters and added tests #4900

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 26, 2017
Merged

Fix importing module remote using version filters and added tests #4900

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8caaaaa
[feature]
SteveL-MSFT Sep 20, 2017
4f28982
address PR feedback to store and revert the changed parameter values
SteveL-MSFT Sep 22, 2017
c6c2f96
[feature]
SteveL-MSFT Sep 25, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -986,14 +986,28 @@ private PSModuleInfo ImportModule_RemotelyViaPsrpSession_SinglePreimportedModule
// import the proxy module just as any other local module
//
object[] oldArgumentList = this.ArgumentList;
Version originalBaseMinimumVersion = BaseMinimumVersion;
Version originalBaseMaximumVersion = BaseMaximumVersion;
Version originalBaseRequiredVersion = BaseRequiredVersion;
try
{
this.ArgumentList = new object[] { psSession };
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand why the PSSession is added/reverted from the ArgumentList here. The remote session has already been imported (via the Export-PSSession Cmdlet) and is no longer needed. It looks like this can be removed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually this does need to be here. It is used to fill the PSSession property on the module info.


// The correct module version has already been imported from the remote session and created locally.
// The locally created module always has a version of 1.0 regardless of the actual module version
// imported from the remote session, and version checking is no longer needed and will not work while
// importing this created local module.
BaseMinimumVersion = null;
BaseMaximumVersion = null;
BaseRequiredVersion = null;
ImportModule_LocallyViaName(importModuleOptions, wildcardEscapedPsd1Path);
}
finally
{
this.ArgumentList = oldArgumentList;
BaseMinimumVersion = originalBaseMinimumVersion;
BaseMaximumVersion = originalBaseMaximumVersion;
BaseRequiredVersion = originalBaseRequiredVersion;
}

//
Expand Down
8 changes: 7 additions & 1 deletion test/powershell/Modules/Microsoft.PowerShell.Core/Import-Module.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,16 @@
{ Import-Module -ModuleInfo $a } | Should Not Throw
(Get-Module -Name $moduleName).Name | Should Be $moduleName
}

It "should be able to load an already loaded module" {
Import-Module $moduleName
{ $script:module = Import-Module $moduleName -PassThru } | Should Not Throw
Get-Module -Name $moduleName | Should Be $script:module
}
}

Describe "Import-Module with ScriptsToProcess" -Tags "CI" {

BeforeAll {
$moduleRootPath = Join-Path $TestDrive 'TestModules'
New-Item $moduleRootPath -ItemType Directory -Force | Out-Null
Expand Down
111 changes: 111 additions & 0 deletions test/powershell/Modules/Microsoft.PowerShell.Core/RemoteImportModule.Tests.ps1
View file
Open in desktop
8000
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
Describe "Remote import-module tests" -Tags 'Feature','RequireAdminOnWindows' {

BeforeAll {
$originalDefaultParameterValues = $PSDefaultParameterValues.Clone()
$modulePath = "$testdrive\Modules\TestImport"
if (!$IsWindows) {
$PSDefaultParameterValues["it:skip"] = $true
} else {
$pssession = New-RemoteSession
Invoke-Command -Session $pssession -ScriptBlock { $env:PSModulePath += ";${using:testdrive}" }
# pending https://github.com/PowerShell/PowerShell/issues/4819
# $cimsession = New-RemoteSession -CimSession
$null = New-Item -ItemType Directory -Path $modulePath
Set-Content -Path $modulePath\testimport.psm1 -Value "function test-hello { 'world' }"
New-ModuleManifest -Path $modulePath\testimport.psd1 -ModuleVersion 1.2.3 -RootModule testimport.psm1 -FunctionsToExport "test-hello" `
-HelpInfoUri "https://help" -Guid (New-Guid)
}
}

AfterAll {
$global:PSDefaultParameterValues = $originalDefaultParameterValues
if ($IsWindows) {
$pssession | Remove-PSSession -ErrorAction SilentlyContinue
}
}

BeforeEach {
Remove-Module TestImport -Force -ErrorAction SilentlyContinue
}

It "Import-Module can be called as an API with '<parameter>' = '<value>'" -TestCases @(
@{parameter = "Global" ; value = $true},
@{parameter = "Global" ; value = $false},
@{parameter = "Prefix" ; value = "Hello"},
@{parameter = "Name" ; value = "foo","bar"},
@{parameter = "FullyQualifiedName" ; value = @{ModuleName='foo';RequiredVersion='0.0'},@{ModuleName='bar';RequiredVersion='1.1'}},
@{parameter = "Assembly" ; script = { [System.AppDomain]::CurrentDomain.GetAssemblies() | Select-Object -First 2 }}
Copy link
Contributor

Choose a reason for hiding this comment

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

8000

Can't the Assembly array be pre-computed and passed in as a "value" parameter? Then you wouldn't need the special case for "script" parameter.

Copy link
Member Author

Choose a reason for hiding this comment

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

My thinking is that if we add new test cases here, we can reuse script even though it's only used once here. Also, I don't like having variables defined at the top of the script in a BeforeAll statement if it's only used once for a specific It, prefer to have it closer to the code where it's being used.

@{parameter = "Function" ; value = "foo","bar"},
@{parameter = "Cmdlet" ; value = "foo","bar"},
@{parameter = "Variable" ; value = "foo","bar"},
@{parameter = "Alias" ; value = "foo","bar"},
@{parameter = "Force" ; value = $true},
@{parameter = "Force" ; value = $false},
@{parameter = "PassThru" ; value = $true},
@{parameter = "PassThru" ; value = $false},
@{parameter = "AsCustomObject" ; value = $true},
@{parameter = "AsCustomObject" ; value = $false},
@{parameter = "MinimumVersion" ; value = "1.2.3"},
@{parameter = "MaximumVersion" ; value = "3.2.1"},
@{parameter = "RequiredVersion" ; value = "1.1.1"},
@{parameter = "ArgumentList" ; value = "hello","world"},
@{parameter = "DisableNameChecking"; value = $true},
@{parameter = "DisableNameChecking"; value = $false},
@{parameter = "NoClobber" ; value = $true},
@{parameter = "NoClobber" ; value = $false},
@{parameter = "Scope" ; value = "Local"},
@{parameter = "Scope" ; value = "Global"},
@{parameter = "PSSession" ; value = $pssession},
# @{parameter = "CimSession" ; value = $cimsession},
@{parameter = "CimResourceUri" ; value = "http://foo/"},
@{parameter = "CimNamespace" ; value = "foo"}
) {
param($parameter, $value, $script)

$importModuleCommand = [Microsoft.PowerShell.Commands.ImportModuleCommand]::new()
if ($script -ne $null) {
$val 9E91 ue = & $script
}
$importModuleCommand.$parameter = $value
if ($parameter -eq "FullyQualifiedName") {
$importModuleCommand.FullyQualifiedName.Count | Should BeExactly 2
$importModuleCommand.FullyQualifiedName | Should BeOfType "Microsoft.PowerShell.Commands.ModuleSpecification"
$importModuleCommand.FullyQualifiedName[0].Name | Should Be "foo"
$importModuleCommand.FullyQualifiedName[0].RequiredVersion | Should Be "0.0"
$importModuleCommand.FullyQualifiedName[1].Name | Should Be "bar"
$importModuleCommand.FullyQualifiedName[1].RequiredVersion | Should Be "1.1"
} else {
$importModuleCommand.$parameter | Should BeExactly $value
}
}

It "Import-Module can import over remote session: <test>" -TestCases @(
@{ test = "pssession" ; parameters = @{Name="TestImport";PSSession=$pssession}},
# @{ test = "cimsession" ; parameters = @{Name="TestImport";CimSession=$cimsession}},
@{ test = "minimumversion" ; parameters = @{Name="TestImport";PSSession=$pssession;MinimumVersion="1.0";Force=$true}},
@{ test = "requiredversion" ; parameters = @{Name="TestImport";PSSession=$pssession;RequiredVersion="1.2.3"}},
@{ test = "maxiumversion" ; parameters = @{Name="TestImport";PSSession=$pssession;MaximumVersion="2.0"}},
@{ test = "invalid miniumversion" ; parameters = @{Name="TestImport";PSSession=$pssession;MinimumVersion="2.0"};
errorid = "Modules_ModuleWithVersionNotFound,Microsoft.PowerShell.Commands.ImportModuleCommand,Microsoft.PowerShell.Commands.ImportModuleCommand"},
@{ test = "invalid maximumversion" ; parameters = @{Name="TestImport";PSSession=$pssession;MaximumVersion="1.0"};
errorid = "Modules_ModuleWithVersionNotFound,Microsoft.PowerShell.Commands.ImportModuleCommand,Microsoft.PowerShell.Commands.ImportModuleCommand"},
@{ test = "fullyqualifiedname" ; parameters = @{FullyQualifiedName=@{Modulename="TestImport"; RequiredVersion="1.2.3"};PSSession=$pssession}}
) {
param ($test, $parameters, $errorid)

Invoke-Command -Session $pssession -ScriptBlock { $env:PSModulePath += ";$(Split-Path $using:modulePath)"}
Get-Module TestImport | Should BeNullOrEmpty
if ($errorid) {
{ Import-Module @parameters -ErrorAction Stop } | ShouldBeErrorId $errorid
} else {
Import-Module @parameters
$module = Get-Module TestImport
$module.Name | Should BeExactly "TestImport"

# generated proxy module always uses 1.0
$module.Version | Should BeExactly "1.0"

test-hello | Should BeExactly "world"
}
}
}
0