Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for passing a runsettings path to the server #6265

Merged
merged 5 commits into from
Sep 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to have one integration test to cover the test run?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add one in a followup to cover runsettings - want to make sure this one gets in for the snap

const omnisharpConfig = vscode.workspace.getConfiguration('dotnet');
await omnisharpConfig.update('unitTests.runSettingsPath', relativeRunSettingPath);

const eventWaiter = testAssetWorkspace.waitForEvent<OmnisharpRequestMessage>(
eventStream,
Expand Down
2 changes: 1 addition & 1 deletion omnisharptest/omnisharpUnitTests/fakes/fakeOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export function getEmptyOptions(): Options {
excludePaths: [],
defaultSolution: '',
unitTestDebuggingOptions: {},
runSettingsPath: '',
},
{
useModernNet: false,
Expand All @@ -40,7 +41,6 @@ export function getEmptyOptions(): Options {
sdkPath: '',
sdkVersion: '',
sdkIncludePrereleases: false,
testRunSettings: '',
dotNetCliPaths: [],
useFormatting: false,
showReferencesCodeLens: false,
Expand Down
6 changes: 2 additions & 4 deletions omnisharptest/omnisharpUnitTests/options.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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');
});
});
10 changes: 5 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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%",
Expand Down
2 changes: 1 addition & 1 deletion src/features/dotnetTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/lsptoolshost/roslynLanguageServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 5 additions & 0 deletions src/lsptoolshost/roslynProtocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
56 changes: 49 additions & 7 deletions src/lsptoolshost/unitTesting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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);
Expand Down Expand Up @@ -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;
}
4 changes: 4 additions & 0 deletions src/shared/migrateOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
11 changes: 8 additions & 3 deletions src/shared/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,12 @@ export class Options {
{},
'csharp.unitTestDebuggingOptions'
);
const runSettingsPath = Options.readOption<string>(
config,
'dotnet.unitTests.runSettingsPath',
'',
'omnisharp.testRunSettings'
);

// Omnisharp Server Options

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -312,6 +317,7 @@ export class Options {
excludePaths: excludePaths,
defaultSolution: defaultSolution,
unitTestDebuggingOptions: unitTestDebuggingOptions,
runSettingsPath: runSettingsPath,
},
{
useModernNet: useModernNet,
Expand All @@ -337,7 +343,6 @@ export class Options {
sdkPath: sdkPath,
sdkVersion: sdkVersion,
sdkIncludePrereleases: sdkIncludePrereleases,
testRunSettings: testRunSettings,
dotNetCliPaths: dotNetCliPaths,
useFormatting: useFormatting,
showReferencesCodeLens: showReferencesCodeLens,
Expand Down Expand Up @@ -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> = [
Expand Down Expand Up @@ -458,7 +464,6 @@ export interface OmnisharpServerOptions {
sdkPath: string;
sdkVersion: string;
sdkIncludePrereleases: boolean;
testRunSettings: string;
dotNetCliPaths: string[];
useFormatting: boolean;
showReferencesCodeLens: boolean;
Expand Down