From 76c9da189dd5990010fbce3d44890d9659be9b0e Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Tue, 25 Sep 2018 14:04:28 -0700 Subject: [PATCH 1/6] Fix ensuring NestedModules property gets populated even with no further analysis --- .../engine/Modules/ModuleCmdletBase.cs | 14 ++++++++ .../Module/TestModuleManifest.Tests.ps1 | 36 ++++++++++--------- 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index 576a9a295ae..7dea6845d54 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -2797,6 +2797,20 @@ internal PSModuleInfo LoadModuleManifest( if (!needToAnalyzeScriptModules) { + // need to add nested modules to the manifestInfo when no more analysis needs to be done + PSModuleInfo fakeNestedModuleInfo = null; + foreach (ModuleSpecification nestedModule in nestedModules) + { + fakeNestedModuleInfo = new PSModuleInfo(nestedModule.Name, Context, null); + if (nestedModule.Guid.HasValue) + { + fakeNestedModuleInfo.SetGuid(nestedModule.Guid.Value); + } + fakeNestedModuleInfo.SetVersion(nestedModule.RequiredVersion ?? nestedModule.Version); + + manifestInfo.AddNestedModule(fakeNestedModuleInfo); + } + return manifestInfo; } } diff --git a/test/powershell/engine/Module/TestModuleManifest.Tests.ps1 b/test/powershell/engine/Module/TestModuleManifest.Tests.ps1 index bbd0093c6c1..301b9a93156 100644 --- a/test/powershell/engine/Module/TestModuleManifest.Tests.ps1 +++ b/test/powershell/engine/Module/TestModuleManifest.Tests.ps1 @@ -2,13 +2,17 @@ # Licensed under the MIT License. Describe "Test-ModuleManifest tests" -tags "CI" { + BeforeEach { + $testModulePath = "testdrive:/module/test.psd1" + New-Item -ItemType Directory -Path testdrive:/module + } + AfterEach { Remove-Item -Recurse -Force -ErrorAction SilentlyContinue testdrive:/module } It "module manifest containing paths with backslashes or forwardslashes are resolved correctly" { - New-Item -ItemType Directory -Path testdrive:/module New-Item -ItemType Directory -Path testdrive:/module/foo New-Item -ItemType Directory -Path testdrive:/module/bar New-Item -ItemType File -Path testdrive:/module/foo/bar.psm1 @@ -38,10 +42,8 @@ Describe "Test-ModuleManifest tests" -tags "CI" { param ($parameter, $error) - New-Item -ItemType Directory -Path testdrive:/module New-Item -ItemType Directory -Path testdrive:/module/foo New-Item -ItemType File -Path testdrive:/module/foo/bar.psm1 - $testModulePath = "testdrive:/module/test.psd1" $args = @{$parameter = "doesnotexist.psm1"} New-ModuleManifest -Path $testModulePath @args @@ -57,9 +59,6 @@ Describe "Test-ModuleManifest tests" -tags "CI" { param($rootModuleValue) - New-Item -ItemType Directory -Path testdrive:/module - $testModulePath = "testdrive:/module/test.psd1" - New-Item -ItemType File -Path testdrive:/module/$rootModuleValue New-ModuleManifest -Path $testModulePath -RootModule $rootModuleValue $moduleManifest = Test-ModuleManifest -Path $testModulePath -ErrorAction Stop @@ -74,9 +73,6 @@ Describe "Test-ModuleManifest tests" -tags "CI" { param($rootModuleValue, $error) - New-Item -ItemType Directory -Path testdrive:/module - $testModulePath = "testdrive:/module/test.psd1" - New-Item -ItemType File -Path testdrive:/module/$rootModuleValue New-ModuleManifest -Path $testModulePath -RootModule $rootModuleValue { Test-ModuleManifest -Path $testModulePath -ErrorAction Stop } | Should -Throw -ErrorId "$error,Microsoft.PowerShell.Commands.TestModuleManifestCommand" @@ -89,9 +85,6 @@ Describe "Test-ModuleManifest tests" -tags "CI" { param($rootModuleValue) - New-Item -ItemType Directory -Path testdrive:/module - $testModulePath = "testdrive:/module/test.psd1" - New-ModuleManifest -Path $testModulePath -RootModule $rootModuleValue $moduleManifest = Test-ModuleManifest -Path $testModulePath -ErrorAction Stop $moduleManifest | Should -BeOfType System.Management.Automation.PSModuleInfo @@ -104,8 +97,6 @@ Describe "Test-ModuleManifest tests" -tags "CI" { param($rootModuleValue, $error) - $testModulePath = "testdrive:/module/test.psd1" - New-Item -ItemType Directory -Path testdrive:/module New-Item -ItemType File -Path testdrive:/module/$rootModuleValue New-ModuleManifest -Path $testModulePath -RootModule $rootModuleValue @@ -118,12 +109,23 @@ Describe "Test-ModuleManifest tests" -tags "CI" { param($rootModuleValue, $error) - $testModulePath = "testdrive:/module/test.psd1" - New-Item -ItemType Directory -Path testdrive:/module - New-ModuleManifest -Path $testModulePath -RootModule $rootModuleValue { Test-ModuleManifest -Path $testModulePath -ErrorAction Stop } | Should -Throw -ErrorId "$error,Microsoft.PowerShell.Commands.TestModuleManifestCommand" } + + It "module manifest containing nested module gets returned: " -TestCases ( + @{variation = "no analysis as all exported with no wildcard"; exportValue = "@()"}, + @{variation = "analysis as exported with wildcard"; exportValue = "*"} + ) { + + param($exportValue) + + New-Item -ItemType File -Path testdrive:/module/Foo.psm1 + New-ModuleManifest -Path $testModulePath -NestedModules "Foo.psm1" -FunctionsToExport $exportValue -CmdletsToExport $exportValue -VariablesToExport $exportValue -AliasesToExport $exportValue + $module = Test-ModuleManifest -Path $testModulePath + $module.NestedModules | Should -HaveCount 1 + $module.NestedModules.Name | Should -BeExactly "Foo" + } } Describe "Tests for circular references in required modules" -tags "CI" { From 36fc6ead27b712a775207cfdf02f3096349ec4b6 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Tue, 25 Sep 2018 15:12:00 -0700 Subject: [PATCH 2/6] address Paul's feedback address codefactor issue added test for update-modulemanifest --- .../engine/Modules/ModuleCmdletBase.cs | 3 +- .../Module/UpdateModuleManifest.Tests.ps1 | 33 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 test/powershell/engine/Module/UpdateModuleManifest.Tests.ps1 diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index 7dea6845d54..dbf651a35e7 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -2797,7 +2797,7 @@ internal PSModuleInfo LoadModuleManifest( if (!needToAnalyzeScriptModules) { - // need to add nested modules to the manifestInfo when no more analysis needs to be done + // Add nested modules to the manifestInfo when no more analysis needs to be done PSModuleInfo fakeNestedModuleInfo = null; foreach (ModuleSpecification nestedModule in nestedModules) { @@ -3442,6 +3442,7 @@ internal PSModuleInfo LoadModuleManifest( updated.Add(element); } } + ss.Internal.ExportedVariables.Clear(); ss.Internal.ExportedVariables.AddRange(updated); } diff --git a/test/powershell/engine/Module/UpdateModuleManifest.Tests.ps1 b/test/powershell/engine/Module/UpdateModuleManifest.Tests.ps1 new file mode 100644 index 00000000000..22f2a1c6388 --- /dev/null +++ b/test/powershell/engine/Module/UpdateModuleManifest.Tests.ps1 @@ -0,0 +1,33 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +Describe "Update-ModuleManifest tests" -tags "CI" { + + BeforeEach { + $testModulePath = "testdrive:/module/test.psd1" + New-Item -ItemType Directory -Path testdrive:/module + } + + AfterEach { + Remove-Item -Recurse -Force -ErrorAction SilentlyContinue testdrive:/module + } + + It "Update should not clear out NestedModules: " -TestCases @( + @{ variation = "export with wildcards"; exportValue = "*" }, + @{ variation = "export without wildcards"; exportValue = "@()"} + ) { + param($exportValue) + + New-Item -ItemType File -Path testdrive:/module/foo.psm1 + New-ModuleManifest -Path $testModulePath -NestedModules foo.psm1 -HelpInfoUri http://foo.com -AliasesToExport $exportValue -CmdletsToExport $exportValue -FunctionsToExport $exportValue -VariablesToExport $exportValue -DscResourcesToExport $exportValue + $module = Test-ModuleManifest -Path $testModulePath + $module.HelpInfoUri | Should -BeExactly "http://foo.com/" + $module.NestedModules | Should -HaveCount 1 + $module.NestedModules.Name | Should -BeExactly foo + Update-ModuleManifest -Path $testModulePath -HelpInfoUri https://bar.org + $module = Test-ModuleManifest -Path $testModulePath + $module.HelpInfoUri | Should -BeExactly "https://bar.org/" + $module.NestedModules | Should -HaveCount 1 + $module.NestedModules.Name | Should -BeExactly foo + } +} From a27c5c8bc47e4316b6033fe857c10465973ad252 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Thu, 27 Sep 2018 13:30:16 -0700 Subject: [PATCH 3/6] address Dongbo's feedback --- .../engine/Modules/ModuleCmdletBase.cs | 39 ++++++++++--------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index dbf651a35e7..a77eb2e157d 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -1338,6 +1338,25 @@ internal bool LoadModuleManifestData(ExternalScriptInfo scriptInfo, ManifestProc return true; } + /// + /// Helper function to generate fake PSModuleInfo objects from ModuleSpecification objects + /// + /// Collection of ModuleSpecification objects + /// Collection of fake PSModuleInfo objects + private IEnumerable CreateFakeModuleObject(IEnumerable moduleSpecs) + { + foreach (ModuleSpecification moduleSpec in moduleSpecs) + { + var fakeModuleInfo = new PSModuleInfo(moduleSpec.Name, Context, null); + if (moduleSpec.Guid.HasValue) + { + fakeModuleInfo.SetGuid(moduleSpec.Guid.Value); + } + fakeModuleInfo.SetVersion(moduleSpec.RequiredVersion ?? moduleSpec.Version); + yield return fakeModuleInfo; + } + } + private ErrorRecord GetErrorRecordIfUnsupportedRootCdxmlAndNestedModuleScenario( Hashtable data, string moduleManifestPath, @@ -1977,16 +1996,8 @@ internal PSModuleInfo LoadModuleManifest( } else { - PSModuleInfo fakeRequiredModuleInfo = null; - foreach (ModuleSpecification requiredModule in requiredModules) + foreach (PSModuleInfo fakeRequiredModuleInfo in CreateFakeModuleObject(requiredModules)) { - fakeRequiredModuleInfo = new PSModuleInfo(requiredModule.Name, Context, null); - if (requiredModule.Guid.HasValue) - { - fakeRequiredModuleInfo.SetGuid(requiredModule.Guid.Value); - } - fakeRequiredModuleInfo.SetVersion(requiredModule.RequiredVersion ?? requiredModule.Version); - requiredModulesSpecifiedInModuleManifest.Add(fakeRequiredModuleInfo); } } @@ -2798,16 +2809,8 @@ internal PSModuleInfo LoadModuleManifest( if (!needToAnalyzeScriptModules) { // Add nested modules to the manifestInfo when no more analysis needs to be done - PSModuleInfo fakeNestedModuleInfo = null; - foreach (ModuleSpecification nestedModule in nestedModules) + foreach (PSModuleInfo fakeNestedModuleInfo in CreateFakeModuleObject(nestedModules)) { - fakeNestedModuleInfo = new PSModuleInfo(nestedModule.Name, Context, null); - if (nestedModule.Guid.HasValue) - { - fakeNestedModuleInfo.SetGuid(nestedModule.Guid.Value); - } - fakeNestedModuleInfo.SetVersion(nestedModule.RequiredVersion ?? nestedModule.Version); - manifestInfo.AddNestedModule(fakeNestedModuleInfo); } From 7d4b364f7e0c2cdc5e04e4a5d298b1cfc692fb23 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Fri, 28 Sep 2018 10:31:31 -0700 Subject: [PATCH 4/6] Add the ending dot for doc comment. --- .../engine/Modules/ModuleCmdletBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index a77eb2e157d..e3c85d9d31a 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -1339,7 +1339,7 @@ internal bool LoadModuleManifestData(ExternalScriptInfo scriptInfo, ManifestProc } /// - /// Helper function to generate fake PSModuleInfo objects from ModuleSpecification objects + /// Helper function to generate fake PSModuleInfo objects from ModuleSpecification objects. /// /// Collection of ModuleSpecification objects /// Collection of fake PSModuleInfo objects From d7c718a86fca3eb15226743ed617f4c5d2eb1e7d Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Fri, 28 Sep 2018 10:35:06 -0700 Subject: [PATCH 5/6] Ignore the output of 'New-Item' as appropriate --- .../Module/TestModuleManifest.Tests.ps1 | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/test/powershell/engine/Module/TestModuleManifest.Tests.ps1 b/test/powershell/engine/Module/TestModuleManifest.Tests.ps1 index 301b9a93156..350d5504dcf 100644 --- a/test/powershell/engine/Module/TestModuleManifest.Tests.ps1 +++ b/test/powershell/engine/Module/TestModuleManifest.Tests.ps1 @@ -4,7 +4,7 @@ Describe "Test-ModuleManifest tests" -tags "CI" { BeforeEach { $testModulePath = "testdrive:/module/test.psd1" - New-Item -ItemType Directory -Path testdrive:/module + New-Item -ItemType Directory -Path testdrive:/module > $null } AfterEach { @@ -13,10 +13,10 @@ Describe "Test-ModuleManifest tests" -tags "CI" { It "module manifest containing paths with backslashes or forwardslashes are resolved correctly" { - New-Item -ItemType Directory -Path testdrive:/module/foo - New-Item -ItemType Directory -Path testdrive:/module/bar - New-Item -ItemType File -Path testdrive:/module/foo/bar.psm1 - New-Item -ItemType File -Path testdrive:/module/bar/foo.psm1 + New-Item -ItemType Directory -Path testdrive:/module/foo > $null + New-Item -ItemType Directory -Path testdrive:/module/bar > $null + New-Item -ItemType File -Path testdrive:/module/foo/bar.psm1 > $null + New-Item -ItemType File -Path testdrive:/module/bar/foo.psm1 > $null $testModulePath = "testdrive:/module/test.psd1" $fileList = "foo\bar.psm1","bar/foo.psm1" @@ -42,8 +42,8 @@ Describe "Test-ModuleManifest tests" -tags "CI" { param ($parameter, $error) - New-Item -ItemType Directory -Path testdrive:/module/foo - New-Item -ItemType File -Path testdrive:/module/foo/bar.psm1 + New-Item -ItemType Directory -Path testdrive:/module/foo > $null + New-Item -ItemType File -Path testdrive:/module/foo/bar.psm1 > $null $args = @{$parameter = "doesnotexist.psm1"} New-ModuleManifest -Path $testModulePath @args @@ -59,7 +59,7 @@ Describe "Test-ModuleManifest tests" -tags "CI" { param($rootModuleValue) - New-Item -ItemType File -Path testdrive:/module/$rootModuleValue + New-Item -ItemType File -Path testdrive:/module/$rootModuleValue > $null New-ModuleManifest -Path $testModulePath -RootModule $rootModuleValue $moduleManifest = Test-ModuleManifest -Path $testModulePath -ErrorAction Stop $moduleManifest | Should -BeOfType System.Management.Automation.PSModuleInfo @@ -73,7 +73,7 @@ Describe "Test-ModuleManifest tests" -tags "CI" { param($rootModuleValue, $error) - New-Item -ItemType File -Path testdrive:/module/$rootModuleValue + New-Item -ItemType File -Path testdrive:/module/$rootModuleValue > $null New-ModuleManifest -Path $testModulePath -RootModule $rootModuleValue { Test-ModuleManifest -Path $testModulePath -ErrorAction Stop } | Should -Throw -ErrorId "$error,Microsoft.PowerShell.Commands.TestModuleManifestCommand" } @@ -97,7 +97,7 @@ Describe "Test-ModuleManifest tests" -tags "CI" { param($rootModuleValue, $error) - New-Item -ItemType File -Path testdrive:/module/$rootModuleValue + New-Item -ItemType File -Path testdrive:/module/$rootModuleValue > $null New-ModuleManifest -Path $testModulePath -RootModule $rootModuleValue { Test-ModuleManifest -Path $testModulePath -ErrorAction Stop } | Should -Throw -ErrorId "$error,Microsoft.PowerShell.Commands.TestModuleManifestCommand" @@ -120,7 +120,7 @@ Describe "Test-ModuleManifest tests" -tags "CI" { param($exportValue) - New-Item -ItemType File -Path testdrive:/module/Foo.psm1 + New-Item -ItemType File -Path testdrive:/module/Foo.psm1 > $null New-ModuleManifest -Path $testModulePath -NestedModules "Foo.psm1" -FunctionsToExport $exportValue -CmdletsToExport $exportValue -VariablesToExport $exportValue -AliasesToExport $exportValue $module = Test-ModuleManifest -Path $testModulePath $module.NestedModules | Should -HaveCount 1 @@ -176,7 +176,7 @@ Describe "Tests for circular references in required modules" -tags "CI" { function TestImportModule([bool]$AddVersion, [bool]$AddGuid, [bool]$AddCircularReference) { $moduleRootPath = Join-Path $TestDrive 'TestModules' - New-Item $moduleRootPath -ItemType Directory -Force + New-Item $moduleRootPath -ItemType Directory -Force > $null Push-Location $moduleRootPath $moduleCount = 6 # this depth was enough to find a bug in cyclic reference detection product code; greater depth will slow tests down From 89e330c0c5f6d120e72e4659a1777dae349f73d3 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Fri, 28 Sep 2018 10:36:52 -0700 Subject: [PATCH 6/6] Ignore the output of 'New-Item' - take 2 --- test/powershell/engine/Module/UpdateModuleManifest.Tests.ps1 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/powershell/engine/Module/UpdateModuleManifest.Tests.ps1 b/test/powershell/engine/Module/UpdateModuleManifest.Tests.ps1 index 22f2a1c6388..0ac58a92c3c 100644 --- a/test/powershell/engine/Module/UpdateModuleManifest.Tests.ps1 +++ b/test/powershell/engine/Module/UpdateModuleManifest.Tests.ps1 @@ -5,7 +5,7 @@ Describe "Update-ModuleManifest tests" -tags "CI" { BeforeEach { $testModulePath = "testdrive:/module/test.psd1" - New-Item -ItemType Directory -Path testdrive:/module + New-Item -ItemType Directory -Path testdrive:/module > $null } AfterEach { @@ -18,7 +18,7 @@ Describe "Update-ModuleManifest tests" -tags "CI" { ) { param($exportValue) - New-Item -ItemType File -Path testdrive:/module/foo.psm1 + New-Item -ItemType File -Path testdrive:/module/foo.psm1 > $null New-ModuleManifest -Path $testModulePath -NestedModules foo.psm1 -HelpInfoUri http://foo.com -AliasesToExport $exportValue -CmdletsToExport $exportValue -FunctionsToExport $exportValue -VariablesToExport $exportValue -DscResourcesToExport $exportValue $module = Test-ModuleManifest -Path $testModulePath $module.HelpInfoUri | Should -BeExactly "http://foo.com/"