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 reading runsettings passed by the client #69792

Merged
merged 2 commits into from
Sep 8, 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
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,9 @@
<data name="Failed_0_Passed_1_Skipped_2_Total_3_Duration_4" xml:space="preserve">
<value>Failed: {0}, Passed: {1}, Skipped: {2}, Total: {3}, Duration: {4}</value>
</data>
<data name="Failed_to_read_runsettings_file_at_0:1" xml:space="preserve">
<value>Failed to read .runsettings file at {0}:{1}</value>
</data>
<data name="Found_0_tests_in_1" xml:space="preserve">
<value>Found {0} tests in {1}</value>
</data>
Expand All @@ -162,6 +165,9 @@
<data name="Running_tests" xml:space="preserve">
<value>Running tests...</value>
</data>
<data name="Runsettings_file_does_not_exist_at_0" xml:space="preserve">
<value>.runsettings file does not exist at {0}</value>
</data>
<data name="Show_csharp_logs" xml:space="preserve">
<value>Show C# logs</value>
</data>
Expand Down Expand Up @@ -192,4 +198,7 @@
<data name="There_were_problems_loading_your_projects_See_log_for_details" xml:space="preserve">
<value>There were problems loading your projects. See log for details.</value>
</data>
<data name="Using_runsettings_file_at_0" xml:space="preserve">
<value>Using .runsettings file at {0}</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.LanguageServer.Handler;
using Microsoft.CodeAnalysis.LanguageServer.Handler.Testing;
using Microsoft.Extensions.Logging;
using Microsoft.TestPlatform.VsTestConsole.TranslationLayer;
using Roslyn.Utilities;
using LSP = Microsoft.VisualStudio.LanguageServer.Protocol;
Expand All @@ -17,11 +18,13 @@ namespace Microsoft.CodeAnalysis.LanguageServer.Testing;
[Method(RunTestsMethodName)]
[method: ImportingConstructor]
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
internal class RunTestsHandler(DotnetCliHelper dotnetCliHelper, TestDiscoverer testDiscoverer, TestRunner testRunner, ServerConfiguration serverConfiguration)
internal class RunTestsHandler(DotnetCliHelper dotnetCliHelper, TestDiscoverer testDiscoverer, TestRunner testRunner, ServerConfiguration serverConfiguration, ILoggerFactory loggerFactory)
: ILspServiceDocumentRequestHandler<RunTestsParams, RunTestsPartialResult[]>
{
private const string RunTestsMethodName = "textDocument/runTests";

private readonly ILogger _logger = loggerFactory.CreateLogger<RunTestsHandler>();

public bool MutatesSolutionState => false;

public bool RequiresLSPSolution => true;
Expand Down Expand Up @@ -59,11 +62,13 @@ public async Task<RunTestsPartialResult[]> HandleRequestAsync(RunTestsParams req
}
});

var testCases = await testDiscoverer.DiscoverTestsAsync(request.Range, context.Document, projectOutputPath, progress, vsTestConsoleWrapper, cancellationToken);
var runSettingsPath = request.RunSettingsPath;
var runSettings = await GetRunSettingsAsync(runSettingsPath, progress, cancellationToken);
var testCases = await testDiscoverer.DiscoverTestsAsync(request.Range, context.Document, projectOutputPath, runSettings, progress, vsTestConsoleWrapper, cancellationToken);
if (!testCases.IsEmpty)
{
var clientLanguageServerManager = context.GetRequiredLspService<IClientLanguageServerManager>();
await testRunner.RunTestsAsync(testCases, progress, vsTestConsoleWrapper, request.AttachDebugger, clientLanguageServerManager, cancellationToken);
await testRunner.RunTestsAsync(testCases, progress, vsTestConsoleWrapper, request.AttachDebugger, runSettings, clientLanguageServerManager, cancellationToken);
}

return progress.GetValues() ?? Array.Empty<RunTestsPartialResult>();
Expand Down Expand Up @@ -160,4 +165,33 @@ private static TraceLevel GetTraceLevel(ServerConfiguration serverConfiguration)
_ => throw new InvalidOperationException($"Unexpected log level {serverConfiguration.MinimumLogLevel}"),
};
}

