Merge pull request #6265 from dibarbet/runsettings

Add support for passing a runsettings path to the server
dotnet · Sep 11, 2023 · f0f0e4a · f0f0e4a
2 parents 097b7ed + 67425d6
commit f0f0e4a
Show file tree

Hide file tree

Showing 11 changed files with 94 additions and 28 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,12 +8,24 @@
 - Debug from .csproj and .sln [#5876](https://github.com/dotnet/vscode-csharp/issues/5876)
 
 ## Latest
+* Add support for specifying a .runsettings file when using Roslyn LSP (PR: [#6265](https://github.com/dotnet/vscode-csharp/pull/6265))
+* Update Roslyn version (PR: [#6265](https://github.com/dotnet/vscode-csharp/pull/6265))
+  * Add server support for .runsettings in unit tests (PR: [#69792](https://github.com/dotnet/roslyn/pull/69792))
+  * Log more information when we're unable to parse a URI (PR: [#69840](https://github.com/dotnet/roslyn/pull/69840))
+  * Bump ICSharpCode.Decompiler to 8.1.0.745 (PR: [#69772](https://github.com/dotnet/roslyn/pull/69772))
+  * Fix override completion erroring when framework assemblies are not found (PR: [#69795](https://github.com/dotnet/roslyn/pull/69795))
+* Fix override completion when drive letter casing does not match (PR: [#6315](https://github.com/dotnet/vscode-csharp/pull/6315))
+* Allow the server path to be specified by the `DOTNET_ROSLYN_SERVER_PATH` environment variable (PR: [#6316](https://github.com/dotnet/vscode-csharp/pull/6316))
+
+## 2.1.10
 * Update Roslyn version
   * Includes better support for .NET 8 and .NET Framework-targeting projects (PR: [#69616](https://github.com/dotnet/roslyn/pull/69616))
     * This should fix a number of reports where projects don't have full IntelliSense. .NET Framework projects on Windows should load without errors. .NET Framework targeting projects on Mac and Linux which would use Mono are still processed as if they are .NET Core projects and may not load correctly; support for Mono is coming in a future update.
   * Fix issues where some projects fail to load being unable to find NuGet.Frameworks (PR: [#69824](https://github.com/dotnet/roslyn/pull/69824))
 * Update Razor to 7.0.0-preview.23455.5 (PR: [#6291](https://github.com/dotnet/vscode-csharp/pull/6291))
   * Fixes issue reading razor.format.enable and other options (PR: [dotnet/razor#9240](https://github.com/dotnet/razor/issues/9240))
+* Fix parsing of tasks.json with comments in certain locations (PR: [#6288](https://github.com/dotnet/vscode-csharp/pull/6288))
+* Fix Razor browser discovery issues on Mac and Linux `DOTNET_ROSLYN_SERVER_PATH` environment variable (PR: [#6269](https://github.com/dotnet/vscode-csharp/pull/6269))
 
 ## 2.0.448
 * Update Roslyn version (PR: [#6264](https://github.com/dotnet/vscode-csharp/pull/6264))

diff --git a/omnisharptest/omnisharpIntegrationTests/dotnetTest.integration.test.ts b/omnisharptest/omnisharpIntegrationTests/dotnetTest.integration.test.ts
@@ -47,8 +47,8 @@ suite(`DotnetTest: ${testAssetWorkspace.description}`, function () {
     });
 
     test('Undefined runsettings path is unchanged', async function () {
-        const omnisharpConfig = vscode.workspace.getConfiguration('omnisharp');
-        await omnisharpConfig.update('testRunSettings', undefined);
+        const omnisharpConfig = vscode.workspace.getConfiguration('dotnet');
+        await omnisharpConfig.update('unitTests.runSettingsPath', undefined);
 
         const eventWaiter = testAssetWorkspace.waitForEvent<OmnisharpRequestMessage>(
             eventStream,
@@ -69,8 +69,8 @@ suite(`DotnetTest: ${testAssetWorkspace.description}`, function () {
         const relativeRunSettingsPath = `.\\settings\\TestSettings.runsettings`.replace('\\', path.sep);
         const absoluteRunSettingsPath = path.join(process.cwd(), relativeRunSettingsPath);
 
-        const omnisharpConfig = vscode.workspace.getConfiguration('omnisharp');
-        await omnisharpConfig.update('testRunSettings', absoluteRunSettingsPath);
+        const omnisharpConfig = vscode.workspace.getConfiguration('dotnet');
+        await omnisharpConfig.update('unitTests.runSettingsPath', absoluteRunSettingsPath);
 
         const eventWaiter = testAssetWorkspace.waitForEvent<OmnisharpRequestMessage>(
             eventStream,
@@ -91,8 +91,8 @@ suite(`DotnetTest: ${testAssetWorkspace.description}`, function () {
         const endingPath = 'settings\\TestSettings.runsettings'.replace('\\', path.sep);
         const relativeRunSettingPath = `.\\${endingPath}`.replace('\\', path.sep);
 
-        const omnisharpConfig = vscode.workspace.getConfiguration('omnisharp');
-        await omnisharpConfig.update('testRunSettings', relativeRunSettingPath);
+        const omnisharpConfig = vscode.workspace.getConfiguration('dotnet');
+        await omnisharpConfig.update('unitTests.runSettingsPath', relativeRunSettingPath);
 
         const eventWaiter = testAssetWorkspace.waitForEvent<OmnisharpRequestMessage>(
             eventStream,

diff --git a/omnisharptest/omnisharpUnitTests/fakes/fakeOptions.ts b/omnisharptest/omnisharpUnitTests/fakes/fakeOptions.ts
@@ -15,6 +15,7 @@ export function getEmptyOptions(): Options {
             excludePaths: [],
             defaultSolution: '',
             unitTestDebuggingOptions: {},
+            runSettingsPath: '',
         },
         {
             useModernNet: false,
@@ -40,7 +41,6 @@ export function getEmptyOptions(): Options {
             sdkPath: '',
             sdkVersion: '',
             sdkIncludePrereleases: false,
-            testRunSettings: '',
             dotNetCliPaths: [],
             useFormatting: false,
             showReferencesCodeLens: false,

diff --git a/omnisharptest/omnisharpUnitTests/options.test.ts b/omnisharptest/omnisharpUnitTests/options.test.ts
@@ -38,7 +38,7 @@ suite('Options tests', () => {
         options.omnisharpOptions.enableImportCompletion.should.equal(false);
         options.omnisharpOptions.enableAsyncCompletion.should.equal(false);
         options.omnisharpOptions.analyzeOpenDocumentsOnly.should.equal(true);
-        options.omnisharpOptions.testRunSettings.should.equal('');
+        options.commonOptions.runSettingsPath.should.equal('');
     });
 
     test('Verify return no excluded paths when files.exclude empty', () => {
@@ -128,8 +128,6 @@ suite('Options tests', () => {
 
         const options = Options.Read(vscode);
 
-        options.omnisharpOptions.testRunSettings.should.equal(
-            'some_valid_path\\some_valid_runsettings_files.runsettings'
-        );
+        options.commonOptions.runSettingsPath.should.equal('some_valid_path\\some_valid_runsettings_files.runsettings');
     });
 });
diff --git a/package.json b/package.json
@@ -37,7 +37,7 @@
     }
   },
   "defaults": {
-    "roslyn": "4.8.0-3.23455.11",
+    "roslyn": "4.8.0-3.23458.4",
     "omniSharp": "1.39.7",
     "razor": "7.0.0-preview.23455.5",
     "razorOmnisharp": "7.0.0-preview.23363.1"
@@ -1087,10 +1087,6 @@
             "default": false,
             "description": "(EXPERIMENTAL) Enables support for resolving completion edits asynchronously. This can speed up time to show the completion list, particularly override and partial method completion lists, at the cost of slight delays after inserting a completion item. Most completion items will have no noticeable impact with this feature, but typing immediately after inserting an override or partial method completion, before the insert is completed, can have unpredictable results."
           },
-          "omnisharp.testRunSettings": {
-            "type": "string",
-            "description": "Path to the .runsettings file which should be used when running unit tests."
-          },
           "omnisharp.dotNetCliPaths": {
             "type": "array",
             "items": {
@@ -1364,6 +1360,10 @@
             "description": "%configuration.dotnet.symbolSearch.searchReferenceAssemblies%",
             "order": 80
           },
+          "dotnet.unitTests.runSettingsPath": {
+            "type": "string",
+            "description": "Path to the .runsettings file which should be used when running unit tests. (Previously `omnisharp.testRunSettings`)"
+          },
           "dotnet.unitTestDebuggingOptions": {
             "type": "object",
             "description": "%configuration.dotnet.unitTestDebuggingOptions%",

diff --git a/src/features/dotnetTest.ts b/src/features/dotnetTest.ts
@@ -233,7 +233,7 @@ export default class TestManager extends AbstractProvider {
     }
 
     private _getRunSettings(filename: string): string | undefined {
-        const testSettingsPath = this.optionProvider.GetLatestOptions().omnisharpOptions.testRunSettings;
+        const testSettingsPath = this.optionProvider.GetLatestOptions().commonOptions.runSettingsPath;
         if (testSettingsPath.length === 0) {
             return undefined;
         }

diff --git a/src/lsptoolshost/roslynLanguageServer.ts b/src/lsptoolshost/roslynLanguageServer.ts
@@ -748,7 +748,7 @@ export async function activateRoslynLanguageServer(
 
     registerRazorCommands(context, languageServer);
 
-    registerUnitTestingCommands(context, languageServer, dotnetTestChannel);
+    registerUnitTestingCommands(context, languageServer, dotnetTestChannel, optionProvider);
 
     // Register any needed debugger components that need to communicate with the language server.
     registerDebugger(context, languageServer, languageServerEvents, platformInfo, optionProvider, _channel);

diff --git a/src/lsptoolshost/roslynProtocol.ts b/src/lsptoolshost/roslynProtocol.ts
@@ -83,6 +83,11 @@ export interface RunTestsParams extends lsp.WorkDoneProgressParams, lsp.PartialR
      * Whether the request should attempt to call back to the client to attach a debugger before running the tests.
      */
     attachDebugger: boolean;
+
+    /**
+     * The absolute path to a .runsettings file to configure the test run.
+     */
+    runSettingsPath?: string;
 }
 
 export interface TestProgress {

diff --git a/src/lsptoolshost/unitTesting.ts b/src/lsptoolshost/unitTesting.ts
@@ -3,43 +3,51 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
+import * as fs from 'fs';
+import * as path from 'path';
 import * as vscode from 'vscode';
 import * as languageClient from 'vscode-languageclient/node';
 import { RoslynLanguageServer } from './roslynLanguageServer';
 import { RunTestsParams, RunTestsPartialResult, RunTestsRequest, TestProgress } from './roslynProtocol';
+import OptionProvider from '../shared/observers/optionProvider';
 
 export function registerUnitTestingCommands(
     context: vscode.ExtensionContext,
     languageServer: RoslynLanguageServer,
-    dotnetTestChannel: vscode.OutputChannel
+    dotnetTestChannel: vscode.OutputChannel,
+    optionProvider: OptionProvider
 ) {
     context.subscriptions.push(
         vscode.commands.registerCommand(
             'dotnet.test.run',
-            async (request): Promise<TestProgress | undefined> => runTests(request, languageServer, dotnetTestChannel)
+            async (request): Promise<TestProgress | undefined> =>
+                runTests(request, languageServer, dotnetTestChannel, optionProvider)
         )
     );
     context.subscriptions.push(
         // We don't use registerTextEditorCommand because it is required to run synchronously and is not awaitable.
         // See https://github.com/microsoft/vscode/issues/16814 for more info.
         vscode.commands.registerCommand(
             'dotnet.test.runTestsInContext',
-            async (): Promise<TestProgress | undefined> => runTestsInContext(false, languageServer, dotnetTestChannel)
+            async (): Promise<TestProgress | undefined> =>
+                runTestsInContext(false, languageServer, dotnetTestChannel, optionProvider)
         )
     );
 
     context.subscriptions.push(
         vscode.commands.registerCommand(
             'dotnet.test.debugTestsInContext',
-            async (): Promise<TestProgress | undefined> => runTestsInContext(true, languageServer, dotnetTestChannel)
+            async (): Promise<TestProgress | undefined> =>
+                runTestsInContext(true, languageServer, dotnetTestChannel, optionProvider)
         )
     );
 }
 
 async function runTestsInContext(
     debug: boolean,
     languageServer: RoslynLanguageServer,
-    dotnetTestChannel: vscode.OutputChannel
+    dotnetTestChannel: vscode.OutputChannel,
+    optionProvider: OptionProvider
 ): Promise<TestProgress | undefined> {
     const activeEditor = vscode.window.activeTextEditor;
     if (!activeEditor) {
@@ -51,21 +59,24 @@ async function runTestsInContext(
     };
     const textDocument: languageClient.TextDocumentIdentifier = { uri: activeEditor.document.fileName };
     const request: RunTestsParams = { textDocument: textDocument, range: contextRange, attachDebugger: debug };
-    return runTests(request, languageServer, dotnetTestChannel);
+    return runTests(request, languageServer, dotnetTestChannel, optionProvider);
 }
 
 let _testRunInProgress = false;
 
 async function runTests(
     request: RunTestsParams,
     languageServer: RoslynLanguageServer,
-    dotnetTestChannel: vscode.OutputChannel
+    dotnetTestChannel: vscode.OutputChannel,
+    optionProvider: OptionProvider
 ): Promise<TestProgress | undefined> {
     if (_testRunInProgress) {
         vscode.window.showErrorMessage('Test run already in progress');
         return;
     }
 
+    request.runSettingsPath = getRunSettings(request.textDocument.uri, optionProvider, dotnetTestChannel);
+
     _testRunInProgress = true;
 
     dotnetTestChannel.show(true);
@@ -138,3 +149,34 @@ async function runTests(
 
     return lastProgress;
 }
+
+function getRunSettings(
+    documentUri: string,
+    optionProvider: OptionProvider,
+    dotnetTestChannel: vscode.OutputChannel
+): string | undefined {
+    const runSettingsPathOption = optionProvider.GetLatestOptions().commonOptions.runSettingsPath;
+    if (runSettingsPathOption.length === 0) {
+        return undefined;
+    }
+
+    let absolutePath = runSettingsPathOption;
+    if (!path.isAbsolute(runSettingsPathOption)) {
+        // Path is relative to the workspace. Create absolute path.
+        const workspaceFolder = vscode.workspace.getWorkspaceFolder(vscode.Uri.parse(documentUri));
+        if (workspaceFolder === undefined) {
+            dotnetTestChannel.appendLine(
+                `Warning: Unable to find workspace folder for ${documentUri}, cannot resolve run settings path ${runSettingsPathOption}.`
+            );
+            return undefined;
+        }
+        absolutePath = path.join(workspaceFolder.uri.fsPath, runSettingsPathOption);
+    }
+
+    if (!fs.existsSync(absolutePath)) {
+        dotnetTestChannel.appendLine(`Warning: Unable to find run settings file at ${absolutePath}.`);
+        return undefined;
+    }
+
+    return absolutePath;
+}
diff --git a/src/shared/migrateOptions.ts b/src/shared/migrateOptions.ts
@@ -70,6 +70,10 @@ export const migrateOptions = [
         omnisharpOption: 'csharp.unitTestDebuggingOptions',
         roslynOption: 'dotnet.unitTestDebuggingOptions',
     },
+    {
+        omnisharpOption: 'omnisharp.testRunSettings',
+        roslynOption: 'dotnet.unitTests.runSettingsPath',
+    },
 ];
 
 export async function MigrateOptions(vscode: vscode): Promise<void> {

diff --git a/src/shared/options.ts b/src/shared/options.ts
@@ -97,6 +97,12 @@ export class Options {
             {},
             'csharp.unitTestDebuggingOptions'
         );
+        const runSettingsPath = Options.readOption<string>(
+            config,
+            'dotnet.unitTests.runSettingsPath',
+            '',
+            'omnisharp.testRunSettings'
+        );
 
         // Omnisharp Server Options
 
@@ -179,7 +185,6 @@ export class Options {
             'omnisharp.enableMsBuildLoadProjectsOnDemand',
             false
         );
-        const testRunSettings = Options.readOption<string>(config, 'omnisharp.testRunSettings', '');
         const dotNetCliPaths = Options.readOption<string[]>(config, 'omnisharp.dotNetCliPaths', []);
 
         const useFormatting = Options.readOption<boolean>(config, 'csharp.format.enable', true);
@@ -312,6 +317,7 @@ export class Options {
                 excludePaths: excludePaths,
                 defaultSolution: defaultSolution,
                 unitTestDebuggingOptions: unitTestDebuggingOptions,
+                runSettingsPath: runSettingsPath,
             },
             {
                 useModernNet: useModernNet,
@@ -337,7 +343,6 @@ export class Options {
                 sdkPath: sdkPath,
                 sdkVersion: sdkVersion,
                 sdkIncludePrereleases: sdkIncludePrereleases,
-                testRunSettings: testRunSettings,
                 dotNetCliPaths: dotNetCliPaths,
                 useFormatting: useFormatting,
                 showReferencesCodeLens: showReferencesCodeLens,
@@ -425,6 +430,7 @@ export interface CommonOptions {
     /** The default solution; this has been normalized to a full file path from the workspace folder it was configured in, or the string "disable" if that has been disabled */
     defaultSolution: string;
     unitTestDebuggingOptions: object;
+    runSettingsPath: string;
 }
 
 export const CommonOptionsThatTriggerReload: ReadonlyArray<keyof CommonOptions> = [
@@ -458,7 +464,6 @@ export interface OmnisharpServerOptions {
     sdkPath: string;
     sdkVersion: string;
     sdkIncludePrereleases: boolean;
-    testRunSettings: string;
     dotNetCliPaths: string[];
     useFormatting: boolean;
     showReferencesCodeLens: boolean;