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

"Run All Tests" and "Debug All Tests" #1961

Merged
merged 28 commits into from
Feb 16, 2018
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
05aaa2d
Run All Tests Running But Building repeatedly
akshita31 Dec 28, 2017
e93efc8
Disposable variable for run all test
akshita31 Dec 28, 2017
9386779
Added code for debug all tests
akshita31 Dec 28, 2017
70b126f
Code Cleaning
akshita31 Jan 4, 2018
ee93887
Run all Tests running - Better logs required
akshita31 Jan 6, 2018
f7b5eb8
Run Tests running, all output shown at the end of all the tests
akshita31 Jan 8, 2018
8645f42
Renamed variable to methodsInClass
akshita31 Jan 8, 2018
63aa55b
Merge remote-tracking branch 'upstream/master' into class_test
akshita31 Jan 8, 2018
45792a5
Changes for Debug All Tests
akshita31 Jan 8, 2018
c30ee75
Changes for debug tests request
akshita31 Jan 9, 2018
75c9d64
Debug All Tests running
akshita31 Jan 10, 2018
2f33327
Added common helpers for single test and all test functions
akshita31 Jan 13, 2018
08337b5
Merge remote-tracking branch 'upstream/master' into class_test
akshita31 Jan 13, 2018
ba21a1d
Improved logs for Run All Tests
akshita31 Jan 13, 2018
a6f6390
Merge branch 'master' into class_test
akshita31 Jan 15, 2018
5a796b7
Changes to get only 1 response for a set of methods
akshita31 Jan 20, 2018
2eab3f2
Extracted a common helper to get the test feature
akshita31 Jan 20, 2018
c1eed78
Resolved review comments
akshita31 Jan 23, 2018
d8ddfb6
Changes to not show this change for legacy projects
akshita31 Jan 23, 2018
1ec2448
Renamed incorrect variable
akshita31 Jan 23, 2018
128641d
Removing foreach for proper order of execution
akshita31 Jan 23, 2018
f3d873e
Remove unnecessary import
akshita31 Jan 23, 2018
7ca1540
Do not show the festure for legacy projects
akshita31 Jan 24, 2018
6d6608d
Merge branch 'master' into class_test
akshita31 Jan 24, 2018
5a62bfa
Merge branch 'master' into class_test
Feb 2, 2018
1d8f653
Merge branch 'master' into class_test
Feb 6, 2018
5b9b090
Merge branch 'master' into class_test
akshita31 Feb 7, 2018
50fca56
Merge branch 'master' into class_test
akshita31 Feb 15, 2018
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
47 changes: 43 additions & 4 deletions src/features/codeLensProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ export default class OmniSharpCodeLensProvider extends AbstractProvider implemen

private _options: Options;

constructor(server: OmniSharpServer, reporter: TelemetryReporter, testManager: TestManager)
{
constructor(server: OmniSharpServer, reporter: TelemetryReporter, testManager: TestManager) {
super(server, reporter);

this._resetCachedOptions();
Expand All @@ -53,8 +52,7 @@ export default class OmniSharpCodeLensProvider extends AbstractProvider implemen
};

provideCodeLenses(document: vscode.TextDocument, token: vscode.CancellationToken): vscode.CodeLens[] | Thenable<vscode.CodeLens[]> {
if (!this._options.showReferencesCodeLens && !this._options.showTestsCodeLens)
{
if (!this._options.showReferencesCodeLens && !this._options.showTestsCodeLens) {
return [];
}

Expand Down Expand Up @@ -119,6 +117,10 @@ export default class OmniSharpCodeLensProvider extends AbstractProvider implemen
return;
}

if (node.Kind == "ClassDeclaration" && node.ChildNodes.length > 0) {
this._updateCodeLensForTestClass(bucket, fileName, node);
}

let testFeature = node.Features.find(value => (value.Name == 'XunitTestMethod' || value.Name == 'NUnitTestMethod' || value.Name == 'MSTestMethod'));
if (testFeature) {
// this test method has a test feature
Expand All @@ -139,4 +141,41 @@ export default class OmniSharpCodeLensProvider extends AbstractProvider implemen
{ title: "debug test", command: 'dotnet.test.debug', arguments: [testFeature.Data, fileName, testFrameworkName] }));
}
}

