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

Endpoints for running and debugging all tests in a class #1089

Merged
merged 16 commits into from
Jan 29, 2018
Merged
Changes from 7 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
2 changes: 2 additions & 0 deletions src/OmniSharp.Abstractions/OmniSharpEndpoints.cs
Original file line number Diff line number Diff line change
@@ -50,9 +50,11 @@ public static class V2

public const string GetTestStartInfo = "/v2/getteststartinfo";
public const string RunTest = "/v2/runtest";
public const string RunAllTestsInClass = "/v2/runtestsinclass";
public const string DebugTestGetStartInfo = "/v2/debugtest/getstartinfo";
public const string DebugTestLaunch = "/v2/debugtest/launch";
public const string DebugTestStop = "/v2/debugtest/stop";
public const string DebugTestsInClassGetStartInfo = "/v2/debugtestsinclass/getstartinfo";
}
}
}
6 changes: 5 additions & 1 deletion src/OmniSharp.DotNetTest/DebugSessionManager.cs
Original file line number Diff line number Diff line change
@@ -78,10 +78,14 @@ public void EndSession()
}

public Task<DebugTestGetStartInfoResponse> DebugGetStartInfoAsync(string methodName, string testFrameworkName, string targetFrameworkVersion, CancellationToken cancellationToken)
=> DebugGetStartInfoAsync(new string[] { methodName }, testFrameworkName, targetFrameworkVersion, cancellationToken);

Copy link

Choose a reason for hiding this comment

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

Extra blank line

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed that.


public Task<DebugTestGetStartInfoResponse> DebugGetStartInfoAsync(string[] methodNames, string testFrameworkName, string targetFrameworkVersion, CancellationToken cancellationToken)
{
VerifySession(isStarted: true);

return _testManager.DebugGetStartInfoAsync(methodName, testFrameworkName, targetFrameworkVersion, cancellationToken);
return _testManager.DebugGetStartInfoAsync(methodNames, testFrameworkName, targetFrameworkVersion, cancellationToken);
}

public async Task<DebugTestLaunchResponse> DebugLaunchAsync(int targetProcessId)
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
using OmniSharp.Mef;
using OmniSharp.Models;

namespace OmniSharp.DotNetTest.Models
{
[OmniSharpEndpoint(OmniSharpEndpoints.V2.DebugTestsInClassGetStartInfo, typeof(DebugTestClassGetStartInfoRequest), typeof(DebugTestGetStartInfoResponse))]
class DebugTestClassGetStartInfoRequest : Request
{
public string[] MethodsInClass { get; set; }
Copy link

Choose a reason for hiding this comment

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

If this is just a list of methods, what happens if someone passes methods that aren't all in the same class?

public string TestFrameworkName { get; set; }
/// <summary>
/// e.g. .NETCoreApp, Version=2.0
/// </summary>
public string TargetFrameworkVersion { get; set; }
}
}
16 changes: 16 additions & 0 deletions src/OmniSharp.DotNetTest/Models/RunTestInClassRequest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
using OmniSharp.Mef;
using OmniSharp.Models;

namespace OmniSharp.DotNetTest.Models
{
[OmniSharpEndpoint(OmniSharpEndpoints.V2.RunAllTestsInClass, typeof(RunTestsInClassRequest), typeof(RunTestResponse[]))]
public class RunTestsInClassRequest : Request
Copy link

Choose a reason for hiding this comment

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

I feel like this could share a base class with the DebugTests request (they're identical, no?)

{
public string TestFrameworkName { get; set; }
/// <summary>
/// e.g. .NETCoreApp, Version=2.0
/// </summary>
public string TargetFrameworkVersion { get; set; }
public string[] MethodNamesInClass { get; set; }
}
}
34 changes: 34 additions & 0 deletions src/OmniSharp.DotNetTest/Services/DebugTestClassService.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
using System.Collections.Generic;
using System.Composition;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.Extensions.Logging;
using OmniSharp.DotNetTest.Models;
using OmniSharp.Eventing;
using OmniSharp.Mef;
using OmniSharp.Services;

namespace OmniSharp.DotNetTest.Services
{
[OmniSharpHandler(OmniSharpEndpoints.V2.DebugTestsInClassGetStartInfo, LanguageNames.CSharp)]
class DebugTestClassService : BaseTestService,
IRequestHandler<DebugTestClassGetStartInfoRequest, DebugTestGetStartInfoResponse>
{
private DebugSessionManager _debugSessionManager;

[ImportingConstructor]
public DebugTestClassService(DebugSessionManager debugSessionManager, OmniSharpWorkspace workspace, DotNetCliService dotNetCli, IEventEmitter eventEmitter, ILoggerFactory loggerFactory)
: base(workspace, dotNetCli, eventEmitter, loggerFactory)
{
_debugSessionManager = debugSessionManager;
}
Copy link

Choose a reason for hiding this comment

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

Needs a blank line.

public async Task<DebugTestGetStartInfoResponse> Handle(DebugTestClassGetStartInfoRequest request)
{
var testManager = CreateTestManager(request.FileName);
_debugSessionManager.StartSession(testManager);

return await _debugSessionManager.DebugGetStartInfoAsync(request.MethodsInClass, request.TestFrameworkName, request.TargetFrameworkVersion, CancellationToken.None);
}
}
}
42 changes: 42 additions & 0 deletions src/OmniSharp.DotNetTest/Services/RunTestsInClassService.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
using System.Collections.Generic;
using System.Composition;
using Microsoft.CodeAnalysis;
using Microsoft.Extensions.Logging;
using OmniSharp.DotNetTest.Models;
using OmniSharp.Eventing;
using OmniSharp.Mef;
using OmniSharp.Services;

namespace OmniSharp.DotNetTest.Services
{
[OmniSharpHandler(OmniSharpEndpoints.V2.RunAllTestsInClass, LanguageNames.CSharp)]
internal class RunTestsInClassService : BaseTestService<RunTestsInClassRequest, RunTestResponse[]>
Copy link

Choose a reason for hiding this comment

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

How similar is this to the class that runs a single test in a class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is the same now, only the type of request differs. For one it is RunTestRequest and for the other it is RunTestInClassRequest and the corresponding fields - MethodName and MethodNames also differ.

{
[ImportingConstructor]
public RunTestsInClassService(OmniSharpWorkspace workspace, DotNetCliService dotNetCli, IEventEmitter eventEmitter, ILoggerFactory loggerFactory)
: base(workspace, dotNetCli, eventEmitter, loggerFactory)
{
}

protected override RunTestResponse[] HandleRequest(RunTestsInClassRequest request, TestManager testManager)
{
List<RunTestResponse> responses = new List<RunTestResponse>();
if (testManager.IsConnected)
{
foreach (var methodName in request.MethodNamesInClass)
responses.Add(testManager.RunTest(methodName, request.TestFrameworkName, request.TargetFrameworkVersion));
Copy link
Contributor

Choose a reason for hiding this comment

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

Should there be a method that can run tests in bulk (taking several method names) rather than looping through the tests and running them individually?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would expect running several tests at once to be faster.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at what you've done with DebugGetStartInfoAsync in VSTestManager, I think you should probably do the same for RunTest.

}
else
{
var response = new RunTestResponse
{
Failure = "Failed to connect to 'dotnet test' process",
Pass = false
};
responses.Add(response);
}

return responses.ToArray();
}
}
}
5 changes: 5 additions & 0 deletions src/OmniSharp.DotNetTest/TestManager.cs
Original file line number Diff line number Diff line change
@@ -76,6 +76,11 @@ public static TestManager Create(Project project, DotNetCliService dotNetCli, IE
public abstract GetTestStartInfoResponse GetTestStartInfo(string methodName, string testFrameworkName, string targetFrameworkVersion);

public abstract Task<DebugTestGetStartInfoResponse> DebugGetStartInfoAsync(string methodName, string testFrameworkName, string targetFrameworkVersion, CancellationToken cancellationToken);

public virtual Task<DebugTestGetStartInfoResponse> DebugGetStartInfoAsync(string[] methodNames, string testFrameworkName, string targetFrameworkVersion, CancellationToken cancellationToken)
Copy link

Choose a reason for hiding this comment

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

What other types derive from TestManager? Should they implement this?

Copy link
Contributor

Choose a reason for hiding this comment

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

There's just two -- a legacy one for the old 'dotnet test' on project.json projects and the current 'dotnet vstest' version. I would only expect us to make this work for the newer stuff.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have implemented this function only for the VSTestManager and not LegacyTestManager. Is that required ?

Copy link

Choose a reason for hiding this comment

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

What is the behavior if you open a project with legacy tests? Does the run tests button show up? If it does, and you click it, what happens?

Copy link
Contributor

Choose a reason for hiding this comment

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

A project with legacy tests == a project.json project. I don't think we should invest effort in that. So long as we don't break the ability to run/debug a single test in a project.json project, I don't think we need to make any changes to LegacyTestManager.

Copy link

Choose a reason for hiding this comment

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

Sounds good. I'm happy with our solution of not showing the indicator for legacy projects.

{
throw new NotImplementedException();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing blank line.

public abstract Task DebugLaunchAsync(CancellationToken cancellationToken);

protected virtual bool PrepareToConnect()
7 changes: 6 additions & 1 deletion src/OmniSharp.DotNetTest/VSTestManager.cs
Original file line number Diff line number Diff line change
@@ -127,10 +127,15 @@ public override GetTestStartInfoResponse GetTestStartInfo(string methodName, str
}

public override async Task<DebugTestGetStartInfoResponse> DebugGetStartInfoAsync(string methodName, string testFrameworkName, string targetFrameworkVersion, CancellationToken cancellationToken)
=> await DebugGetStartInfoAsync(new string[] { methodName}, testFrameworkName, targetFrameworkVersion, cancellationToken);

public override async Task<DebugTestGetStartInfoResponse> DebugGetStartInfoAsync(string[] methodNames, string testFrameworkName, string targetFrameworkVersion, CancellationToken cancellationToken)
{
VerifyTestFramework(testFrameworkName);

var testCases = await DiscoverTestsAsync(methodName, targetFrameworkVersion, cancellationToken);
var testCases = new List<TestCase>();
foreach(var methodName in methodNames)
Copy link

Choose a reason for hiding this comment

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

Formatting:

foreach (....) 
{
    //stuff
}

testCases.AddRange(await DiscoverTestsAsync(methodName, targetFrameworkVersion, cancellationToken));

SendMessage(MessageType.GetTestRunnerProcessStartInfoForRunSelected,
new