Merge pull request #1664 from PowerShell/andschwa/set-var

Fix `DebuggerSetsVariablesWithConversion` test

Author: andyleejordan

src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs

@@ -400,8 +400,14 @@ public async Task<string> SetVariableAsync(int variableContainerReferenceId, str
             }
 
             VariableDetailsBase variable = variableContainer.Children[name];
-            // Determine scope in which the variable lives so we can pass it to `Get-Variable -Scope`.
-            string scope = null; // TODO: Can this use a fancy pattern matcher?
+
+            // Determine scope in which the variable lives so we can pass it to `Get-Variable
+            // -Scope`. The default is scope 0 which is safe because if a user is able to see a
+            // variable in the debugger and so change it through this interface, it's either in the
+            // top-most scope or in one of the following named scopes. The default scope is most
+            // likely in the case of changing from the "auto variables" container.
+            string scope = "0";
+            // NOTE: This can't use a switch because the IDs aren't constant.
             if (variableContainerReferenceId == localScopeVariables.Id)
             {
                 scope = VariableContainerDetails.LocalScopeName;
@@ -414,11 +420,6 @@ public async Task<string> SetVariableAsync(int variableContainerReferenceId, str
             {
                 scope = VariableContainerDetails.GlobalScopeName;
             }
-            else
-            {
-                // Hmm, this would be unexpected. No scope means do not pass GO, do not collect $200.
-                throw new Exception("Could not find the scope for this variable.");
-            }
 
             // Now that we have the scope, get the associated PSVariable object for the variable to be set.
             var getVariableCommand = new PSCommand()
@@ -456,22 +457,25 @@ public async Task<string> SetVariableAsync(int variableContainerReferenceId, str
 
             if (argTypeConverterAttr is not null)
             {
+                // PSVariable *is* strongly typed, so we have to convert it.
                 _logger.LogTrace($"Setting variable '{name}' using conversion to value: {expressionResult ?? "<null>"}");
 
-                // TODO: This is throwing a 'PSInvalidOperationException' thus causing
-                // 'DebuggerSetsVariablesWithConversion' to fail.
-                psVariable.Value = await _executionService.ExecuteDelegateAsync(
-                    "PS debugger argument converter",
-                    ExecutionOptions.Default,
-                    (pwsh, _) =>
-                    {
-                        var engineIntrinsics = (EngineIntrinsics)pwsh.Runspace.SessionStateProxy.GetVariable("ExecutionContext");
-
-                        // TODO: This is almost (but not quite) the same as LanguagePrimitives.Convert(), which does not require the pipeline thread.
-                        //       We should investigate changing it.
-                        return argTypeConverterAttr.Transform(engineIntrinsics, expressionResult);
-                    },
+                // NOTE: We use 'Get-Variable' here instead of 'SessionStateProxy.GetVariable()'
+                // because we already have a pipeline running (the debugger) and the latter cannot
+                // run concurrently (threw 'NoSessionStateProxyWhenPipelineInProgress').
+                IReadOnlyList<EngineIntrinsics> results = await _executionService.ExecutePSCommandAsync<EngineIntrinsics>(
+                    new PSCommand()
+                        .AddCommand(@"Microsoft.PowerShell.Utility\Get-Variable")
+                        .AddParameter("Name", "ExecutionContext")
+                        .AddParameter("ValueOnly"),
                     CancellationToken.None).ConfigureAwait(false);
+                EngineIntrinsics engineIntrinsics = results.Count > 0
+                    ? results[0]
+                    : throw new Exception("Couldn't get EngineIntrinsics!");
+
+                // TODO: This is almost (but not quite) the same as 'LanguagePrimitives.Convert()',
+                // which does not require the pipeline thread. We should investigate changing it.
+                psVariable.Value = argTypeConverterAttr.Transform(engineIntrinsics, expressionResult);
             }
             else
             {
@@ -641,7 +645,7 @@ private Task<VariableContainerDetails> FetchVariableContainerAsync(string scope)
 
         private async Task<VariableContainerDetails> FetchVariableContainerAsync(string scope, bool autoVarsOnly)
         {
-            PSCommand psCommand = new PSCommand().AddCommand("Get-Variable").AddParameter("Scope", scope);
+            PSCommand psCommand = new PSCommand().AddCommand(@"Microsoft.PowerShell.Utility\Get-Variable").AddParameter("Scope", scope);
 
             var scopeVariableContainer = new VariableContainerDetails(nextVariableId++, "Scope: " + scope);
             variables.Add(scopeVariableContainer);

test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs

@@ -613,7 +613,7 @@ await debugService.SetLineBreakpointsAsync(
             Assert.Equal(newGlobalIntValue, intGlobalVar.ValueString);
         }
 
-        [Fact(Skip = "Variable conversion is broken")]
+        [Fact]
         public async Task DebuggerSetsVariablesWithConversion()
         {
             await debugService.SetLineBreakpointsAsync(
@@ -628,7 +628,7 @@ await debugService.SetLineBreakpointsAsync(
             VariableDetailsBase[] variables = GetVariables(VariableContainerDetails.LocalScopeName);
 
             // Test set of a local string variable (not strongly typed but force conversion)
-            const string newStrValue = "False";
+            const string newStrValue = "\"False\"";
             const string newStrExpr = "$false";
             VariableScope localScope = Array.Find(scopes, s => s.Name == VariableContainerDetails.LocalScopeName);
             string setStrValue = await debugService.SetVariableAsync(localScope.Id, "$strVar2", newStrExpr).ConfigureAwait(true);
@@ -658,8 +658,6 @@ await debugService.SetLineBreakpointsAsync(
             var strVar = Array.Find(variables, v => v.Name == "$strVar2");
             Assert.Equal(newStrValue, strVar.ValueString);
 
-            scopes = debugService.GetVariableScopes(0);
-
             // Test set of script scope bool variable (strongly typed)
             variables = GetVariables(VariableContainerDetails.ScriptScopeName);
             var boolVar = Array.Find(variables, v => v.Name == "$scriptBool");

test/PowerShellEditorServices.Test/PsesHostFactory.cs

@@ -20,21 +20,13 @@ internal static class PsesHostFactory
         // NOTE: These paths are arbitrarily chosen just to verify that the profile paths can be set
         // to whatever they need to be for the given host.
 
-        public static readonly ProfilePathInfo TestProfilePaths =
-            new(
-                    Path.GetFullPath(
-                        TestUtilities.NormalizePath("../../../../PowerShellEditorServices.Test.Shared/Profile/Test.PowerShellEditorServices_profile.ps1")),
-                    Path.GetFullPath(
-                        TestUtilities.NormalizePath("../../../../PowerShellEditorServices.Test.Shared/Profile/ProfileTest.ps1")),
-                    Path.GetFullPath(
-                        TestUtilities.NormalizePath("../../../../PowerShellEditorServices.Test.Shared/Test.PowerShellEditorServices_profile.ps1")),
-                    Path.GetFullPath(
-                        TestUtilities.NormalizePath("../../../../PowerShellEditorServices.Test.Shared/ProfileTest.ps1")));
+        public static readonly ProfilePathInfo TestProfilePaths = new(
+            Path.GetFullPath(TestUtilities.NormalizePath("../../../../PowerShellEditorServices.Test.Shared/Profile/Test.PowerShellEditorServices_profile.ps1")),
+            Path.GetFullPath(TestUtilities.NormalizePath("../../../../PowerShellEditorServices.Test.Shared/Profile/ProfileTest.ps1")),
+            Path.GetFullPath(TestUtilities.NormalizePath("../../../../PowerShellEditorServices.Test.Shared/Test.PowerShellEditorServices_profile.ps1")),
+            Path.GetFullPath(TestUtilities.NormalizePath("../../../../PowerShellEditorServices.Test.Shared/ProfileTest.ps1")));
 
-        public static readonly string BundledModulePath = Path.GetFullPath(
-            TestUtilities.NormalizePath("../../../../../module"));
-
-        public static System.Management.Automation.Runspaces.Runspace InitialRunspace;
+        public static readonly string BundledModulePath = Path.GetFullPath(TestUtilities.NormalizePath("../../../../../module"));
 
         public static PsesInternalHost Create(ILoggerFactory loggerFactory)
         {
@@ -53,25 +45,24 @@ public static PsesInternalHost Create(ILoggerFactory loggerFactory)
             }
 
             HostStartupInfo testHostDetails = new(
-                "PowerShell Editor Services Test Host",
-                "Test.PowerShellEditorServices",
-                new Version("1.0.0"),
+                name: "PowerShell Editor Services Test Host",
+                profileId: "Test.PowerShellEditorServices",
+                version: new Version("1.0.0"),
                 psHost: new NullPSHost(),
-                TestProfilePaths,
+                profilePaths: TestProfilePaths,
                 featureFlags: Array.Empty<string>(),
                 additionalModules: Array.Empty<string>(),
-                initialSessionState,
+                initialSessionState: initialSessionState,
                 logPath: null,
-                (int)LogLevel.None,
+                logLevel: (int)LogLevel.None,
                 consoleReplEnabled: false,
                 usesLegacyReadLine: false,
                 bundledModulePath: BundledModulePath);
 
             var psesHost = new PsesInternalHost(loggerFactory, null, testHostDetails);
 
             // NOTE: Because this is used by constructors it can't use await.
-            // TODO: Should we actually load profiles here?
-            if (psesHost.TryStartAsync(new HostStartOptions { LoadProfiles = true }, CancellationToken.None).GetAwaiter().GetResult())
+            if (psesHost.TryStartAsync(new HostStartOptions { LoadProfiles = false }, CancellationToken.None).GetAwaiter().GetResult())
             {
                 return psesHost;
             }

test/PowerShellEditorServices.Test/Session/PsesInternalHostTests.cs

@@ -94,6 +94,7 @@ await Assert.ThrowsAsync<TaskCanceledException>(() =>
         }
 
         [Fact]
+        [System.Diagnostics.CodeAnalysis.SuppressMessage("Usage", "VSTHRD003:Avoid awaiting foreign Tasks", Justification = "Explicitly checking task cancellation status.")]
         public async Task CanCancelExecutionWithMethod()
         {
             var executeTask = psesHost.ExecutePSCommandAsync(