Skip to content

Fix 1541 "-WhatIf results in extra, unwanted output and a spurious error" #1711

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 5 commits into from
Oct 10, 2024
Merged

Fix 1541 "-WhatIf results in extra, unwanted output and a spurious error" #1711

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2911289
Fixed check for whether WhatIf was passed in
o-l-a-v Sep 22, 2024
1de3675
There will be no packages to install if -WhatIf
o-l-a-v Sep 22, 2024
e6aed72
Add whatif test to nuget v3 test too + fix and supress psscriptanalyz…
o-l-a-v Sep 22, 2024
da57db6
WhatIf test $? should be true + fix and supress psscriptanalyzer warn…
o-l-a-v Sep 22, 2024
88458d7
Simplify new variable
o-l-a-v Sep 22, 2024
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
28 changes: 14 additions & 14 deletions src/code/InstallHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -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)
{
Expand Down
34 changes: 19 additions & 15 deletions test/InstallPSResourceTests/InstallPSResourceLocal.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -51,7 +53,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
Expand All @@ -67,7 +69,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
}
Expand All @@ -80,7 +82,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"
Expand All @@ -94,7 +96,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"
Expand All @@ -107,7 +109,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
Expand All @@ -128,7 +130,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"
Expand Down Expand Up @@ -168,14 +170,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
Expand All @@ -187,7 +189,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)
}

Expand All @@ -209,7 +211,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
Expand Down Expand Up @@ -237,7 +239,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
}

Expand All @@ -254,6 +256,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
}
Expand All @@ -267,11 +271,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
Expand Down
Loading
Loading