(#3318) Add ShouldProcess support to commands

Some cmdlets should support ShouldProcess, this change ensures that they do correctly support it, along with adding checks to the underlying methods to make it work as users will expect. As Chocolatey CLI already supports --dry-run itself, this is primarily going to be useful for effectively unit testing the commands.
chocolatey · Nov 1, 2024 · 8c82727 · 8c82727
1 parent 7f7186f
commit 8c82727
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 46 deletions.
diff --git a/src/Chocolatey.PowerShell/Commands/InstallChocolateyPathCommand.cs b/src/Chocolatey.PowerShell/Commands/InstallChocolateyPathCommand.cs
@@ -21,7 +21,7 @@
 
 namespace Chocolatey.PowerShell.Commands
 {
-    [Cmdlet(VerbsLifecycle.Install, "ChocolateyPath")]
+    [Cmdlet(VerbsLifecycle.Install, "ChocolateyPath", SupportsShouldProcess = true, ConfirmImpact = ConfirmImpact.Medium)]
     [OutputType(typeof(void))]
     public class InstallChocolateyPathCommand : ChocolateyCmdlet
     {

diff --git a/src/Chocolatey.PowerShell/Commands/SetEnvironmentVariableCommand.cs b/src/Chocolatey.PowerShell/Commands/SetEnvironmentVariableCommand.cs
@@ -21,7 +21,7 @@
 
 namespace Chocolatey.PowerShell.Commands
 {
-    [Cmdlet(VerbsCommon.Set, "EnvironmentVariable")]
+    [Cmdlet(VerbsCommon.Set, "EnvironmentVariable", SupportsShouldProcess = true, ConfirmImpact = ConfirmImpact.Medium)]
     [OutputType(typeof(void))]
     public sealed class SetEnvironmentVariableCommand : ChocolateyCmdlet
     {

diff --git a/src/Chocolatey.PowerShell/Commands/UninstallChocolateyPathCommand.cs b/src/Chocolatey.PowerShell/Commands/UninstallChocolateyPathCommand.cs
@@ -21,7 +21,7 @@
 
 namespace Chocolatey.PowerShell.Commands
 {
-    [Cmdlet(VerbsLifecycle.Uninstall, "ChocolateyPath")]
+    [Cmdlet(VerbsLifecycle.Uninstall, "ChocolateyPath", SupportsShouldProcess = true, ConfirmImpact = ConfirmImpact.Medium)]
     [OutputType(typeof(void))]
     public class UninstallChocolateyPathCommand : ChocolateyCmdlet
     {

diff --git a/src/Chocolatey.PowerShell/Commands/UpdateSessionEnvironmentCommand.cs b/src/Chocolatey.PowerShell/Commands/UpdateSessionEnvironmentCommand.cs
@@ -20,7 +20,7 @@
 
 namespace Chocolatey.PowerShell.Commands
 {
-    [Cmdlet(VerbsData.Update, "SessionEnvironment")]
+    [Cmdlet(VerbsData.Update, "SessionEnvironment", SupportsShouldProcess = true, ConfirmImpact = ConfirmImpact.Low)]
     [OutputType(typeof(void))]
     public sealed class UpdateSessionEnvironmentCommand : ChocolateyCmdlet
     {

diff --git a/src/Chocolatey.PowerShell/Helpers/EnvironmentHelper.cs b/src/Chocolatey.PowerShell/Helpers/EnvironmentHelper.cs
@@ -139,7 +139,11 @@ public static void SetVariable(PSCmdlet cmdlet, string name, EnvironmentVariable
         {
             if (scope == EnvironmentVariableTarget.Process)
             {
-                Environment.SetEnvironmentVariable(name, value);
+                if (cmdlet.ShouldProcess(name, "Set Process environment variable"))
+                {
+                    Environment.SetEnvironmentVariable(name, value);
+                }
+
                 return;
             }
 
@@ -166,32 +170,38 @@ public static void SetVariable(PSCmdlet cmdlet, string name, EnvironmentVariable
 
                 cmdlet.WriteDebug($"Registry type for {name} is/will be {registryType}");
 
-                if (string.IsNullOrEmpty(value))
-                {
-                    registryKey.DeleteValue(name, throwOnMissingValue: false);
-                }
-                else
+                if (cmdlet.ShouldProcess(name, $"Set {scope} environment variable"))
                 {
-                    registryKey.SetValue(name, value, registryType);
+                    if (string.IsNullOrEmpty(value))
+                    {
+                        registryKey.DeleteValue(name, throwOnMissingValue: false);
+                    }
+                    else
+                    {
+                        registryKey.SetValue(name, value, registryType);
+                    }
                 }
             }
 
             try
             {
-                // Trigger environment refresh in explorer.exe:
-                // 1. Notify all windows of environment block change
-                NativeMethods.SendMessageTimeout(
-                    hWnd: (IntPtr)NativeMethods.HWND_BROADCAST,
-                    Msg: NativeMethods.WM_SETTINGCHANGE,
-                    wParam: UIntPtr.Zero,
-                    lParam: "Environment",
-                    fuFlags: 2,
-                    uTimeout: 5000,
-                    out UIntPtr result);
-
-                // 2. Set a user environment variable making the system refresh
-                var setxPath = string.Format(@"{0}\System32\setx.exe", GetVariable(cmdlet, EnvironmentVariables.SystemRoot, EnvironmentVariableTarget.Process));
-                cmdlet.InvokeCommand.InvokeScript($"& \"{setxPath}\" {EnvironmentVariables.ChocolateyLastPathUpdate} \"{DateTime.Now.ToFileTime()}\"");
+                if (cmdlet.ShouldProcess("Environment variables", "Notify system of changes"))
+                {
+                    // Trigger environment refresh in explorer.exe:
+                    // 1. Notify all windows of environment block change
+                    NativeMethods.SendMessageTimeout(
+                        hWnd: (IntPtr)NativeMethods.HWND_BROADCAST,
+                        Msg: NativeMethods.WM_SETTINGCHANGE,
+                        wParam: UIntPtr.Zero,
+                        lParam: "Environment",
+                        fuFlags: 2,
+                        uTimeout: 5000,
+                        out UIntPtr result);
+
+                    // 2. Set a user environment variable making the system refresh
+                    var setxPath = string.Format(@"{0}\System32\setx.exe", GetVariable(cmdlet, EnvironmentVariables.SystemRoot, EnvironmentVariableTarget.Process));
+                    cmdlet.InvokeCommand.InvokeScript($"& \"{setxPath}\" {EnvironmentVariables.ChocolateyLastPathUpdate} \"{DateTime.Now.ToFileTime()}\"");
+                }
             }
             catch (Exception error)
             {
@@ -221,37 +231,40 @@ public static void UpdateSession(PSCmdlet cmdlet)
                 scopeList.Add(EnvironmentVariableTarget.User);
             }
 
-            foreach (var scope in scopeList)
+            if (cmdlet.ShouldProcess("Current process", "Refresh all environment variables"))
             {
-                foreach (var name in GetVariableNames(scope))
+                foreach (var scope in scopeList)
                 {
-                    var value = GetVariable(cmdlet, name, scope);
-                    if (!string.IsNullOrEmpty(value))
+                    foreach (var name in GetVariableNames(scope))
                     {
-                        SetVariable(cmdlet, name, EnvironmentVariableTarget.Process, value);
+                        var value = GetVariable(cmdlet, name, scope);
+                        if (!string.IsNullOrEmpty(value))
+                        {
+                            SetVariable(cmdlet, name, EnvironmentVariableTarget.Process, value);
+                        }
                     }
                 }
-            }
 
-            // Update PATH, combining both scopes' values.
-            var paths = new string[2];
-            paths[0] = GetVariable(cmdlet, EnvironmentVariables.Path, EnvironmentVariableTarget.Machine);
-            paths[1] = GetVariable(cmdlet, EnvironmentVariables.Path, EnvironmentVariableTarget.User);
+                // Update PATH, combining both scopes' values.
+                var paths = new string[2];
+                paths[0] = GetVariable(cmdlet, EnvironmentVariables.Path, EnvironmentVariableTarget.Machine);
+                paths[1] = GetVariable(cmdlet, EnvironmentVariables.Path, EnvironmentVariableTarget.User);
 
-            SetVariable(cmdlet, EnvironmentVariables.Path, EnvironmentVariableTarget.Process, string.Join(";", paths));
+                SetVariable(cmdlet, EnvironmentVariables.Path, EnvironmentVariableTarget.Process, string.Join(";", paths));
 
-            // Preserve PSModulePath as it's almost always updated by process, preserve it
-            SetVariable(cmdlet, EnvironmentVariables.PSModulePath, EnvironmentVariableTarget.Process, psModulePath);
+                // Preserve PSModulePath as it's almost always updated by process, preserve it
+                SetVariable(cmdlet, EnvironmentVariables.PSModulePath, EnvironmentVariableTarget.Process, psModulePath);
 
-            // Preserve user and architecture
-            if (!string.IsNullOrEmpty(userName))
-            {
-                SetVariable(cmdlet, EnvironmentVariables.Username, EnvironmentVariableTarget.Process, userName);
-            }
+                // Preserve user and architecture
+                if (!string.IsNullOrEmpty(userName))
+                {
+                    SetVariable(cmdlet, EnvironmentVariables.Username, EnvironmentVariableTarget.Process, userName);
+                }
 
-            if (!string.IsNullOrEmpty(architecture))
-            {
-                SetVariable(cmdlet, EnvironmentVariables.ProcessorArchitecture, EnvironmentVariableTarget.Process, architecture);
+                if (!string.IsNullOrEmpty(architecture))
+                {
+                    SetVariable(cmdlet, EnvironmentVariables.ProcessorArchitecture, EnvironmentVariableTarget.Process, architecture);
+                }
             }
         }
     }