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

Update right click menu label based on test status #811

Merged
merged 8 commits into from
Dec 13, 2018
21 changes: 18 additions & 3 deletions packages/salesforcedx-vscode-apex/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -92,16 +92,23 @@
"view/item/context": [
{
"command": "sfdx.force.test.view.showError",
"when": "view == sfdx.force.test.view"
"when":
"view == sfdx.force.test.view && viewItem =~ /(apexTest|apexTestGroup)_Fail/"
},
{
"command": "sfdx.force.test.view.goToDefinition",
"when":
"view == sfdx.force.test.view && viewItem =~ /(apexTest|apexTestGroup)(_Pass|_Skip|\\b)/"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

VSCode doesn't exactly have a way of dynamically updating the labels for the same command in a context menu. I created another command that runs the same method, but has the "Go to Definition" label. Then conditionally render one command or the other based on the where clause.

Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Yeah, there isn't a way to dynamically update the labels.
  2. Does this mean that you won't be able to see the "Go to Definition" menu if you are on a failing node?
  3. What about calling it goToDefinition. I think you're really going to the definition more than showing it in-line.

},
{
"command": "sfdx.force.test.view.runClassTests",
"when": "view == sfdx.force.test.view && viewItem == apexTestGroup",
"when": "view == sfdx.force.test.view && viewItem =~ /apexTestGroup/",
"group": "inline"
},
{
"command": "sfdx.force.test.view.runSingleTest",
"when": "view == sfdx.force.test.view && viewItem == apexTest",
"when":
"view == sfdx.force.test.view && viewItem =~ /(apexTest)(_.*|\\b)/",
"group": "inline"
}
]
Expand Down Expand Up @@ -131,6 +138,14 @@
"dark": "resources/dark/document/notRun.svg"
}
},
{
"command": "sfdx.force.test.view.goToDefinition",
"title": "%go_to_definition_title%",
"icon": {
"light": "resources/light/document/notRun.svg",
"dark": "resources/dark/document/notRun.svg"
}
},
{
"command": "sfdx.force.test.view.runClassTests",
"title": "%run_tests_title%",
Expand Down
1 change: 1 addition & 0 deletions packages/salesforcedx-vscode-apex/package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"refresh_test_title": "Refresh Tests",
"run_single_test_title": "Run Single Test",
"show_error_title": "Display Error",
"go_to_definition_title": "Go to Definition",
"java_home_description":
"Specifies the folder path to the Java 8 runtime used to launch the Apex Language Server (for example, /Library/Java/JavaVirtualMachines/jdk1.8.0_131.jdk/Contents/Home).",
"apex_rename_description":
Expand Down
7 changes: 7 additions & 0 deletions packages/salesforcedx-vscode-apex/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,13 @@ async function registerTestView(
testRunner.showErrorMessage(test)
)
);
// Show Definition command
testViewItems.push(
vscode.commands.registerCommand(
'sfdx.force.test.view.goToDefinition',
test => testRunner.showErrorMessage(test)
)
);
// Run Class Tests command
testViewItems.push(
vscode.commands.registerCommand(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ export class ApexTestOutlineProvider
) as ApexTestNode;
if (apexTest) {
apexTest.outcome = testResult.Outcome;
apexTest.updateIcon();
apexTest.updateOutcome();
if (testResult.Outcome === 'Fail') {
apexTest.errorMessage = testResult.Message;
apexTest.stackTrace = testResult.StackTrace;
Expand Down Expand Up @@ -226,7 +226,7 @@ export abstract class TestNode extends vscode.TreeItem {
return this.description;
}

public updateIcon(outcome: string) {
public updateOutcome(outcome: string) {
if (outcome === 'Pass') {
// Passed Test
this.iconPath = {
Expand All @@ -246,6 +246,9 @@ export abstract class TestNode extends vscode.TreeItem {
dark: DARK_ORANGE_BUTTON
};
}

const nodeType = this.contextValue.split('_')[0];
this.contextValue = `${nodeType}_${outcome}`;
}

public abstract contextValue: string;
Expand Down Expand Up @@ -278,19 +281,19 @@ export class ApexTestGroupNode extends TestNode {

if (this.passing + this.failing + this.skipping === this.children.length) {
if (this.failing !== 0) {
this.updateIcon('Fail');
this.updateOutcome('Fail');
} else {
this.updateIcon('Pass');
this.updateOutcome('Pass');
}
}
}

public updateIcon(outcome: string) {
super.updateIcon(outcome);
public updateOutcome(outcome: string) {
super.updateOutcome(outcome);
if (outcome === 'Pass') {
this.children.forEach(child => {
// Update all the children as well
child.updateIcon(outcome);
child.updateOutcome(outcome);
});
}
}
Expand All @@ -305,8 +308,8 @@ export class ApexTestNode extends TestNode {
super(label, vscode.TreeItemCollapsibleState.None, location);
}

public updateIcon() {
super.updateIcon(this.outcome);
public updateOutcome() {
super.updateOutcome(this.outcome);
if (this.outcome === 'Pass') {
this.errorMessage = '';
}
Expand Down
33 changes: 25 additions & 8 deletions packages/salesforcedx-vscode-apex/src/views/testRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import * as vscode from 'vscode';
import { nls } from '../messages';
import { ReadableApexTestRunExecutor } from './readableApexTestRunExecutor';
import {
ApexTestGroupNode,
ApexTestNode,
ApexTestOutlineProvider,
TestNode
Expand All @@ -27,18 +28,33 @@ const SfdxWorkspaceChecker = sfdxCoreExports.SfdxWorkspaceChecker;
const channelService = sfdxCoreExports.channelService;
export class ApexTestRunner {
private testOutline: ApexTestOutlineProvider;
private eventsEmitter = new events.EventEmitter();
constructor(testOutline: ApexTestOutlineProvider) {
private eventsEmitter: events.EventEmitter;
constructor(
testOutline: ApexTestOutlineProvider,
eventsEmitter?: events.EventEmitter
) {
this.testOutline = testOutline;
this.eventsEmitter = eventsEmitter || new events.EventEmitter();
this.eventsEmitter.on('sfdx:update_selection', this.updateSelection);
}

public async showErrorMessage(test: TestNode) {
public showErrorMessage(test: TestNode) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Very cool to get rid of the async here.

let testNode = test;
let position: vscode.Range | number = test.location!.range;
if (test instanceof ApexTestNode) {
const errorMessage = test.errorMessage;
if (testNode instanceof ApexTestGroupNode) {
if (test.contextValue === 'apexTestGroup_Fail') {
const failedTest = test.children.find(
testCase => testCase.contextValue === 'apexTest_Fail'
);
if (failedTest) {
testNode = failedTest;
}
}
}
if (testNode instanceof ApexTestNode) {
const errorMessage = testNode.errorMessage;
if (errorMessage && errorMessage !== '') {
const stackTrace = test.stackTrace;
const stackTrace = testNode.stackTrace;
position =
parseInt(
stackTrace.substring(
Expand All @@ -54,8 +70,9 @@ export class ApexTestRunner {
channelService.showChannelOutput();
}
}
if (test.location) {
vscode.window.showTextDocument(test.location.uri).then(() => {

if (testNode.location) {
vscode.window.showTextDocument(testNode.location.uri).then(() => {
this.eventsEmitter.emit('sfdx:update_selection', position);
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ const testResultsMultipleFiles = [
MethodName: 'test1',
Outcome: 'Fail',
RunTime: 1,
Message: '',
StackTrace: '',
Message: 'System.AssertException: Assertion Failed',
StackTrace: 'Class.fakeClass.test1: line 40, column 1',
FullName: 'file0.test1'
},
{
Expand Down Expand Up @@ -114,8 +114,8 @@ const testResultsMultipleFiles = [
MethodName: 'test6',
Outcome: 'Fail',
RunTime: 1,
Message: '',
StackTrace: '',
Message: 'System.AssertException: Assertion Failed',
StackTrace: 'Class.fakeClass.test6: line 22, column 1',
FullName: 'file3.test6'
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

// tslint:disable:no-unused-expression
import { expect } from 'chai';
import * as events from 'events';
import * as fs from 'fs';
import { SinonStub, stub } from 'sinon';
import * as vscode from 'vscode';
Expand All @@ -16,8 +17,10 @@ import { ApexTestMethod } from '../../src/views/lspConverter';
import {
ApexTestGroupNode,
ApexTestNode,
ApexTestOutlineProvider
ApexTestOutlineProvider,
TestNode
} from '../../src/views/testOutlineProvider';
import { ApexTestRunner } from '../../src/views/testRunner';
import {
jsonSummaryMultipleFiles,
jsonSummaryOneFilePass
Expand Down Expand Up @@ -189,4 +192,78 @@ describe('TestView', () => {
}
});
});

describe('Navigate to test definition or error', () => {
let readFolderStub: SinonStub;
let readFileStub: SinonStub;
let parseJSONStub: SinonStub;
let showTextDocumentStub: SinonStub;
let eventEmitterStub: SinonStub;

let testRunner: ApexTestRunner;
const eventEmitter = new events.EventEmitter();

beforeEach(() => {
readFolderStub = stub(fs, 'readdirSync');
readFolderStub.callsFake(folderName => ['test-result.json']);
readFileStub = stub(fs, 'readFileSync');
readFileStub.callsFake(fileName => 'nonsense');
parseJSONStub = stub(JSON, 'parse');
parseJSONStub.callsFake(() => jsonSummaryMultipleFiles);
eventEmitterStub = stub(eventEmitter, 'emit');
showTextDocumentStub = stub(vscode.window, 'showTextDocument');
showTextDocumentStub.returns(Promise.resolve());

testOutline = new ApexTestOutlineProvider(apexTestInfo);
testOutline.readJSONFile('multipleFilesMixed');
testRunner = new ApexTestRunner(testOutline, eventEmitter);
});

afterEach(() => {
readFolderStub.restore();
readFileStub.restore();
parseJSONStub.restore();
eventEmitterStub.restore();
showTextDocumentStub.restore();
});

it('Should go to definition if a test does not have an error message', async () => {
const testNode = new ApexTestNode('sampleTest', apexTestInfo[0].location);
const testRange = testNode.location!.range;

await testRunner.showErrorMessage(testNode);

// make sure we emit the update_selection event with the correct position
expect(eventEmitterStub.getCall(0).args).to.be.deep.equal([
'sfdx:update_selection',
testRange
]);
});

it('Should go to error if a test has one', async () => {
const lineFailure = 22;
const testNode = new ApexTestNode('failedTest', apexTestInfo[0].location);
testNode.errorMessage = 'System.AssertException: Assertion Failed';
testNode.stackTrace = `Class.fakeClass.test0: line ${lineFailure}, column 1`;

await testRunner.showErrorMessage(testNode);

expect(eventEmitterStub.getCall(0).args).to.be.deep.equal([
'sfdx:update_selection',
lineFailure - 1
]);
});

it('Should go to error of first failing test in a failed test class', async () => {
const testClass = testOutline.getHead().children[0] as ApexTestGroupNode;
const lineFailure = 40; // first failure in testJSONOutputs.testResultsMultipleFiles

await testRunner.showErrorMessage(testClass);

expect(eventEmitterStub.getCall(0).args).to.be.deep.equal([
'sfdx:update_selection',
lineFailure - 1
]);
});
});
});