private _updateCodeLensForTestClass(bucket: vscode.CodeLens[], fileName: string, node: protocol.Node) {
//if the class doesnot contain any method then return
Copy link

Choose a reason for hiding this comment

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

space after //

if (!node.ChildNodes.find(value => (value.Kind == "MethodDeclaration"))) {
Copy link
Member

Choose a reason for hiding this comment

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

===

return;
}

let testMethodsInClass = new Array<string>();
Copy link

Choose a reason for hiding this comment

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

Is this the most idiomatic way to declare an empty array in TS? (@TheRealPiotrP )

let testFrameworkName: string = null;
for (let child of node.ChildNodes) {
if (child.Kind == "MethodDeclaration") {
let testFeature = child.Features.find(value => (value.Name == 'XunitTestMethod' || value.Name == 'NUnitTestMethod' || value.Name == 'MSTestMethod'));
Copy link

Choose a reason for hiding this comment

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

This looks like a copy-paste from line 124--let's extract a helper

if (testFeature) {
// this test method has a test feature
if (testFrameworkName == null) {
testFrameworkName = 'xunit';
Copy link

Choose a reason for hiding this comment

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

Is it possible for someone to have tests for different frameworks in the same class? What happens if they do?

Copy link
Contributor Author

@akshita31 akshita31 Jan 20, 2018

Choose a reason for hiding this comment

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

I don't think so. The code currently assumes that every test method in the class has the same frameworkname as the first testmethod in the class and the test request also contains only one field TestFrameworkName (https://github.com/akshita31/omnisharp-vscode/blob/504e97edf968229e9f14f8603b7ebeaf5b764ade/src/omnisharp/protocol.ts#L588) for a group of methods. Does this need to change ? @rchande

Copy link

Choose a reason for hiding this comment

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

It's probably fine then. Thanks for checking.

if (testFeature.Name == 'NUnitTestMethod') {
testFrameworkName = 'nunit';
}
else if (testFeature.Name == 'MSTestMethod') {
testFrameworkName = 'mstest';
}
}
testMethodsInClass.push(testFeature.Data);
}
}
}

if (testMethodsInClass.length) {
bucket.push(new vscode.CodeLens(
toRange(node.Location),
{ title: "run all test", command: 'dotnet.classTests.run', arguments: [testMethodsInClass, fileName, testFrameworkName] }));
Copy link

Choose a reason for hiding this comment

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

tests

bucket.push(new vscode.CodeLens(
toRange(node.Location),
{ title: "debug all test", command: 'dotnet.classTests.debug', arguments: [testMethodsInClass, fileName, testFrameworkName] }));
Copy link

Choose a reason for hiding this comment

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

tests

}
}
}
200 changes: 154 additions & 46 deletions src/features/dotnetTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ import * as os from 'os';
import * as path from 'path';
import TelemetryReporter from 'vscode-extension-telemetry';
import AbstractProvider from './abstractProvider';
import { outputFileSync } from 'fs-extra';

const TelemetryReportingDelay = 2 * 60 * 1000; // two minutes

