Skip to content

Commit 4286765

Browse files
dibarbetdibarbet
committed
Review feedback
1 parent 33d6c79 commit 4286765

File tree

5 files changed

+57
-47
lines changed

5 files changed

+57
-47
lines changed

‎omnisharptest/omnisharpIntegrationTests/omnisharpCommands.integration.test.ts

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { expect, test, beforeAll, afterAll, describe } from '@jest/globals';
77
import * as vscode from 'vscode';
88
import { activateCSharpExtension } from './integrationHelpers';
99
import testAssetWorkspace from './testAssets/activeTestAssetWorkspace';
10+
import { CommonCommands, OmniSharpCommands, RoslynCommands } from '../../test/integrationTests/expectedCommands';
1011

1112
describe(`Command Enablement: ${testAssetWorkspace.description}`, function () {
1213
beforeAll(async function () {
@@ -19,31 +20,20 @@ describe(`Command Enablement: ${testAssetWorkspace.description}`, function () {
1920
await testAssetWorkspace.cleanupWorkspace();
2021
});
2122

22-
test('Only expected commands are available', async function () {
23+
test('Omnisharp commands are available', async function () {
2324
const commands = await vscode.commands.getCommands(true);
2425

25-
// Ensure the O# commands are available.
26-
expect(commands).toContain('o.restart');
27-
expect(commands).toContain('o.pickProjectAndStart');
28-
expect(commands).toContain('o.fixAll.solution');
29-
expect(commands).toContain('o.fixAll.project');
30-
expect(commands).toContain('o.fixAll.document');
31-
expect(commands).toContain('o.reanalyze.allProjects');
32-
expect(commands).toContain('o.reanalyze.currentProject');
33-
expect(commands).toContain('dotnet.generateAssets');
34-
expect(commands).toContain('dotnet.restore.project');
35-
expect(commands).toContain('dotnet.restore.all');
36-
expect(commands).toContain('dotnet.test.runTestsInContext');
37-
expect(commands).toContain('dotnet.test.debugTestsInContext');
38-
expect(commands).toContain('csharp.listProcess');
39-
expect(commands).toContain('csharp.listRemoteProcess');
40-
expect(commands).toContain('csharp.listRemoteDockerProcess');
41-
expect(commands).toContain('csharp.attachToProcess');
42-
expect(commands).toContain('csharp.reportIssue');
43-
expect(commands).toContain('csharp.showDecompilationTerms');
26+
// Ensure O# commands are available.
27+
OmniSharpCommands.forEach((command) => {
28+
expect(commands).toContain(command);
29+
});
30+
CommonCommands.forEach((command) => {
31+
expect(commands).toContain(command);
32+
});
4433

45-
// Ensure the non-O# commands are not available.
46-
expect(commands).not.toContain('dotnet.openSolution');
47-
expect(commands).not.toContain('dotnet.restartServer');
34+
// Ensure Roslyn standalone commands are not available.
35+
RoslynCommands.forEach((command) => {
36+
expect(commands).not.toContain(command);
37+
});
4838
});
4939
});

‎package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5673,12 +5673,12 @@
56735673
"editor/context": [
56745674
{
56755675
"command": "dotnet.test.runTestsInContext",
5676-
"when": "editorLangId == csharp && dotnet.server.activationContext == 'Roslyn' || dotnet.server.activationContext == 'OmniSharp'",
5676+
"when": "editorLangId == csharp && (dotnet.server.activationContext == 'Roslyn' || dotnet.server.activationContext == 'OmniSharp')",
56775677
"group": "2_dotnet@1"
56785678
},
56795679
{
56805680
"command": "dotnet.test.debugTestsInContext",
5681-
"when": "editorLangId == csharp && dotnet.server.activationContext == 'Roslyn' || dotnet.server.activationContext == 'OmniSharp'",
5681+
"when": "editorLangId == csharp && (dotnet.server.activationContext == 'Roslyn' || dotnet.server.activationContext == 'OmniSharp')",
56825682
"group": "2_dotnet@2"
56835683
}
56845684
]

‎src/lsptoolshost/roslynLanguageServer.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,7 @@ export class RoslynLanguageServer {
518518
_channel.appendLine('Activating C# + C# Dev Kit...');
519519
}
520520

521-
// Set command enablement to use DevKit commands.
521+
// Set command enablement as soon as we know devkit is available.
522522
vscode.commands.executeCommand('setContext', 'dotnet.server.activationContext', 'RoslynDevKit');
523523

524524
const csharpDevKitArgs = this.getCSharpDevKitExportArgs();

‎test/integrationTests/commandEnablement.integration.test.ts

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { expect, test, beforeAll, afterAll, describe } from '@jest/globals';
77
import * as vscode from 'vscode';
88
import { activateCSharpExtension } from './integrationHelpers';
99
import testAssetWorkspace from './testAssets/testAssetWorkspace';
10+
import { CommonCommands, OmniSharpCommands, RoslynCommands } from './expectedCommands';
1011

1112
describe(`Command Enablement: ${testAssetWorkspace.description}`, function () {
1213
beforeAll(async function () {
@@ -17,30 +18,20 @@ describe(`Command Enablement: ${testAssetWorkspace.description}`, function () {
1718
await testAssetWorkspace.cleanupWorkspace();
1819
});
1920

20-
test('Only expected commands are available', async function () {
21+
test('Roslyn commands are available', async function () {
2122
const commands = await vscode.commands.getCommands(true);
2223

2324
// Ensure the standalone Roslyn commands are available.
24-
expect(commands).toContain('dotnet.openSolution');
25-
expect(commands).toContain('dotnet.restartServer');
26-
expect(commands).toContain('dotnet.generateAssets');
27-
expect(commands).toContain('dotnet.restore.project');
28-
expect(commands).toContain('dotnet.restore.all');
29-
expect(commands).toContain('dotnet.test.runTestsInContext');
30-
expect(commands).toContain('dotnet.test.debugTestsInContext');
31-
expect(commands).toContain('csharp.listProcess');
32-
expect(commands).toContain('csharp.listRemoteProcess');
33-
expect(commands).toContain('csharp.listRemoteDockerProcess');
34-
expect(commands).toContain('csharp.attachToProcess');
35-
expect(commands).toContain('csharp.reportIssue');
25+
RoslynCommands.forEach((command) => {
26+
expect(commands).toContain(command);
27+
});
28+
CommonCommands.forEach((command) => {
29+
expect(commands).toContain(command);
30+
});
3631

37-
// Ensure the O#-only commands are not available.
38-
expect(commands).not.toContain('o.restart');
39-
expect(commands).not.toContain('o.pickProjectAndStart');
40-
expect(commands).not.toContain('o.fixAll.solution');
41-
expect(commands).not.toContain('o.fixAll.project');
42-
expect(commands).not.toContain('o.fixAll.document');
43-
expect(commands).not.toContain('o.reanalyze.allProjects');
44-
expect(commands).not.toContain('o.reanalyze.currentProject');
32+
// Ensure O# commands are not available.
33+
OmniSharpCommands.forEach((command) => {
34+
expect(commands).not.toContain(command);
35+
});
4536
});
4637
});

‎test/integrationTests/expectedCommands.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
export const OmniSharpCommands = [
7+
'o.restart',
8+
'o.pickProjectAndStart',
9+
'o.fixAll.solution',
10+
'o.fixAll.project',
11+
'o.fixAll.document',
12+
'o.reanalyze.allProjects',
13+
'o.reanalyze.currentProject',
14+
];
15+
16+
export const RoslynCommands = ['dotnet.openSolution', 'dotnet.restartServer'];
17+
18+
export const CommonCommands = [
19+
'dotnet.generateAssets',
20+
'dotnet.restore.project',
21+
'dotnet.restore.all',
22+
'dotnet.test.runTestsInContext',
23+
'dotnet.test.debugTestsInContext',
24+
'csharp.listProcess',
25+
'csharp.listRemoteProcess',
26+
'csharp.listRemoteDockerProcess',
27+
'csharp.attachToProcess',
28+
'csharp.reportIssue',
29+
];

0 commit comments

Comments
 (0)