Set focus to editor and navigate to tests upon selection in test explorer (#4354)

* Set focus to editor and focus upon selection in tree
* More telemetry
* Fixed tests

Author: DonJayamanne

src/client/telemetry/index.ts

@@ -115,10 +115,10 @@ export function captureTelemetry<P extends IEventNamePropertyMapping, E extends
     failureEventName?: E
 ) {
     // tslint:disable-next-line:no-function-expression no-any
-    return function(target: Object, propertyKey: string, descriptor: TypedPropertyDescriptor<any>) {
+    return function (target: Object, propertyKey: string, descriptor: TypedPropertyDescriptor<any>) {
         const originalMethod = descriptor.value;
         // tslint:disable-next-line:no-function-expression no-any
-        descriptor.value = function(...args: any[]) {
+        descriptor.value = function (...args: any[]) {
             if (!captureDuration) {
                 sendTelemetryEvent(eventName, undefined, properties);
                 // tslint:disable-next-line:no-invalid-this
@@ -327,6 +327,6 @@ interface IEventNamePropertyMapping {
     [Telemetry.SubmitCellThroughInput]: never | undefined;
     [Telemetry.Undo]: never | undefined;
     [EventName.UNITTEST_NAVIGATE_TEST_FILE]: never | undefined;
-    [EventName.UNITTEST_NAVIGATE_TEST_FUNCTION]: never | undefined;
-    [EventName.UNITTEST_NAVIGATE_TEST_SUITE]: never | undefined;
+    [EventName.UNITTEST_NAVIGATE_TEST_FUNCTION]: { focus: boolean };
+    [EventName.UNITTEST_NAVIGATE_TEST_SUITE]: { focus: boolean };
 }

src/client/unittests/navigation/fileNavigator.ts

@@ -13,10 +13,10 @@ import { ITestCodeNavigator, ITestNavigatorHelper } from './types';
 
 @injectable()
 export class TestFileCodeNavigator implements ITestCodeNavigator {
-    constructor(@inject(ITestNavigatorHelper) private readonly helper: ITestNavigatorHelper) {}
+    constructor(@inject(ITestNavigatorHelper) private readonly helper: ITestNavigatorHelper) { }
     @swallowExceptions('Navigate to test file')
     @captureTelemetry(EventName.UNITTEST_NAVIGATE_TEST_FILE, undefined, true)
-    public async navigateTo(_: Uri, item: TestFile): Promise<void> {
+    public async navigateTo(_: Uri, item: TestFile, __: boolean): Promise<void> {
         await this.helper.openFile(Uri.file(item.fullPath));
     }
 }

src/client/unittests/navigation/functionNavigator.ts

@@ -5,9 +5,10 @@
 
 import { inject, injectable } from 'inversify';
 import { CancellationTokenSource, Range, SymbolInformation, SymbolKind, TextEditorRevealType, Uri } from 'vscode';
+import { IDocumentManager } from '../../common/application/types';
 import { traceError } from '../../common/logger';
 import { swallowExceptions } from '../../common/utils/decorators';
-import { captureTelemetry } from '../../telemetry';
+import { captureTelemetry, sendTelemetryEvent } from '../../telemetry';
 import { EventName } from '../../telemetry/constants';
 import { ITestCollectionStorageService, TestFunction } from '../common/types';
 import { ITestCodeNavigator, ITestNavigatorHelper } from './types';
@@ -17,11 +18,13 @@ export class TestFunctionCodeNavigator implements ITestCodeNavigator {
     private cancellationToken?: CancellationTokenSource;
     constructor(
         @inject(ITestNavigatorHelper) private readonly helper: ITestNavigatorHelper,
+        @inject(IDocumentManager) private readonly docManager: IDocumentManager,
         @inject(ITestCollectionStorageService) private readonly storage: ITestCollectionStorageService
-    ) {}
+    ) { }
     @swallowExceptions('Navigate to test function')
     @captureTelemetry(EventName.UNITTEST_NAVIGATE_TEST_FUNCTION, undefined, true)
-    public async navigateTo(resource: Uri, fn: TestFunction): Promise<void> {
+    public async navigateTo(resource: Uri, fn: TestFunction, focus: boolean = true): Promise<void> {
+        sendTelemetryEvent(EventName.UNITTEST_NAVIGATE_TEST_FUNCTION, undefined, { focus });
         if (this.cancellationToken) {
             this.cancellationToken.cancel();
         }
@@ -33,7 +36,7 @@ export class TestFunctionCodeNavigator implements ITestCodeNavigator {
         const [doc, editor] = await this.helper.openFile(Uri.file(item.parentTestFile.fullPath));
         let range: Range | undefined;
         if (item.testFunction.line) {
-            range = new Range(item.testFunction.line, 0, item.testFunction.line + 1, 0);
+            range = new Range(item.testFunction.line, 0, item.testFunction.line, 0);
         } else {
             const predicate = (s: SymbolInformation) => s.name === item.testFunction.name && (s.kind === SymbolKind.Method || s.kind === SymbolKind.Function);
             const symbol = await this.helper.findSymbol(doc, predicate, this.cancellationToken.token);
@@ -43,6 +46,11 @@ export class TestFunctionCodeNavigator implements ITestCodeNavigator {
             traceError('Unable to navigate to test function', new Error('Test Function not found'));
             return;
         }
-        editor.revealRange(range, TextEditorRevealType.Default);
+        if (focus) {
+            range = new Range(range.start.line, range.start.character, range.start.line, range.start.character);
+            await this.docManager.showTextDocument(doc, { preserveFocus: false, selection: range });
+        } else {
+            editor.revealRange(range, TextEditorRevealType.Default);
+        }
     }
 }

src/client/unittests/navigation/helper.ts

@@ -15,7 +15,7 @@ export class TestNavigatorHelper implements ITestNavigatorHelper {
     constructor(
         @inject(IDocumentManager) private readonly documentManager: IDocumentManager,
         @inject(IDocumentSymbolProvider) @named('test') private readonly symbolProvider: IDocumentSymbolProvider
-    ) {}
+    ) { }
     public async openFile(file?: Uri): Promise<[TextDocument, TextEditor]> {
         if (!file) {
             throw new Error('Unable to navigate to an undefined test file');

src/client/unittests/navigation/suiteNavigator.ts

@@ -5,9 +5,10 @@
 
 import { inject, injectable } from 'inversify';
 import { CancellationTokenSource, Range, SymbolInformation, SymbolKind, TextEditorRevealType, Uri } from 'vscode';
+import { IDocumentManager } from '../../common/application/types';
 import { traceError } from '../../common/logger';
 import { swallowExceptions } from '../../common/utils/decorators';
-import { captureTelemetry } from '../../telemetry';
+import { captureTelemetry, sendTelemetryEvent } from '../../telemetry';
 import { EventName } from '../../telemetry/constants';
 import { ITestCollectionStorageService, TestSuite } from '../common/types';
 import { ITestCodeNavigator, ITestNavigatorHelper } from './types';
@@ -17,11 +18,13 @@ export class TestSuiteCodeNavigator implements ITestCodeNavigator {
     private cancellationToken?: CancellationTokenSource;
     constructor(
         @inject(ITestNavigatorHelper) private readonly helper: ITestNavigatorHelper,
+        @inject(IDocumentManager) private readonly docManager: IDocumentManager,
         @inject(ITestCollectionStorageService) private readonly storage: ITestCollectionStorageService
-    ) {}
+    ) { }
     @swallowExceptions('Navigate to test suite')
     @captureTelemetry(EventName.UNITTEST_NAVIGATE_TEST_SUITE, undefined, true)
-    public async navigateTo(resource: Uri, suite: TestSuite): Promise<void> {
+    public async navigateTo(resource: Uri, suite: TestSuite, focus: boolean = true): Promise<void> {
+        sendTelemetryEvent(EventName.UNITTEST_NAVIGATE_TEST_SUITE, undefined, { focus });
         if (this.cancellationToken) {
             this.cancellationToken.cancel();
         }
@@ -33,7 +36,7 @@ export class TestSuiteCodeNavigator implements ITestCodeNavigator {
         const [doc, editor] = await this.helper.openFile(Uri.file(item.parentTestFile.fullPath));
         let range: Range | undefined;
         if (item.testSuite.line) {
-            range = new Range(item.testSuite.line, 0, item.testSuite.line + 1, 0);
+            range = new Range(item.testSuite.line, 0, item.testSuite.line, 0);
         } else {
             const predicate = (s: SymbolInformation) => s.name === item.testSuite.name && s.kind === SymbolKind.Class;
             const symbol = await this.helper.findSymbol(doc, predicate, this.cancellationToken.token);
@@ -43,6 +46,11 @@ export class TestSuiteCodeNavigator implements ITestCodeNavigator {
             traceError('Unable to navigate to test suite', new Error('Test Suite not found'));
             return;
         }
-        editor.revealRange(range, TextEditorRevealType.Default);
+        if (focus) {
+            range = new Range(range.start.line, range.start.character, range.start.line, range.start.character);
+            await this.docManager.showTextDocument(doc, { preserveFocus: false, selection: range });
+        } else {
+            editor.revealRange(range, TextEditorRevealType.Default);
+        }
     }
 }

src/client/unittests/navigation/types.ts

@@ -20,7 +20,7 @@ export enum NavigableItemType {
 
 export const ITestCodeNavigator = Symbol('ITestCodeNavigator');
 export interface ITestCodeNavigator {
-    navigateTo(resource: Uri, item: NavigableItem): Promise<void>;
+    navigateTo(resource: Uri, item: NavigableItem, focus: boolean): Promise<void>;
 }
 
 export const ITestNavigatorHelper = Symbol('ITestNavigatorHelper');

src/client/unittests/providers/commandHandlers.ts

@@ -50,7 +50,7 @@ export class TestExplorerCommandHandler implements ITestExplorerCommandHandler {
             throw new Error('Unknown Test Type');
         }
 
-        this.cmdManager.executeCommand(command, item.resource, item.data);
+        this.cmdManager.executeCommand(command, item.resource, item.data, true);
     }
     protected async runDebugTestNode(item: TestTreeItem, runType: 'run' | 'debug'): Promise<void> {
         let testToRun: TestsToRun;

src/client/unittests/providers/testTreeViewItem.ts

@@ -6,6 +6,8 @@
 import {
     TreeItem, TreeItemCollapsibleState, Uri
 } from 'vscode';
+import { Commands } from '../../common/constants';
+import { noop } from '../../common/utils/misc';
 import { TestsHelper } from '../common/testUtils';
 import {
     TestFile, TestFolder, TestFunction,
@@ -43,6 +45,7 @@ export class TestTreeItem extends TreeItem {
         this.tooltip = `Status: ${testStatus}`;
         this.testType = TestsHelper.getTestType(this.data);
         this.contextValue = TestsHelper.getTestType(this.data);
+        this.setCommand();
     }
 
     public static createFromFolder(
@@ -151,4 +154,23 @@ export class TestTreeItem extends TreeItem {
     public get parent(): TestTreeItem | undefined {
         return this.myParent;
     }
+    private setCommand() {
+        switch (this.testType) {
+            case TestType.testFile: {
+                this.command = { command: Commands.navigateToTestFile, title: 'Open', arguments: [this.resource, this.data] };
+                break;
+            }
+            case TestType.testFunction: {
+                this.command = { command: Commands.navigateToTestFunction, title: 'Open', arguments: [this.resource, this.data, false] };
+                break;
+            }
+            case TestType.testSuite: {
+                this.command = { command: Commands.navigateToTestSuite, title: 'Open', arguments: [this.resource, this.data, false] };
+                break;
+            }
+            default: {
+                noop();
+            }
+        }
+    }
 }

src/test/unittests/explorer/testExplorerCommandHandler.unit.test.ts

@@ -61,7 +61,7 @@ suite('Unit Tests - Test Explorer Command Hanlder', () => {
         const handler = capture(cmdManager.registerCommand).last()[1];
         await handler.bind(commandHandler)(instance(treeItem));
 
-        verify(cmdManager.executeCommand(expectedCommand, resource, data)).once();
+        verify(cmdManager.executeCommand(expectedCommand, resource, data, true)).once();
     }
     test('Opening a file will invoke correct command', async () => {
         const testFilePath = 'some file path';

src/test/unittests/navigation/fileNavigator.unit.test.ts

@@ -25,7 +25,7 @@ suite('Unit Tests - Navigation File', () => {
         const filePath = Uri.file('some file Path');
         when(helper.openFile(anything())).thenResolve();
 
-        await navigator.navigateTo(filePath, { fullPath: filePath.fsPath } as any);
+        await navigator.navigateTo(filePath, { fullPath: filePath.fsPath } as any, false);
 
         verify(helper.openFile(anything())).once();
         expect(capture(helper.openFile).first()[0]!.fsPath).to.equal(filePath.fsPath);
@@ -34,7 +34,7 @@ suite('Unit Tests - Navigation File', () => {
         const filePath = Uri.file('some file Path');
         when(helper.openFile(anything())).thenReject(new Error('kaboom'));
 
-        await navigator.navigateTo(filePath, { fullPath: filePath.fsPath } as any);
+        await navigator.navigateTo(filePath, { fullPath: filePath.fsPath } as any, false);
 
         verify(helper.openFile(anything())).once();
         expect(capture(helper.openFile).first()[0]!.fsPath).to.equal(filePath.fsPath);

src/test/unittests/navigation/functionNavigator.unit.test.ts

@@ -5,9 +5,11 @@
 
 import { expect, use } from 'chai';
 import * as chaisAsPromised from 'chai-as-promised';
-import { anything, capture, instance, mock, verify, when } from 'ts-mockito';
+import { anything, capture, deepEqual, instance, mock, verify, when } from 'ts-mockito';
 import * as typemoq from 'typemoq';
 import { Location, Range, SymbolInformation, SymbolKind, TextDocument, TextEditor, TextEditorRevealType, Uri } from 'vscode';
+import { DocumentManager } from '../../../client/common/application/documentManager';
+import { IDocumentManager } from '../../../client/common/application/types';
 import { TestCollectionStorageService } from '../../../client/unittests/common/services/storageService';
 import { ITestCollectionStorageService } from '../../../client/unittests/common/types';
 import { TestFunctionCodeNavigator } from '../../../client/unittests/navigation/functionNavigator';
@@ -20,15 +22,17 @@ use(chaisAsPromised);
 suite('Unit Tests - Navigation Function', () => {
     let navigator: TestFunctionCodeNavigator;
     let helper: ITestNavigatorHelper;
+    let docManager: IDocumentManager;
     let doc: typemoq.IMock<TextDocument>;
     let editor: typemoq.IMock<TextEditor>;
     let storage: ITestCollectionStorageService;
     setup(() => {
         doc = typemoq.Mock.ofType<TextDocument>();
         editor = typemoq.Mock.ofType<TextEditor>();
         helper = mock(TestNavigatorHelper);
+        docManager = mock(DocumentManager);
         storage = mock(TestCollectionStorageService);
-        navigator = new TestFunctionCodeNavigator(instance(helper), instance(storage));
+        navigator = new TestFunctionCodeNavigator(instance(helper), instance(docManager), instance(storage));
     });
     test('Ensure file is opened', async () => {
         const filePath = Uri.file('some file Path');
@@ -52,13 +56,13 @@ suite('Unit Tests - Navigation Function', () => {
         verify(helper.openFile(anything())).once();
         expect(capture(helper.openFile).first()[0]!.fsPath).to.equal(filePath.fsPath);
     });
-    test('Ensure we use line number from test function when navigating in file', async () => {
+    async function navigateToFunction(focusCode: boolean) {
         const filePath = Uri.file('some file Path');
         const line = 999;
         when(helper.openFile(anything())).thenResolve([doc.object, editor.object]);
         const flattenedFn = { parentTestFile: { fullPath: filePath.fsPath }, testFunction: { name: 'function_name' } };
         when(storage.findFlattendTestFunction(filePath, anything())).thenReturn(flattenedFn as any);
-        const range = new Range(line, 0, line + 1, 0);
+        const range = new Range(line, 0, line, 0);
         const symbol: SymbolInformation = {
             containerName: '',
             kind: SymbolKind.Function,
@@ -67,27 +71,22 @@ suite('Unit Tests - Navigation Function', () => {
         };
         when(helper.findSymbol(doc.object, anything(), anything())).thenResolve(symbol);
 
-        await navigator.navigateTo(filePath, { name: 'function_name' } as any);
+        await navigator.navigateTo(filePath, { name: 'function_name' } as any, focusCode);
 
         verify(helper.openFile(anything())).once();
         verify(helper.findSymbol(doc.object, anything(), anything())).once();
         expect(capture(helper.openFile).first()[0]!.fsPath).to.equal(filePath.fsPath);
-        editor.verify(e => e.revealRange(range, TextEditorRevealType.Default), typemoq.Times.once());
+        if (focusCode) {
+            verify(docManager.showTextDocument(doc.object, deepEqual({ preserveFocus: false, selection: range }))).once();
+        } else {
+            editor.verify(e => e.revealRange(typemoq.It.isAny(), TextEditorRevealType.Default), typemoq.Times.once());
+        }
+    }
+    test('Ensure we use line number from test function when navigating in file (without focusing code)', async () => {
+        await navigateToFunction(false);
     });
-    test('Ensure we use line number from test function when navigating in file', async () => {
-        const filePath = Uri.file('some file Path');
-        const line = 999;
-        when(helper.openFile(anything())).thenResolve([doc.object, editor.object]);
-        const flattenedFn = { parentTestFile: { fullPath: filePath.fsPath }, testFunction: { line } };
-        when(storage.findFlattendTestFunction(filePath, anything())).thenReturn(flattenedFn as any);
-        const range = new Range(line, 0, line + 1, 0);
-
-        await navigator.navigateTo(filePath, { line } as any);
-
-        verify(helper.openFile(anything())).once();
-        verify(helper.findSymbol(anything(), anything(), anything())).never();
-        expect(capture(helper.openFile).first()[0]!.fsPath).to.equal(filePath.fsPath);
-        editor.verify(e => e.revealRange(range, TextEditorRevealType.Default), typemoq.Times.once());
+    test('Ensure we use line number from test function when navigating in file (focusing code)', async () => {
+        await navigateToFunction(true);
     });
     test('Ensure file is opened and range not revealed', async () => {
         const filePath = Uri.file('some file Path');