export default class TestManager extends AbstractProvider {

private _channel: vscode.OutputChannel;

private _runCounts: { [testFrameworkName: string]: number };
Expand All @@ -36,6 +38,14 @@ export default class TestManager extends AbstractProvider {
'dotnet.test.debug',
(testMethod, fileName, testFrameworkName) => this._debugDotnetTest(testMethod, fileName, testFrameworkName));

let d4 = vscode.commands.registerCommand(
'dotnet.classTests.run',
(methodsInClass, fileName, testFrameworkName) => this._runDotnetTestsInClass(methodsInClass, fileName, testFrameworkName, ));

let d5 = vscode.commands.registerCommand(
'dotnet.classTests.debug',
(methodsInClass, fileName, testFrameworkName) => this._debugDotnetTestsInClass(methodsInClass, fileName, testFrameworkName));

this._telemetryIntervalId = setInterval(() =>
this._reportTelemetry(), TelemetryReportingDelay);

Expand All @@ -48,7 +58,7 @@ export default class TestManager extends AbstractProvider {
}
});

this.addDisposables(d1, d2, d3);
this.addDisposables(d1, d2, d3, d4, d5);
}

private _getOutputChannel(): vscode.OutputChannel {
Expand Down Expand Up @@ -117,18 +127,22 @@ export default class TestManager extends AbstractProvider {
FileName: fileName,
MethodName: testMethod,
TestFrameworkName: testFrameworkName,
TargetFrameworkVersion: targetFrameworkVersion
TargetFrameworkVersion: targetFrameworkVersion,
Copy link
Member

Choose a reason for hiding this comment

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

unintended comma?

};

return serverUtils.runTest(this._server, request)
.then(response => response.Results);
}

