Review feedback

dotnet · Jan 5, 2024 · 202d445 · 202d445
1 parent 33d6c79
commit 202d445
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 46 deletions.
diff --git a/omnisharptest/omnisharpIntegrationTests/omnisharpCommands.integration.test.ts b/omnisharptest/omnisharpIntegrationTests/omnisharpCommands.integration.test.ts
@@ -7,6 +7,7 @@ import { expect, test, beforeAll, afterAll, describe } from '@jest/globals';
 import * as vscode from 'vscode';
 import { activateCSharpExtension } from './integrationHelpers';
 import testAssetWorkspace from './testAssets/activeTestAssetWorkspace';
+import { CommonCommands, OmniSharpCommands } from '../../test/integrationTests/commandEnablement.integration.test';
 
 describe(`Command Enablement: ${testAssetWorkspace.description}`, function () {
     beforeAll(async function () {
@@ -22,28 +23,17 @@ describe(`Command Enablement: ${testAssetWorkspace.description}`, function () {
     test('Only expected commands are available', async function () {
         const commands = await vscode.commands.getCommands(true);
 
-        // Ensure the O# commands are available.
-        expect(commands).toContain('o.restart');
-        expect(commands).toContain('o.pickProjectAndStart');
-        expect(commands).toContain('o.fixAll.solution');
-        expect(commands).toContain('o.fixAll.project');
-        expect(commands).toContain('o.fixAll.document');
-        expect(commands).toContain('o.reanalyze.allProjects');
-        expect(commands).toContain('o.reanalyze.currentProject');
-        expect(commands).toContain('dotnet.generateAssets');
-        expect(commands).toContain('dotnet.restore.project');
-        expect(commands).toContain('dotnet.restore.all');
-        expect(commands).toContain('dotnet.test.runTestsInContext');
-        expect(commands).toContain('dotnet.test.debugTestsInContext');
-        expect(commands).toContain('csharp.listProcess');
-        expect(commands).toContain('csharp.listRemoteProcess');
-        expect(commands).toContain('csharp.listRemoteDockerProcess');
-        expect(commands).toContain('csharp.attachToProcess');
-        expect(commands).toContain('csharp.reportIssue');
-        expect(commands).toContain('csharp.showDecompilationTerms');
+        // Ensure O# commands are available.
+        OmniSharpCommands.forEach((command) => {
+            expect(commands).toContain(command);
+        });
+        CommonCommands.forEach((command) => {
+            expect(commands).toContain(command);
+        });
 
-        // Ensure the non-O# commands are not available.
-        expect(commands).not.toContain('dotnet.openSolution');
-        expect(commands).not.toContain('dotnet.restartServer');
+        // Ensure Roslyn standalone commands are not available.
+        OmniSharpCommands.forEach((command) => {
+            expect(commands).not.toContain(command);
+        });
     });
 });
diff --git a/package.json b/package.json
@@ -5673,12 +5673,12 @@
       "editor/context": [
         {
           "command": "dotnet.test.runTestsInContext",
-          "when": "editorLangId == csharp && dotnet.server.activationContext == 'Roslyn' || dotnet.server.activationContext == 'OmniSharp'",
+          "when": "editorLangId == csharp && (dotnet.server.activationContext == 'Roslyn' || dotnet.server.activationContext == 'OmniSharp')",
           "group": "2_dotnet@1"
         },
         {
           "command": "dotnet.test.debugTestsInContext",
-          "when": "editorLangId == csharp && dotnet.server.activationContext == 'Roslyn' || dotnet.server.activationContext == 'OmniSharp'",
+          "when": "editorLangId == csharp && (dotnet.server.activationContext == 'Roslyn' || dotnet.server.activationContext == 'OmniSharp')",
           "group": "2_dotnet@2"
         }
       ]

diff --git a/src/lsptoolshost/roslynLanguageServer.ts b/src/lsptoolshost/roslynLanguageServer.ts
@@ -518,7 +518,7 @@ export class RoslynLanguageServer {
                 _channel.appendLine('Activating C# + C# Dev Kit...');
             }
 
-            // Set command enablement to use DevKit commands.
+            // Set command enablement as soon as we know devkit is available.
             vscode.commands.executeCommand('setContext', 'dotnet.server.activationContext', 'RoslynDevKit');
 
             const csharpDevKitArgs = this.getCSharpDevKitExportArgs();

diff --git a/test/integrationTests/commandEnablement.integration.test.ts b/test/integrationTests/commandEnablement.integration.test.ts
@@ -21,26 +21,41 @@ describe(`Command Enablement: ${testAssetWorkspace.description}`, function () {
         const commands = await vscode.commands.getCommands(true);
 
         // Ensure the standalone Roslyn commands are available.
-        expect(commands).toContain('dotnet.openSolution');
-        expect(commands).toContain('dotnet.restartServer');
-        expect(commands).toContain('dotnet.generateAssets');
-        expect(commands).toContain('dotnet.restore.project');
-        expect(commands).toContain('dotnet.restore.all');
-        expect(commands).toContain('dotnet.test.runTestsInContext');
-        expect(commands).toContain('dotnet.test.debugTestsInContext');
-        expect(commands).toContain('csharp.listProcess');
-        expect(commands).toContain('csharp.listRemoteProcess');
-        expect(commands).toContain('csharp.listRemoteDockerProcess');
-        expect(commands).toContain('csharp.attachToProcess');
-        expect(commands).toContain('csharp.reportIssue');
-
-        // Ensure the O#-only commands are not available.
-        expect(commands).not.toContain('o.restart');
-        expect(commands).not.toContain('o.pickProjectAndStart');
-        expect(commands).not.toContain('o.fixAll.solution');
-        expect(commands).not.toContain('o.fixAll.project');
-        expect(commands).not.toContain('o.fixAll.document');
-        expect(commands).not.toContain('o.reanalyze.allProjects');
-        expect(commands).not.toContain('o.reanalyze.currentProject');
+        RoslynCommands.forEach((command) => {
+            expect(commands).toContain(command);
+        });
+        CommonCommands.forEach((command) => {
+            expect(commands).toContain(command);
+        });
+
+        // Ensure O# commands are not available.
+        OmniSharpCommands.forEach((command) => {
+            expect(commands).not.toContain(command);
+        });
     });
 });
+
+export const OmniSharpCommands = [
+    'o.restart',
+    'o.pickProjectAndStart',
+    'o.fixAll.solution',
+    'o.fixAll.project',
+    'o.fixAll.document',
+    'o.reanalyze.allProjects',
+    'o.reanalyze.currentProject',
+];
+
+export const RoslynCommands = ['dotnet.openSolution', 'dotnet.restartServer'];
+
+export const CommonCommands = [
+    'dotnet.generateAssets',
+    'dotnet.restore.project',
+    'dotnet.restore.all',
+    'dotnet.test.runTestsInContext',
+    'dotnet.test.debugTestsInContext',
+    'csharp.listProcess',
+    'csharp.listRemoteProcess',
+    'csharp.listRemoteDockerProcess',
+    'csharp.attachToProcess',
+    'csharp.reportIssue',
+];