Fix 1541 "-WhatIf results in extra, unwanted output and a spurious …

…error" (#1711)
PowerShell · Oct 10, 2024 · e3d0303 · e3d0303
1 parent 913a92e
commit e3d0303
Show file tree

Hide file tree

Showing 4 changed files with 164 additions and 145 deletions.
diff --git a/src/code/InstallHelper.cs b/src/code/InstallHelper.cs
@@ -60,7 +60,7 @@ internal class InstallHelper
 
         public InstallHelper(PSCmdlet cmdletPassedIn, NetworkCredential networkCredential)
         {
-            CancellationTokenSource source = new CancellationTokenSource();
+            CancellationTokenSource source = new();
             _cancellationToken = source.Token;
             _cmdletPassedIn = cmdletPassedIn;
             _networkCredential = networkCredential;
@@ -183,7 +183,7 @@ private List<PSResourceInfo> ProcessRepositories(
             ScopeType scope)
         {
             _cmdletPassedIn.WriteDebug("In InstallHelper::ProcessRepositories()");
-            List<PSResourceInfo> allPkgsInstalled = new List<PSResourceInfo>();
+            List<PSResourceInfo> allPkgsInstalled = new();
             if (repository != null && repository.Length != 0)
             {
                 // Write error and disregard repository entries containing wildcards.
@@ -262,7 +262,7 @@ private List<PSResourceInfo> ProcessRepositories(
             var noToAll = false;
 
             var findHelper = new FindHelper(_cancellationToken, _cmdletPassedIn, _networkCredential);
-            List<string> repositoryNamesToSearch = new List<string>();
+            List<string> repositoryNamesToSearch = new();
             bool sourceTrusted = false;
 
             // Loop through all the repositories provided (in priority order) until there no more packages to install.
@@ -330,7 +330,7 @@ private List<PSResourceInfo> ProcessRepositories(
                 allPkgsInstalled.AddRange(installedPkgs);
             }
 
-            if (_pkgNamesToInstall.Count > 0)
+            if (!_cmdletPassedIn.MyInvocation.BoundParameters.ContainsKey("WhatIf") && _pkgNamesToInstall.Count > 0)
             {
                 string repositoryWording = repositoryNamesToSearch.Count > 1 ? "registered repositories" : "repository";
                 _cmdletPassedIn.WriteError(new ErrorRecord(
@@ -547,7 +547,7 @@ private List<PSResourceInfo> InstallPackages(
             FindHelper findHelper)
         {
             _cmdletPassedIn.WriteDebug("In InstallHelper::InstallPackages()");
-            List<PSResourceInfo> pkgsSuccessfullyInstalled = new List<PSResourceInfo>();
+            List<PSResourceInfo> pkgsSuccessfullyInstalled = new();
 
             // Install parent package to the temp directory,
             // Get the dependencies from the installed package,
@@ -658,7 +658,7 @@ private List<PSResourceInfo> InstallPackages(
                     }
 
                     // If -WhatIf is passed in, early out.
-                    if (!_cmdletPassedIn.ShouldProcess("Exit ShouldProcess"))
+                    if (_cmdletPassedIn.MyInvocation.BoundParameters.ContainsKey("WhatIf"))
                     {
                         return pkgsSuccessfullyInstalled;
                     }
@@ -1328,17 +1328,17 @@ private bool CallAcceptLicense(PSResourceInfo p, string moduleManifest, string t
 
             if (File.Exists(moduleManifest))
             {
-                using (StreamReader sr = new StreamReader(moduleManifest))
+                using (StreamReader sr = new(moduleManifest))
                 {
                     var text = sr.ReadToEnd();
 
                     var pattern = "RequireLicenseAcceptance\\s*=\\s*\\$true";
                     var patternToSkip1 = "#\\s*RequireLicenseAcceptance\\s*=\\s*\\$true";
                     var patternToSkip2 = "\\*\\s*RequireLicenseAcceptance\\s*=\\s*\\$true";
 
-                    Regex rgx = new Regex(pattern);
-                    Regex rgxComment1 = new Regex(patternToSkip1);
-                    Regex rgxComment2 = new Regex(patternToSkip2);
+                    Regex rgx = new(pattern);
+                    Regex rgxComment1 = new(patternToSkip1);
+                    Regex rgxComment2 = new(patternToSkip2);
                     if (rgx.IsMatch(text) && !rgxComment1.IsMatch(text) && !rgxComment2.IsMatch(text))
                     {
                         requireLicenseAcceptance = true;
@@ -1409,14 +1409,14 @@ private bool DetectClobber(string pkgName, Hashtable parsedMetadataHashtable, ou
 
             // Get installed modules, then get all possible paths
             // selectPrereleaseOnly is false because even if Prerelease is true we want to include both stable and prerelease, would never select prerelease only.
-            GetHelper getHelper = new GetHelper(_cmdletPassedIn);
+            GetHelper getHelper = new(_cmdletPassedIn);
             IEnumerable<PSResourceInfo> pkgsAlreadyInstalled = getHelper.GetPackagesFromPath(
                 name: new string[] { "*" },
                 versionRange: VersionRange.All,
                 pathsToSearch: _pathsToSearch,
                 selectPrereleaseOnly: false);
 
-            List<string> listOfCmdlets = new List<string>();
+            List<string> listOfCmdlets = new();
             if (parsedMetadataHashtable.ContainsKey("CmdletsToExport"))
             {
                 if (parsedMetadataHashtable["CmdletsToExport"] is object[] cmdletsToExport)
@@ -1430,8 +1430,8 @@ private bool DetectClobber(string pkgName, Hashtable parsedMetadataHashtable, ou
 
             foreach (var pkg in pkgsAlreadyInstalled)
             {
-                List<string> duplicateCmdlets = new List<string>();
-                List<string> duplicateCmds = new List<string>();
+                List<string> duplicateCmdlets = new();
+                List<string> duplicateCmds = new();
                 // See if any of the cmdlets or commands in the pkg we're trying to install exist within a package that's already installed
                 if (pkg.Includes.Cmdlet != null && pkg.Includes.Cmdlet.Length != 0)
                 {

diff --git a/test/InstallPSResourceTests/InstallPSResourceLocal.Tests.ps1 b/test/InstallPSResourceTests/InstallPSResourceLocal.Tests.ps1
@@ -1,16 +1,18 @@
 # Copyright (c) Microsoft Corporation.
 # Licensed under the MIT License.
 
+[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '')]
+Param()
+
 $ProgressPreference = "SilentlyContinue"
 $modPath = "$psscriptroot/../PSGetTestUtils.psm1"
 Import-Module $modPath -Force -Verbose
 
 $psmodulePaths = $env:PSModulePath -split ';'
-Write-Verbose -Verbose "Current module search paths: $psmodulePaths"
+Write-Verbose -Verbose -Message "Current module search paths: $psmodulePaths"
 
 Describe 'Test Install-PSResource for local repositories' -tags 'CI' {
 
-
     BeforeAll {
         $localRepo = "psgettestlocal"
         $localUNCRepo = "psgettestlocal3"
@@ -54,7 +56,7 @@ Describe 'Test Install-PSResource for local repositories' -tags 'CI' {
         $res.Version | Should -Be "5.0.0"
     }
 
-	It "Install resource given Name parameter from UNC repository" {
+    It "Install resource given Name parameter from UNC repository" {
         Install-PSResource -Name $testModuleName -Repository $localUNCRepo -TrustRepository
         $res = Get-InstalledPSResource -Name $testModuleName
         $res.Name | Should -Be $testModuleName
@@ -70,7 +72,7 @@ Describe 'Test Install-PSResource for local repositories' -tags 'CI' {
 
     It "Install multiple resources by name" {
         $pkgNames = @($testModuleName, $testModuleName2)
-        Install-PSResource -Name $pkgNames -Repository $localRepo -TrustRepository  
+        Install-PSResource -Name $pkgNames -Repository $localRepo -TrustRepository
         $pkg = Get-InstalledPSResource $pkgNames
         $pkg.Name | Should -Be $pkgNames
     }
@@ -83,7 +85,7 @@ Describe 'Test Install-PSResource for local repositories' -tags 'CI' {
     }
 
     It "Should install resource given name and exact version with bracket syntax" {
-        Install-PSResource -Name $testModuleName -Version "[1.0.0.0]" -Repository $localRepo -TrustRepository  
+        Install-PSResource -Name $testModuleName -Version "[1.0.0.0]" -Repository $localRepo -TrustRepository
         $res = Get-InstalledPSResource $testModuleName
         $res.Name | Should -Be $testModuleName
         $res.Version | Should -Be "1.0.0"
@@ -97,7 +99,7 @@ Describe 'Test Install-PSResource for local repositories' -tags 'CI' {
     }
 
     It "Should install resource given name and exact range exclusive (1.0.0.0, 5.0.0.0)" {
-        Install-PSResource -Name $testModuleName -Version "(1.0.0.0, 5.0.0.0)" -Repository $localRepo -TrustRepository  
+        Install-PSResource -Name $testModuleName -Version "(1.0.0.0, 5.0.0.0)" -Repository $localRepo -TrustRepository
         $res = Get-InstalledPSResource $testModuleName
         $res.Name | Should -Be $testModuleName
         $res.Version | Should -Be "3.0.0"
@@ -110,7 +112,7 @@ Describe 'Test Install-PSResource for local repositories' -tags 'CI' {
         }
         catch
         {}
-        $Error[0].FullyQualifiedErrorId | Should -be "IncorrectVersionFormat,Microsoft.PowerShell.PSResourceGet.Cmdlets.InstallPSResource"
+        $Error[0].FullyQualifiedErrorId | Should -Be "IncorrectVersionFormat,Microsoft.PowerShell.PSResourceGet.Cmdlets.InstallPSResource"
 
         $res = Get-InstalledPSResource $testModuleName -ErrorAction SilentlyContinue
         $res | Should -BeNullOrEmpty
@@ -131,7 +133,7 @@ Describe 'Test Install-PSResource for local repositories' -tags 'CI' {
     }
 
     It "Install resource with latest (including prerelease) version given Prerelease parameter" {
-        Install-PSResource -Name $testModuleName -Prerelease -Repository $localRepo -TrustRepository 
+        Install-PSResource -Name $testModuleName -Prerelease -Repository $localRepo -TrustRepository
         $pkg = Get-InstalledPSResource $testModuleName
         $pkg.Name | Should -Be $testModuleName
         $pkg.Version | Should -Be "5.2.5"
@@ -171,14 +173,14 @@ Describe 'Test Install-PSResource for local repositories' -tags 'CI' {
     }
 
     It "Install resource via InputObject by piping from Find-PSresource" {
-        Find-PSResource -Name $testModuleName -Repository $localRepo | Install-PSResource -TrustRepository 
+        Find-PSResource -Name $testModuleName -Repository $localRepo | Install-PSResource -TrustRepository
         $pkg = Get-InstalledPSResource $testModuleName
         $pkg.Name | Should -Be $testModuleName
         $pkg.Version | Should -Be "5.0.0"
     }
 
     It "Install resource via InputObject by piping from Find-PSResource" {
-        $modules = Find-PSResource -Name "*" -Repository $localRepo 
+        $modules = Find-PSResource -Name "*" -Repository $localRepo
         $modules.Count | Should -BeGreaterThan 1
 
         Install-PSResource -TrustRepository -InputObject $modules
@@ -190,7 +192,7 @@ Describe 'Test Install-PSResource for local repositories' -tags 'CI' {
     It "Install resource under location specified in PSModulePath" {
         Install-PSResource -Name $testModuleName -Repository $localRepo -TrustRepository
         $pkg = Get-InstalledPSResource $testModuleName
-        $pkg.Name | Should -Be $testModuleName 
+        $pkg.Name | Should -Be $testModuleName
         ($env:PSModulePath).Contains($pkg.InstalledLocation)
     }
 
@@ -212,7 +214,7 @@ Describe 'Test Install-PSResource for local repositories' -tags 'CI' {
 
     # Windows only
     It "Install resource under no specified scope - Windows only" -Skip:(!(Get-IsWindows)) {
-        Install-PSResource -Name $testModuleName -Repository $localRepo -TrustRepository 
+        Install-PSResource -Name $testModuleName -Repository $localRepo -TrustRepository
         $pkg = Get-InstalledPSResource $testModuleName
         $pkg.Name | Should -Be $testModuleName
         $pkg.InstalledLocation.ToString().Contains("Documents") | Should -Be $true
@@ -240,7 +242,7 @@ Describe 'Test Install-PSResource for local repositories' -tags 'CI' {
         Install-PSResource -Name $testModuleName -Repository $localRepo -TrustRepository
         $pkg = Get-InstalledPSResource $testModuleName
         $pkg.Name | Should -Be $testModuleName
-        Install-PSResource -Name $testModuleName -Repository $localRepo -TrustRepository -WarningVariable WarningVar -warningaction SilentlyContinue
+        Install-PSResource -Name $testModuleName -Repository $localRepo -TrustRepository -WarningVariable WarningVar -WarningAction SilentlyContinue
         $WarningVar | Should -Not -BeNullOrEmpty
     }
 
@@ -257,6 +259,8 @@ Describe 'Test Install-PSResource for local repositories' -tags 'CI' {
 
     It "Install module using -WhatIf, should not install the module" {
         Install-PSResource -Name $testModuleName -Version "1.0.0.0" -Repository $localRepo -TrustRepository -WhatIf
+        $? | Should -BeTrue
+
         $res = Get-InstalledPSResource -Name $testModuleName -ErrorAction SilentlyContinue
         $res | Should -BeNullOrEmpty
     }
@@ -270,11 +274,11 @@ Describe 'Test Install-PSResource for local repositories' -tags 'CI' {
     It "Get definition for alias 'isres'" {
         (Get-Alias isres).Definition | Should -BeExactly 'Install-PSResource'
     }
-    
+
     It "Not install resource that lists dependency packages which cannot be found" {
         $localRepoUri = Join-Path -Path $TestDrive -ChildPath "testdir"
         Save-PSResource -Name "test_script" -Repository "PSGallery" -TrustRepository -Path $localRepoUri -AsNupkg -SkipDependencyCheck
-        Write-Host $localRepoUri
+        Write-Information -InformationAction Continue -MessageData $localRepoUri
         $res = Install-PSResource -Name "test_script" -Repository $localRepo -TrustRepository -PassThru -ErrorVariable err -ErrorAction SilentlyContinue
         $res | Should -BeNullOrEmpty
         $err.Count | Should -Not -Be 0