private async Task<string?> GetRunSettingsAsync(string? runSettingsPath, BufferedProgress<RunTestsPartialResult> progress, CancellationToken cancellationToken)
{
if (string.IsNullOrEmpty(runSettingsPath))
{
return null;
}

try
{
var contents = await File.ReadAllTextAsync(runSettingsPath, cancellationToken);
Copy link
Member

Choose a reason for hiding this comment

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

Should this line be wrapped in IOUtilities.PerformIOAsync?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm not sure. I think I would only catch and log the exception regardless of what kind it is? Not sure if I would do anything different if it wasn't a normal IO exception?

var message = string.Format(LanguageServerResources.Using_runsettings_file_at_0, runSettingsPath);
progress.Report(new(LanguageServerResources.Discovering_tests, message, Progress: null));
_logger.LogTrace($".runsettings:{Environment.NewLine}{contents}");
return contents;
}
catch (FileNotFoundException)
{
var message = string.Format(LanguageServerResources.Runsettings_file_does_not_exist_at_0, runSettingsPath);
progress.Report(new(LanguageServerResources.Discovering_tests, message, Progress: null));
}
catch (Exception ex)
{
var message = string.Format(LanguageServerResources.Failed_to_read_runsettings_file_at_0_1, runSettingsPath, ex);
progress.Report(new(LanguageServerResources.Discovering_tests, message, Progress: null));
}

return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public async Task<ImmutableArray<TestCase>> DiscoverTestsAsync(
LSP.Range range,
Document document,
string projectOutputPath,
string? runSettings,
BufferedProgress<RunTestsPartialResult> progress,
VsTestConsoleWrapper vsTestConsoleWrapper,
CancellationToken cancellationToken)
Expand All @@ -56,8 +57,7 @@ public async Task<ImmutableArray<TestCase>> DiscoverTestsAsync(
var stopwatch = SharedStopwatch.StartNew();

// The async APIs for vs test are broken (current impl ends up just hanging), so we must use the sync API instead.
// TODO - run settings. https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1799066/
var discoveryTask = Task.Run(() => vsTestConsoleWrapper.DiscoverTests(SpecializedCollections.SingletonEnumerable(projectOutputPath), discoverySettings: null, discoveryHandler), cancellationToken);
var discoveryTask = Task.Run(() => vsTestConsoleWrapper.DiscoverTests(SpecializedCollections.SingletonEnumerable(projectOutputPath), discoverySettings: runSettings, discoveryHandler), cancellationToken);
cancellationToken.Register(() => vsTestConsoleWrapper.CancelDiscovery());
await discoveryTask;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ internal partial class TestRunner(ILoggerFactory loggerFactory)
/// A default value for run settings. While the vstest console included with newer SDKs does
/// support passing in a null run settings value, the vstest console in older SDKs (.net 6 for example)
/// will throw if we pass a null value. So for our default we hardcode an empty XML configuration.
/// TODO - Support configuring run settings - https://github.com/dotnet/vscode-csharp/issues/5719
/// </summary>
private const string DefaultRunSettings = "<RunSettings/>";
private readonly ILogger _logger = loggerFactory.CreateLogger<TestRunner>();
Expand All @@ -33,6 +32,7 @@ public async Task RunTestsAsync(
BufferedProgress<RunTestsPartialResult> progress,
VsTestConsoleWrapper vsTestConsoleWrapper,
bool attachDebugger,
string? runSettings,
IClientLanguageServerManager clientLanguageServerManager,
CancellationToken cancellationToken)
{
Expand All @@ -44,7 +44,7 @@ public async Task RunTestsAsync(

var handler = new TestRunHandler(progress, initialProgress, _logger);

var runTask = Task.Run(() => RunTests(testCases, progress, vsTestConsoleWrapper, handler, attachDebugger, clientLanguageServerManager), cancellationToken);
var runTask = Task.Run(() => RunTests(testCases, progress, vsTestConsoleWrapper, handler, attachDebugger, runSettings, clientLanguageServerManager), cancellationToken);
cancellationToken.Register(() => vsTestConsoleWrapper.CancelTestRun());
await runTask;
}
Expand All @@ -55,17 +55,19 @@ private static void RunTests(
VsTestConsoleWrapper vsTestConsoleWrapper,
TestRunHandler handler,
bool attachDebugger,
string? runSettings,
IClientLanguageServerManager clientLanguageServerManager)
{
runSettings ??= DefaultRunSettings;
if (attachDebugger)
{
// When we want to debug tests we need to use a custom test launcher so that we get called back with the process to attach to.
vsTestConsoleWrapper.RunTestsWithCustomTestHost(testCases, runSettings: DefaultRunSettings, handler, new DebugTestHostLauncher(progress, clientLanguageServerManager));
vsTestConsoleWrapper.RunTestsWithCustomTestHost(testCases, runSettings: runSettings, handler, new DebugTestHostLauncher(progress, clientLanguageServerManager));
}
else
{
// The async APIs for vs test are broken (current impl ends up just hanging), so we must use the sync API instead.
vsTestConsoleWrapper.RunTests(testCases, runSettings: DefaultRunSettings, handler);
vsTestConsoleWrapper.RunTests(testCases, runSettings: runSettings, handler);
}
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading