-
Notifications
You must be signed in to change notification settings - Fork 416
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
Changes from 12 commits
3980673
a6876e1
6167298
2e8dd45
2d4d3bb
0aa7772
00f3344
bf2fc45
ddf6978
7bfbb59
c5b404b
21f36bc
9ecc6a2
c67062c
34da9a3
e9ce2b5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
using OmniSharp.Models; | ||
|
||
namespace OmniSharp.DotNetTest.Models | ||
{ | ||
public class BaseTestClassRequest : Request | ||
{ | ||
public string[] MethodNames { get; set; } | ||
public string TestFrameworkName { get; set; } | ||
/// <summary> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Newline There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Above the doc comment |
||
/// e.g. .NETCoreApp, Version=2.0 | ||
/// </summary> | ||
public string TargetFrameworkVersion { get; set; } | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
using OmniSharp.Mef; | ||
using OmniSharp.Models; | ||
|
||
namespace OmniSharp.DotNetTest.Models | ||
{ | ||
[OmniSharpEndpoint(OmniSharpEndpoints.V2.DebugTestsInClassGetStartInfo, typeof(DebugTestClassGetStartInfoRequest), typeof(DebugTestGetStartInfoResponse))] | ||
public class DebugTestClassGetStartInfoRequest : BaseTestClassRequest | ||
{ | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
using OmniSharp.Mef; | ||
using OmniSharp.Models; | ||
|
||
namespace OmniSharp.DotNetTest.Models | ||
{ | ||
[OmniSharpEndpoint(OmniSharpEndpoints.V2.RunAllTestsInClass, typeof(RunTestsInClassRequest), typeof(RunTestResponse))] | ||
public class RunTestsInClassRequest : BaseTestClassRequest | ||
{ | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
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; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.MethodNames, request.TestFrameworkName, request.TargetFrameworkVersion, CancellationToken.None); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
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> | ||
{ | ||
[ImportingConstructor] | ||
public RunTestsInClassService(OmniSharpWorkspace workspace, DotNetCliService dotNetCli, IEventEmitter eventEmitter, ILoggerFactory loggerFactory) | ||
: base(workspace, dotNetCli, eventEmitter, loggerFactory) | ||
{ | ||
} | ||
|
||
protected override RunTestResponse HandleRequest(RunTestsInClassRequest request, TestManager testManager) | ||
{ | ||
if (testManager.IsConnected) | ||
{ | ||
return testManager.RunTest(request.MethodNames, request.TestFrameworkName, request.TargetFrameworkVersion); | ||
} | ||
|
||
var response = new RunTestResponse | ||
{ | ||
Failure = "Failed to connect to 'dotnet test' process", | ||
Pass = false | ||
}; | ||
|
||
return response; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,9 +73,21 @@ public static TestManager Create(Project project, DotNetCliService dotNetCli, IE | |
protected abstract void VersionCheck(); | ||
|
||
public abstract RunTestResponse RunTest(string methodName, string testFrameworkName, string targetFrameworkVersion); | ||
|
||
public virtual RunTestResponse RunTest(string[] methodNames, string testFrameworkName, string targetFrameworkVersion) | ||
{ | ||
throw new NotImplementedException(); | ||
} | ||
|
||
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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What other types derive from TestManager? Should they implement this? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -105,7 +105,7 @@ public override GetTestStartInfoResponse GetTestStartInfo(string methodName, str | |
{ | ||
VerifyTestFramework(testFrameworkName); | ||
|
||
var testCases = DiscoverTests(methodName, targetFrameworkVersion); | ||
var testCases = DiscoverTests(new string[] { methodName }, targetFrameworkVersion); | ||
|
||
SendMessage(MessageType.GetTestRunnerProcessStartInfoForRunSelected, | ||
new | ||
|
@@ -127,10 +127,13 @@ 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 = await DiscoverTestsAsync(methodNames, targetFrameworkVersion, cancellationToken); | ||
|
||
SendMessage(MessageType.GetTestRunnerProcessStartInfoForRunSelected, | ||
new | ||
|
@@ -184,10 +187,13 @@ public override async Task DebugLaunchAsync(CancellationToken cancellationToken) | |
} | ||
|
||
public override RunTestResponse RunTest(string methodName, string testFrameworkName, string targetFrameworkVersion) | ||
=> RunTest(new string[] { methodName }, testFrameworkName, targetFrameworkVersion); | ||
|
||
public override RunTestResponse RunTest(string[] methodNames, string testFrameworkName, string targetFrameworkVersion) | ||
{ | ||
VerifyTestFramework(testFrameworkName); | ||
|
||
var testCases = DiscoverTests(methodName, targetFrameworkVersion); | ||
var testCases = DiscoverTests(methodNames, targetFrameworkVersion); | ||
|
||
var testResults = new List<TestResult>(); | ||
|
||
|
@@ -244,7 +250,7 @@ public override RunTestResponse RunTest(string methodName, string testFrameworkN | |
}; | ||
} | ||
|
||
private async Task<TestCase[]> DiscoverTestsAsync(string methodName, string targetFrameworkVersion, CancellationToken cancellationToken) | ||
private async Task<TestCase[]> DiscoverTestsAsync(string[] methodNames, string targetFrameworkVersion, CancellationToken cancellationToken) | ||
{ | ||
SendMessage(MessageType.StartDiscovery, | ||
new | ||
|
@@ -286,7 +292,7 @@ private async Task<TestCase[]> DiscoverTestsAsync(string methodName, string targ | |
|
||
testName = testName.Trim(); | ||
|
||
if (testName.Equals(methodName, StringComparison.Ordinal)) | ||
if (Array.Exists(methodNames, methodName => testName.Equals(methodName, StringComparison.Ordinal))) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: If you want to remove the O(n) operation here, you could add the method names to a |
||
{ | ||
testCases.Add(testCase); | ||
} | ||
|
@@ -303,9 +309,9 @@ private async Task<TestCase[]> DiscoverTestsAsync(string methodName, string targ | |
return testCases.ToArray(); | ||
} | ||
|
||
private TestCase[] DiscoverTests(string methodName, string targetFrameworkVersion) | ||
private TestCase[] DiscoverTests(string[] methodNames, string targetFrameworkVersion) | ||
{ | ||
return DiscoverTestsAsync(methodName, targetFrameworkVersion, CancellationToken.None).Result; | ||
return DiscoverTestsAsync(methodNames, targetFrameworkVersion, CancellationToken.None).Result; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra blank line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed that.