private _reportResults(results: protocol.V2.DotNetTestResult[]): Promise<void> {
private _reportResults(results: protocol.V2.DotNetTestResult[], printEachResult: boolean): Promise<void> {
const totalTests = results.length;
const output = this._getOutputChannel();

let totalPassed = 0, totalFailed = 0, totalSkipped = 0;
for (let result of results) {
if (printEachResult) {
output.appendLine(`${result.MethodName}: ${result.Outcome}`);
}
switch (result.Outcome) {
case protocol.V2.TestOutcomes.Failed:
totalFailed += 1;
Expand All @@ -142,15 +156,35 @@ export default class TestManager extends AbstractProvider {
}
}

const output = this._getOutputChannel();
output.appendLine('');
output.appendLine(`Total tests: ${totalTests}. Passed: ${totalPassed}. Failed: ${totalFailed}. Skipped: ${totalSkipped}`);
output.appendLine('');

return Promise.resolve();
}

private _runDotnetTest(testMethod: string, fileName: string, testFrameworkName: string) {
private async _recordRunAndGetFrameworkVersion(fileName: string, testFrameworkName: string) {

await this._saveDirtyFiles();
this._recordRunRequest(testFrameworkName);
let projectInfo = await serverUtils.requestProjectInformation(this._server, { FileName: fileName });

let targetFrameworkVersion: string;

if (projectInfo.DotNetProject) {
targetFrameworkVersion = undefined;
Copy link

Choose a reason for hiding this comment

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

wat

Copy link
Contributor Author

@akshita31 akshita31 Jan 20, 2018

Choose a reason for hiding this comment

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

This is the same as was previously being used: https://github.com/OmniSharp/omnisharp-vscode/blob/master/src/features/dotnetTest.ts#L172
Just extracted it to a helper

Copy link
Member

Choose a reason for hiding this comment

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

@rchande: This is because we never pass the target framework in the legacy case.

}
else if (projectInfo.MsBuildProject) {
targetFrameworkVersion = projectInfo.MsBuildProject.TargetFramework;
}
else {
throw new Error('Expected project.json or .csproj project.');
}

return targetFrameworkVersion;
}

private async _runDotnetTest(testMethod: string, fileName: string, testFrameworkName: string) {
const output = this._getOutputChannel();

output.show();
Expand All @@ -161,33 +195,54 @@ export default class TestManager extends AbstractProvider {
output.appendLine(e.Message);
});

this._saveDirtyFiles()
.then(_ => this._recordRunRequest(testFrameworkName))
.then(_ => serverUtils.requestProjectInformation(this._server, { FileName: fileName }))
.then(projectInfo =>
{
let targetFrameworkVersion: string;

if (projectInfo.DotNetProject) {
targetFrameworkVersion = undefined;
}
else if (projectInfo.MsBuildProject) {
targetFrameworkVersion = projectInfo.MsBuildProject.TargetFramework;
}
else {
throw new Error('Expected project.json or .csproj project.');
}
let targetFrameworkVersion = await this._recordRunAndGetFrameworkVersion(fileName, testFrameworkName);

return this._runTest(fileName, testMethod, testFrameworkName, targetFrameworkVersion);
})
.then(results => this._reportResults(results))
return this._runTest(fileName, testMethod, testFrameworkName, targetFrameworkVersion)
.then(results => this._reportResults(results, false))
.then(() => listener.dispose())
.catch(reason => {
listener.dispose();
vscode.window.showErrorMessage(`Failed to run test because ${reason}.`);
});
}

private async _runDotnetTestsInClass(methodsInClass: string[], fileName: string, testFrameworkName: string) {
let allResults: protocol.V2.DotNetTestResult[] = new Array();
const output = this._getOutputChannel();

output.show();
const listener = this._server.onTestMessage(e => {
output.appendLine(e.Message);
});

let targetFrameworkVersion = await this._recordRunAndGetFrameworkVersion(fileName, testFrameworkName);

return this._runTestsInClass(fileName, testFrameworkName, targetFrameworkVersion, methodsInClass)
.then(responses => {
Copy link

Choose a reason for hiding this comment

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

This method is already async, can we await instead of chaining .then?

responses.forEach(response => {
Array.prototype.push.apply(allResults, response.Results);
Copy link

Choose a reason for hiding this comment

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

Why not just allREsults.push?

});
}).then(async () => {
listener.dispose();
await this._reportResults(allResults, true);
})
.catch(reason => {
listener.dispose();
vscode.window.showErrorMessage(`Could not run tests`);
Copy link

Choose a reason for hiding this comment

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

Can we do something with reason?

});
}

private _runTestsInClass(fileName: string, testFrameworkName: string, targetFrameworkVersion: string, methodsToRun: string[]): Promise<protocol.V2.RunTestResponse[]> {
const request: protocol.V2.RunTestsInClassRequest = {
FileName: fileName,
TestFrameworkName: testFrameworkName,
TargetFrameworkVersion: targetFrameworkVersion,
MethodNamesInClass: methodsToRun
};

return serverUtils.runTestsInClass(this._server, request);
}

private _createLaunchConfiguration(program: string, args: string, cwd: string, debuggerEventsPipeName: string) {
let debugOptions = vscode.workspace.getConfiguration('csharp').get('unitTestDebuggingOptions');

Expand Down Expand Up @@ -271,39 +326,64 @@ export default class TestManager extends AbstractProvider {
}
}

private _debugDotnetTest(testMethod: string, fileName: string, testFrameworkName: string) {
// We support to styles of 'dotnet test' for debugging: The legacy 'project.json' testing, and the newer csproj support
// using VS Test. These require a different level of communication.
private async _recordDebugAndGetDebugValues(fileName: string, testFrameworkName: string, output: vscode.OutputChannel) {
await this._saveDirtyFiles();
this._recordDebugRequest(testFrameworkName);
let projectInfo = await serverUtils.requestProjectInformation(this._server, { FileName: fileName });

let debugType: string;
let debugEventListener: DebugEventListener = null;
let targetFrameworkVersion: string;

if (projectInfo.DotNetProject) {
debugType = 'legacy';
targetFrameworkVersion = '';
}
else if (projectInfo.MsBuildProject) {
debugType = 'vstest';
targetFrameworkVersion = projectInfo.MsBuildProject.TargetFramework;
debugEventListener = new DebugEventListener(fileName, this._server, output);
debugEventListener.start();
}
else {
throw new Error('Expected project.json or .csproj project.');
}

return { debugType, debugEventListener, targetFrameworkVersion };
}

private async _debugDotnetTest(testMethod: string, fileName: string, testFrameworkName: string) {
// We support to styles of 'dotnet test' for debugging: The legacy 'project.json' testing, and the newer csproj support
// using VS Test. These require a different level of communication.

const output = this._getOutputChannel();

output.show();
output.appendLine(`Debugging method '${testMethod}'...`);
output.appendLine('');

return this._saveDirtyFiles()
.then(_ => this._recordDebugRequest(testFrameworkName))
.then(_ => serverUtils.requestProjectInformation(this._server, { FileName: fileName }))
.then(projectInfo => {
if (projectInfo.DotNetProject) {
debugType = 'legacy';
targetFrameworkVersion = '';
return Promise.resolve();
}
else if (projectInfo.MsBuildProject) {
debugType = 'vstest';
targetFrameworkVersion = projectInfo.MsBuildProject.TargetFramework;
debugEventListener = new DebugEventListener(fileName, this._server, output);
return debugEventListener.start();
}
else {
throw new Error('Expected project.json or .csproj project.');
}
let { debugType, debugEventListener, targetFrameworkVersion } = await this._recordDebugAndGetDebugValues(fileName, testFrameworkName, output);

return this._getLaunchConfiguration(debugType, fileName, testMethod, testFrameworkName, targetFrameworkVersion, debugEventListener)
.then(config => {
const workspaceFolder = vscode.workspace.getWorkspaceFolder(vscode.Uri.file(fileName));
return vscode.debug.startDebugging(workspaceFolder, config);
})
.then(() => this._getLaunchConfiguration(debugType, fileName, testMethod, testFrameworkName, targetFrameworkVersion, debugEventListener))
.catch(reason => {
vscode.window.showErrorMessage(`Failed to start debugger: ${reason}`);
if (debugEventListener != null) {
debugEventListener.close();
}
});
}

private async _debugDotnetTestsInClass(methodsToRun: string[], fileName: string, testFrameworkName: string) {

const output = this._getOutputChannel();

let { debugType, debugEventListener, targetFrameworkVersion } = await this._recordDebugAndGetDebugValues(fileName, testFrameworkName, output);

return await this._getLaunchConfigurationForClass(debugType, fileName, methodsToRun, testFrameworkName, targetFrameworkVersion, debugEventListener)
.then(config => {
const workspaceFolder = vscode.workspace.getWorkspaceFolder(vscode.Uri.file(fileName));
return vscode.debug.startDebugging(workspaceFolder, config);
Expand All @@ -315,6 +395,34 @@ export default class TestManager extends AbstractProvider {
}
});
}

private _getLaunchConfigurationForClass(debugType: string, fileName: string, methodsToRun: string[], testFrameworkName: string, targetFrameworkVersion: string, debugEventListener: DebugEventListener): Promise<any> {
if (debugType == 'vstest') {
return this._getLaunchConfigurationForVSTestClass(fileName, methodsToRun, testFrameworkName, targetFrameworkVersion, debugEventListener);
}
throw new Error(`Unexpected debug type: ${debugType}`);
}

private _getLaunchConfigurationForVSTestClass(fileName: string, methodsToRun: string[], testFrameworkName: string, targetFrameworkVersion: string, debugEventListener: DebugEventListener): Promise<any> {
const output = this._getOutputChannel();

const listener = this._server.onTestMessage(e => {
output.appendLine(e.Message);
});

const request: protocol.V2.DebugTestClassGetStartInfoRequest = {
FileName: fileName,
MethodsInClass: methodsToRun,
TestFrameworkName: testFrameworkName,
TargetFrameworkVersion: targetFrameworkVersion
};

return serverUtils.debugTestClassGetStartInfo(this._server, request)
.then(response => {
listener.dispose();
return this._createLaunchConfiguration(response.FileName, response.Arguments, response.WorkingDirectory, debugEventListener.pipePath());
});
}
}

class DebugEventListener {
Expand Down
Loading